Sanitize proxy response codes to CONNECT requests. For

anything other than 200 (success) or 400-599 (error), we
rewrite the response code as 500 (internal server error)
to prevent any special handling of the proxy's response to
CONNECT by mistake.

Add a new error code ERR_UNEXPECTED_SERVER_AUTH for a 401
response to a CONNECT request.

Fix nits reported by cpplint.py.

R=darin,eroman
BUG=7338
Review URL: http://codereview.chromium.org/21158

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

3 files changed, 66 insertions, 26 deletions

diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 6a3535b..d29b31e 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -209,6 +209,12 @@ bool URLRequestHttpJob::IsRedirectResponse(GURL* location,
   if (!response_info_->headers->IsRedirect(&value))
     return false;
 
+  // For HTTPS, if we didn't receive a server certificate, the response was
+  // from the proxy server (a response to the CONNECT request) rather than
+  // the server.
+  if (request_->url().SchemeIsSecure() && !response_info_->ssl_info.cert)
+    return false;
+
   *location = request_->url().Resolve(value);
   *http_status_code = response_info_->headers->response_code();
   return true;
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 5c9146f..0cadd0e 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -55,10 +55,10 @@ class URLRequestHttpCacheContext : public URLRequestContext {
 
 class TestURLRequest : public URLRequest {
  public:
-   TestURLRequest(const GURL& url, Delegate* delegate)
-       : URLRequest(url, delegate) {
-     set_context(new URLRequestHttpCacheContext());
-   }
+  TestURLRequest(const GURL& url, Delegate* delegate)
+      : URLRequest(url, delegate) {
+    set_context(new URLRequestHttpCacheContext());
+  }
 };
 
 StringPiece TestNetResourceProvider(int key) {
@@ -82,6 +82,34 @@ bool ContainsString(const std::string& haystack, const char* needle) {
 class URLRequestTest : public PlatformTest {
 };
 
+TEST_F(URLRequestTest, ProxyTunnelRedirectTest) {
+  // In this unit test, we're using the HTTPTestServer as a proxy server and
+  // issue a CONNECT request with the magic host name "www.redirect.com" to
+  // it.  The HTTPTestServer will return a 302 response, which we should not
+  // follow.
+  scoped_refptr<HTTPTestServer> server =
+      HTTPTestServer::CreateServer(L"", NULL);
+  ASSERT_TRUE(NULL != server.get());
+  TestDelegate d;
+  {
+    URLRequest r(GURL("https://www.redirect.com/"), &d);
+    std::string proxy("localhost:");
+    proxy.append(IntToString(kHTTPDefaultPort));
+    r.set_context(new TestURLRequestContext(proxy));
+
+    r.Start();
+    EXPECT_TRUE(r.is_pending());
+
+    MessageLoop::current()->Run();
+
+    EXPECT_EQ(1, d.response_started_count());
+    // We should have rewritten the 302 response code as 500.
+    EXPECT_EQ(500, r.GetResponseCode());
+    // We should not have followed the redirect.
+    EXPECT_EQ(0, d.received_redirect_count());
+  }
+}
+
 TEST_F(URLRequestTest, GetTest_NoCache) {
   scoped_refptr<HTTPTestServer> server =
       HTTPTestServer::CreateServer(L"", NULL);
@@ -100,7 +128,7 @@ TEST_F(URLRequestTest, GetTest_NoCache) {
     EXPECT_NE(0, d.bytes_received());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -122,15 +150,15 @@ TEST_F(URLRequestTest, GetTest) {
     EXPECT_NE(0, d.bytes_received());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
 class HTTPSRequestTest : public testing::Test {
  protected:
-   HTTPSRequestTest() : util_() {};
+  HTTPSRequestTest() : util_() {}
 
-   SSLTestUtil util_;
+  SSLTestUtil util_;
 };
 
 #if defined(OS_MACOSX)
@@ -165,7 +193,7 @@ TEST_F(HTTPSRequestTest, MAYBE_HTTPSGetTest) {
     EXPECT_NE(0, d.bytes_received());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -188,7 +216,7 @@ TEST_F(URLRequestTest, CancelTest) {
     EXPECT_FALSE(d.received_data_before_response());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -217,7 +245,7 @@ TEST_F(URLRequestTest, CancelTest2) {
     EXPECT_EQ(URLRequestStatus::CANCELED, r.status().status());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -245,7 +273,7 @@ TEST_F(URLRequestTest, CancelTest3) {
     EXPECT_EQ(URLRequestStatus::CANCELED, r.status().status());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -320,7 +348,7 @@ TEST_F(URLRequestTest, PostTest) {
   char *uploadBytes = new char[kMsgSize+1];
   char *ptr = uploadBytes;
   char marker = 'a';
-  for(int idx=0; idx<kMsgSize/10; idx++) {
+  for (int idx = 0; idx < kMsgSize/10; idx++) {
     memcpy(ptr, "----------", 10);
     ptr += 10;
     if (idx % 100 == 0) {
@@ -329,7 +357,6 @@ TEST_F(URLRequestTest, PostTest) {
       if (++marker > 'z')
         marker = 'a';
     }
-
   }
   uploadBytes[kMsgSize] = '\0';
 
@@ -354,12 +381,12 @@ TEST_F(URLRequestTest, PostTest) {
 
     EXPECT_FALSE(d.received_data_before_response());
     EXPECT_EQ(uploadBytes, d.data_received());
-    EXPECT_EQ(memcmp(uploadBytes, d.data_received().c_str(), kMsgSize),0);
+    EXPECT_EQ(memcmp(uploadBytes, d.data_received().c_str(), kMsgSize), 0);
     EXPECT_EQ(d.data_received().compare(uploadBytes), 0);
   }
   delete[] uploadBytes;
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -384,7 +411,7 @@ TEST_F(URLRequestTest, PostEmptyTest) {
     EXPECT_TRUE(d.data_received().empty());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -435,7 +462,7 @@ TEST_F(URLRequestTest, PostFileTest) {
     EXPECT_EQ(0, memcmp(d.data_received().c_str(), buf.get(), size));
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -454,7 +481,7 @@ TEST_F(URLRequestTest, AboutBlankTest) {
     EXPECT_EQ(d.bytes_received(), 0);
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -481,7 +508,7 @@ TEST_F(URLRequestTest, FileTest) {
     EXPECT_EQ(d.bytes_received(), static_cast<int>(file_size));
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -497,7 +524,7 @@ TEST_F(URLRequestTest, InvalidUrlTest) {
     EXPECT_TRUE(d.request_failed());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -515,7 +542,7 @@ TEST_F(URLRequestTest, DISABLED_DnsFailureTest) {
     EXPECT_TRUE(d.request_failed());
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 
@@ -666,7 +693,7 @@ TEST_F(URLRequestTest, ResolveShortcutTest) {
   CoUninitialize();
 
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 }
 #endif  // defined(OS_WIN)
@@ -713,7 +740,7 @@ TEST_F(URLRequestTest, FileDirCancelTest) {
     MessageLoop::current()->Run();
   }
 #ifndef NDEBUG
-  DCHECK_EQ(url_request_metrics.object_count,0);
+  DCHECK_EQ(url_request_metrics.object_count, 0);
 #endif
 
   // Take out mock resource provider.
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index de75614..7107d50 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -45,6 +45,14 @@ class TestURLRequestContext : public URLRequestContext {
         net::HttpNetworkLayer::CreateFactory(proxy_service_);
   }
 
+  explicit TestURLRequestContext(const std::string& proxy) {
+    net::ProxyInfo proxy_info;
+    proxy_info.UseNamedProxy(proxy);
+    proxy_service_ = net::ProxyService::Create(&proxy_info);
+    http_transaction_factory_ =
+        net::HttpNetworkLayer::CreateFactory(proxy_service_);
+  }
+
   virtual ~TestURLRequestContext() {
     delete http_transaction_factory_;
     delete proxy_service_;
@@ -355,7 +363,6 @@ class BaseTestServer : public base::RefCounted<BaseTestServer> {
 #endif
     if (!normalized_document_root.empty())
       file_util::AppendToPath(test_data_directory, normalized_document_root);
-
     data_directory_ = *test_data_directory;
   }
 
@@ -482,7 +489,7 @@ class HTTPTestServer : public BaseTestServer {
     {
       MessageLoop* loop = loop_;
       scoped_ptr<base::Thread> io_thread;
-      
+
       if (!loop) {
         io_thread.reset(new base::Thread("MakeGETRequest"));
         base::Thread::Options options;