Clean up our fix for bug 7338 and bug 8473, which went

through several iterations.

R=abarth
BUG=7338,8473
Review URL: http://codereview.chromium.org/42148

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@11589 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 16 insertions, 9 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 95c0aef..bcd5205 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -215,6 +215,8 @@ int HttpNetworkTransaction::Read(IOBuffer* buf, int buf_len,
     // network attacker can already control HTTP sessions.
     // We reach this case when the user cancels a 407 proxy auth prompt.
     // See http://crbug.com/8473
+    DCHECK(response_.headers->response_code() == 407);
+    LogBlockedTunnelResponse(*response_.headers);
     return ERR_TUNNEL_CONNECTION_FAILED;
   }
 
@@ -953,6 +955,14 @@ void HttpNetworkTransaction::LogTransactionMetrics() const {
       static_cast<int> (response_body_read_ / duration.InMilliseconds()));
 }
 
+void HttpNetworkTransaction::LogBlockedTunnelResponse(
+    const HttpResponseHeaders& headers) const {
+  LOG(WARNING) << "Blocked proxy response with status "
+               << headers.response_code() << " to CONNECT request for "
+               << request_->url.host() << ":"
+               << request_->url.EffectiveIntPort() << ".";
+}
+
 int HttpNetworkTransaction::DidReadResponseHeaders() {
   scoped_refptr<HttpResponseHeaders> headers;
   if (has_found_status_line_start()) {
@@ -1011,11 +1021,7 @@ int HttpNetworkTransaction::DidReadResponseHeaders() {
         // 501 response bodies that contain a useful error message.  For
         // example, Squid uses a 404 response to report the DNS error: "The
         // domain name does not exist."
-        LOG(WARNING) <<
-            "Blocked proxy response to CONNECT request with status " <<
-            headers->response_code() << " for " <<
-            request_->url.host() << ":" <<
-            request_->url.EffectiveIntPort() << ".";
+        LogBlockedTunnelResponse(*headers);
         return ERR_TUNNEL_CONNECTION_FAILED;
     }
   }
@@ -1425,6 +1431,7 @@ int HttpNetworkTransaction::HandleAuthChallenge() {
       // We are establishing a tunnel, we can't show the error page because an
       // active network attacker could control its contents.  Instead, we just
       // fail to establish the tunnel.
+      DCHECK(target == HttpAuth::AUTH_PROXY);
       return ERR_PROXY_AUTH_REQUESTED;
     }
     // We found no supported challenge -- let the transaction continue
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index 49e1a41..1e14ab8 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -82,6 +82,10 @@ class HttpNetworkTransaction : public HttpTransaction {
   // well as effective bandwidth used.
   void LogTransactionMetrics() const;
 
+  // Writes a log message to help debugging in the field when we block a proxy
+  // response to a CONNECT request.
+  void LogBlockedTunnelResponse(const HttpResponseHeaders& headers) const;
+
   // Called when header_buf_ contains the complete response headers.
   int DidReadResponseHeaders();
 
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index 918c293..1d15911 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -201,10 +201,6 @@ class HttpResponseHeaders :
   // response code is not found in the raw headers.
   int response_code() const { return response_code_; }
 
-  // Sets the HTTP response code to the new code.  The original HTTP response
-  // code is still available in the raw and parsed headers.
-  void set_response_code(int new_code) { response_code_ = new_code; }
-
   // Returns the raw header string.
   const std::string& raw_headers() const { return raw_headers_; }