-
Notifications
You must be signed in to change notification settings - Fork 290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A new cookie never be accepted if an HTTPClient has the same expired cookie. #154
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before this fix, CookieManager#add find a cookie, and then expire old cookies, so the found cookie might be removed before being updated.
Hiroshi Nakamura » httpclient #68 FAILURE |
nahi
pushed a commit
that referenced
this pull request
Dec 2, 2013
A new cookie never be accepted if an HTTPClient has the same expired cookie.
Thank you! |
jsonn
pushed a commit
to jsonn/pkgsrc
that referenced
this pull request
Jan 21, 2015
(with post 2.6.0 fix: bin/httpclient one-liner broken) ## Changes ### Changes in 2.6.0 This release includes internal CookieManager implementation change. It involves compatibility layer but for the case your library depends on internal implementation it also provides a way to restore the implementation. See below for more details. * Changes * feat: use http-cookie if available for better Cookies spec compliance. Instead of WebAgent 0.6.2 that is not maintained over 10 years. To omit maintaining that library use http-cookie for better spec compliance and healthy development. This introduces following incompatibility from existing cookies implementation. * Expired cookies are not saved. With the old implementation expired cookies are saved in file and not be sent to the server. With the new implementation the expired cookies are not saved to the file and not be sent to the server. * Cookie#domain returns dot-less domain for domain cookies. Instead, Cookie#dot_domain returns with dot. http-cookie is used by default if available but you can restore original CookieManager behavior by loading 'httpclient/webagent-cookie' feature before 'httpclient' like this; ```ruby require 'httpclient/webagent-cookie' require 'httpclient' ``` The new implementation dumps warnings to help you migrate to http-cookie. Please follow the suggestion to avoid future compatibility. ```ruby e.g. WebAgent::Cookie is deprecated and will be replaced with HTTP::Cookie in the near future. Please use Cookie#origin= instead of Cookie#url= for the replacement. Cookie#domain returns dot-less domain name now. Use Cookie#dot_domain if you need "." at the beginning. CookieManager#find is deprecated and will be removed in near future. Use HTTP::Cookie.cookie_value(CookieManager#cookies) instead ``` * feat: Message#previous to get responses in negotiation HTTP::Message#previous keeps previous response in negotiation. For redirection, authorization negotiation and retry from custom filter. Closes #234. * feat: Add JSONClient JSONClient auto-converts Hash <-> JSON in request and response. * For POST or PUT request, convert Hash body to JSON String with 'application/json; charset=utf-8' header. * For response, convert JSON String to Hash when content-type is '(application|text)/(x-)?json' This commit include bin/jsonclient that works as same as bin/httpclient not with HTTPClient but with JSONClient. * feat: Add download command ``` % httpclient download http://host/path > file ``` * Bug fixes * fix: duplicated query params by follow_redirect When the original request has query and the server returns redirection response with Location, HTTPClient wrongly adds query to the new URI. In such case the Location header could include query part; ``` e.g. http://originalhost/api/call?limit=10 -> Location: http://otherhost/api/call?limit=10 ``` HTTPClient should just hit the new location '/api/call?limit=10' not '/api/call?limit=10&limit=10'. Closes #236. * fix: NTLM & Basic dual auth When a server returns two or more WWW-Authenticate headers and the first one is NTLM, say WWW-Authenticate: NTLM and WWW-Authenticate: Basic in this order, HTTPClient sent Basic Authorization header after finishing NTLM auth negotiation. NTLM auth is a connection authentication scheme so HTTPClient deleted the internal auth negotiation state so that NTLM authenticator does not do anything after the negotiation has completed. In such case, for the subsequent requests, NTLM authenticator does nothing but Basic authenticator sends Basic Authorization header to the server that is already negotiated via NTLM authenticator. This can cause authentication failure. This commit changes the internal state handling not to delete the state but introduce :done state. NTLM authenticator returns :skip for the request to the server that auth negotiation has completed. WWWAuth skips other authenticator to avoid above issue. Closes #157. * fix: transplant IO positions to new request in negotiation In authorization negotiation HTTP::Message for request is generated for each request, of course, but HTTPClient did not care the IO position recorded in the previous requests in the subsequent requests. Closes #130. * fix: avoid inconsistent Content-Length and actual body If lengths of all posted arguments are known HTTPClient sends 'Content-Length' as a sum length of all arguments. But the length of actual body was wrong because it read as much as possible regardless of what IO#size returned. So if the file is getting bigger while HTTPClient is processing a request the request has inconsistent Content-Length and body. This bug is found, and the fix is proposed both by @Teshootub7. Thank you very much for patient trouble shooting! Fixes #117. * fix: KeepAliveDisconnected race condition As details explained in #84, current HTTPClient's KeepAliveDisconnected handling has a race condition bug that allows a client to have invalidated connection two or more times. This could be a cause of #185. To avoid this, make HTTPClient acquire new connection for retry of KeepAliveDisconnected. Closes #84. Closes #185. ### Changes in 2.5.3 This release includes behavior changes of POST and PUT requests that has nil as a body. See changes below. Emtpty String as a body is not affected. * Changes * Update cacert. "Certificate data from Mozilla as of: Tue Oct 28 22:03:58 2014" -> Reverted in 2.5.3.3 because it caused unexpected SSLError. See nahi/httpclient#230 * Allow no content POST and PUT. Previously POST or PUT with :body => nil meant that 'POST or PUT with 0 length entity body'. But sometimes you need to POST or PUT actually no content which should not have Content-Type nor Content-Length. It could be incompatible change for user who POST/PUT-ed with empty body but it should be rare, actually WEBrick cannot handle such 'no content' POST and PUT. #128. * Add default_header property. :default_header is for providing default headers Hash that all HTTP requests should have, such as custom 'Authorization' header in API. You can override :default_header with :header Hash parameter in HTTP request methods. * raise if redirect res does not have Location header. #155. * Bug fixes * Avoid NPE by a cookie without domain=. The root cause is still uncertain though. Closes #123 * Suppress verify_callback warning. Because OpenSSL can try multiple certificate chains and some of it can fail, and one of them succeeds. For that case warning is irrelevant. Let it warn only in $DEBUG mode. #221. ### Changes in 2.5.2 Oct 29, 2014 - version 2.5.2 * Changes * Add :force_basic_auth config - #166, #179, #181. Generally HTTP client must send Authorization header after it gets 401 error from server from security reason. But in some situation (e.g. API client) you might want to send Authorization from the beginning. You can turn on/off force_basic_auth flag for sending Authorization header from the beginning. (Of cource, if a request URI matches with the URI you set in set_auth method) Syntax: ```ruby HTTPClient.new(:force_basic_auth => true) # or c = HTTPClient.new c.force_basic_auth = true ``` * Add :base_url to HTTPClient configuration. Passing path to get, post, etc. is recognized as a request to :base_url + uri. If you pass full URL :base_url is ignored. ```ruby api = HTTPClient.new(:base_url => 'https://api.example.com/v1') api.get("/users.json") # => Get https://api.example.com/v1/users.json api.get("https://localhost/path") # => https://localhost/path ``` ### Changes in 2.5.1 Oct 19, 2014 - version 2.5.1 * Changes * Allow to specify :query in POST, PUT, DELETE and OPTIONS requests. Closes #83. * Allow to specify :body in OPTIONS request. Closes #136. ### Changes in 2.5.0 Oct 17, 2014 - version 2.5.0 **IMPORTANT CHANGES** This version changes (again) default SSL options to help BEAST/CRIME/POODLE Attack prevension. * Disabled SSLv3 in favor of POODLE Attack prevention. * Enabled 1/n-1 fragment in favor of BEAST Attack prevention. * No TLS compression in favor of CRIME Attack prevention. You can restore the previous SSL configuration like this; ```ruby client = HTTPClient.new client.ssl_config.ssl_version = :SSLv23 client.ssl_config.options = OpenSSL::SSL::OP_ALL | OpenSSL::SSL::OP_NO_SSLv2 ``` * Changes * Change default SSL options. See above. * Keep cause error of KeepAliveDisconnected. It allows caller to investigate the cause of KeepAliveDisconnected. ### Changes in 2.4.0 Jun 8, 2014 - version 2.4.0 **IMPORTANT CHANGES** This version changes default SSL version to :auto (same as nil) to use SSL/TLS version negotiation. Former versions use SSLv3 as default that does not connect via TLS. This change makes underlying OpenSSL library decide which SSL/TLS version to use but SSLv2 is disabled. This change makes your secure connection safer but if you see SSL connection failure with this version try specifying SSL version to use SSLv3 like; ``` client = HTTPClient.new client.ssl_config.ssl_version = :SSLv3 ``` * Bug fixes * Avoid unnecessary connection retries for OAuth error. [#203](nahi/httpclient#203) * Make authentication drivers Thread-safe. Note that HTTPClient instance is Thread-safe for authentication state update but it shares authentication state across threads by design. If you don't want to share authentication state, such as for using different authentication username/password pair per thread, create HTTPClient instance for each Thread. [#200](nahi/httpclient#200) * Avoid chunked String recycle in callback block. [#193](nahi/httpclient#193) * Do not send empty 'oauth_token' in signed request for compatibility. [#188](nahi/httpclient#188) * Ignore negative Content-Length header from server. [#175](nahi/httpclient#175) * Fix incorrect use of absolute URL for HTTPS proxy requests. [#168](nahi/httpclient#168) * Handle UTF characters in chunked bodies. [#167](nahi/httpclient#167) * A new cookie never be accepted if an HTTPClient has the same expired cookie. [#154](nahi/httpclient#154) * Allow spaces in NO_PROXY environment like; "hosta, hostb" [#141](nahi/httpclient#141) * Avoid HttpClient::Message::Body#dump causes Encoding::CompatibilityError. [#140](nahi/httpclient#140) * Changes * Change default SSL version to :auto to use version negotiation. [#186](nahi/httpclient#186), [#204](nahi/httpclient#204) * Allow to pass client private key passphrase in SSLConfig. [#201](nahi/httpclient#201) * Convert README to markdown syntax [#198](nahi/httpclient#198) * Update default CA certificates: change the source from JDK's to Firefox's. The file is downloaded from https://raw.githubusercontent.com/bagder/ca-bundle/master/ca-bundle.crt (Certificate data from Mozilla as of: Tue Apr 22 08:29:31 2014) [#195](nahi/httpclient#195) * Callback block can be defined as to get 2 arguments to retrieve the response object. [#194](nahi/httpclient#194) * Remove [] from given address for IPv6 compat. [#176](nahi/httpclient#176) * Update API endpoints to those of Twitter REST API v1.1. [#150](nahi/httpclient#150)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It seems that a new cookie never be accepted if an HTTPClient has the same expired cookie.
For example, if a cookie is configured to be expired within 5 seconds, the following
code prints received cookies in the first loop, but prints empty arrays in the second loop.
hc = HTTPClient.new
2.times do
hc.get("http://localhost/books")
p hc.cookies
hc.post("http://localhost/books",
'{"title":"hello","body":"world"}',
"Content-Type"=>"application/json; charset=utf-8")
p hc.cookies
sleep 5
end
It's OK that the old cookie is expired, but the new cookie should be accepted, shouldn't it?