Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1041)

Unified Diff: net/http/http_network_transaction.cc

Issue 242135: Merge r25564 and r26588 from the trunk.... (Closed) Base URL: svn://chrome-svn/chrome/branches/195/src/
Patch Set: Undo an extraneous comment change Created 11 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/http/http_network_transaction.h ('k') | net/http/http_network_transaction_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/http/http_network_transaction.cc
===================================================================
--- net/http/http_network_transaction.cc (revision 28069)
+++ net/http/http_network_transaction.cc (working copy)
@@ -144,6 +144,7 @@
proxy_mode_(kDirectConnection),
establishing_tunnel_(false),
reading_body_from_socket_(false),
+ embedded_identity_used_(false),
request_headers_(new RequestHeaders()),
request_headers_bytes_sent_(0),
header_buf_(new ResponseHeaders()),
@@ -254,6 +255,11 @@
// If auth_identity_[target].source is HttpAuth::IDENT_SRC_NONE,
// auth_identity_[target] contains no identity because identity is not
// required yet.
+ //
+ // TODO(wtc): For NTLM_SSPI, we add the same auth entry to the cache in
+ // round 1 and round 2, which is redundant but correct. It would be nice
+ // to add an auth entry to the cache only once, preferrably in round 1.
+ // See http://crbug.com/21015.
bool has_auth_identity =
auth_identity_[target].source != HttpAuth::IDENT_SRC_NONE;
if (has_auth_identity) {
@@ -278,42 +284,6 @@
// connection even though the server says it's keep-alive.
}
- // If the auth scheme is connection-based but the proxy/server mistakenly
- // marks the connection as non-keep-alive, the auth is going to fail, so log
- // an error message.
- if (!keep_alive && auth_handler_[target]->is_connection_based() &&
- has_auth_identity) {
- LOG(ERROR) << "Can't perform " << auth_handler_[target]->scheme()
- << " auth to the " << AuthTargetString(target) << " "
- << AuthOrigin(target) << " over a non-keep-alive connection";
-
- HttpVersion http_version = response_.headers->GetHttpVersion();
- LOG(ERROR) << " HTTP version is " << http_version.major_value() << "."
- << http_version.minor_value();
-
- std::string header_val;
- void* iter = NULL;
- while (response_.headers->EnumerateHeader(&iter, "connection",
- &header_val)) {
- LOG(ERROR) << " Has header Connection: " << header_val;
- }
-
- iter = NULL;
- while (response_.headers->EnumerateHeader(&iter, "proxy-connection",
- &header_val)) {
- LOG(ERROR) << " Has header Proxy-Connection: " << header_val;
- }
-
- // RFC 4559 requires that a proxy indicate its support of NTLM/Negotiate
- // authentication with a "Proxy-Support: Session-Based-Authentication"
- // response header.
- iter = NULL;
- while (response_.headers->EnumerateHeader(&iter, "proxy-support",
- &header_val)) {
- LOG(ERROR) << " Has header Proxy-Support: " << header_val;
- }
- }
-
// We don't need to drain the response body, so we act as if we had drained
// the response body.
DidDrainBodyForAuthRestart(keep_alive);
@@ -733,6 +703,9 @@
std::string authorization_headers;
+ // TODO(wtc): If BuildAuthorizationHeader fails (returns an authorization
+ // header with no credentials), we should return an error to prevent
+ // entering an infinite auth restart loop. See http://crbug.com/21050.
if (have_proxy_auth)
authorization_headers.append(
BuildAuthorizationHeader(HttpAuth::AUTH_PROXY));
@@ -1617,10 +1590,8 @@
DCHECK(auth_identity_[target].invalid);
// Try to use the username/password encoded into the URL first.
- // (By checking source == IDENT_SRC_NONE, we make sure that this
- // is only done once for the transaction.)
if (target == HttpAuth::AUTH_SERVER && request_->url.has_username() &&
- auth_identity_[target].source == HttpAuth::IDENT_SRC_NONE) {
+ !embedded_identity_used_) {
auth_identity_[target].source = HttpAuth::IDENT_SRC_URL;
auth_identity_[target].invalid = false;
// TODO(wtc) It may be necessary to unescape the username and password
@@ -1628,6 +1599,7 @@
// embedded nulls in that case.
auth_identity_[target].username = ASCIIToWide(request_->url.username());
auth_identity_[target].password = ASCIIToWide(request_->url.password());
+ embedded_identity_used_ = true;
// TODO(eroman): If the password is blank, should we also try combining
// with a password from the cache?
return true;
@@ -1711,10 +1683,17 @@
return ERR_UNEXPECTED_PROXY_AUTH;
// The auth we tried just failed, hence it can't be valid. Remove it from
- // the cache so it won't be used again, unless it's a null identity.
- if (HaveAuth(target) &&
- auth_identity_[target].source != HttpAuth::IDENT_SRC_NONE)
+ // the cache so it won't be used again.
+ // TODO(wtc): IsFinalRound is not the right condition. In a multi-round
+ // auth sequence, the server may fail the auth in round 1 if our first
+ // authorization header is broken. We should inspect response_.headers to
+ // determine if the server already failed the auth or wants us to continue.
+ // See http://crbug.com/21015.
+ if (HaveAuth(target) && auth_handler_[target]->IsFinalRound()) {
InvalidateRejectedAuthFromCache(target);
+ auth_handler_[target] = NULL;
+ auth_identity_[target] = HttpAuth::Identity();
+ }
auth_identity_[target].invalid = true;
@@ -1723,6 +1702,7 @@
// Find the best authentication challenge that we support.
HttpAuth::ChooseBestChallenge(response_.headers.get(),
target,
+ AuthOrigin(target),
&auth_handler_[target]);
}
@@ -1749,14 +1729,11 @@
// If an identity to try is found, it is saved to auth_identity_[target].
SelectNextAuthIdentityToTry(target);
} else {
- // Proceed with a null identity.
+ // Proceed with the existing identity or a null identity.
//
// TODO(wtc): Add a safeguard against infinite transaction restarts, if
// the server keeps returning "NTLM".
- auth_identity_[target].source = HttpAuth::IDENT_SRC_NONE;
auth_identity_[target].invalid = false;
- auth_identity_[target].username.clear();
- auth_identity_[target].password.clear();
}
// Make a note that we are waiting for auth. This variable is inspected
@@ -1777,18 +1754,10 @@
AuthChallengeInfo* auth_info = new AuthChallengeInfo;
auth_info->is_proxy = target == HttpAuth::AUTH_PROXY;
+ auth_info->host_and_port = ASCIIToWide(GetHostAndPort(AuthOrigin(target)));
auth_info->scheme = ASCIIToWide(auth_handler_[target]->scheme());
// TODO(eroman): decode realm according to RFC 2047.
auth_info->realm = ASCIIToWide(auth_handler_[target]->realm());
-
- std::string host_and_port;
- if (target == HttpAuth::AUTH_PROXY) {
- host_and_port = proxy_info_.proxy_server().host_and_port();
- } else {
- DCHECK(target == HttpAuth::AUTH_SERVER);
- host_and_port = GetHostAndPort(request_->url);
- }
- auth_info->host_and_port = ASCIIToWide(host_and_port);
response_.auth_challenge = auth_info;
}
« no previous file with comments | « net/http/http_network_transaction.h ('k') | net/http/http_network_transaction_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698