Allow broken servers to send us multiple identical Location and

Content-Disposition headers.

R=willchan@chromium.org
BUG=103618

Review URL: http://codereview.chromium.org/9757002

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

4 files changed, 39 insertions, 81 deletions

diff --git a/net/http/http_network_transaction_spdy21_unittest.cc b/net/http/http_network_transaction_spdy21_unittest.cc
index d528065..c46ba1a 100644
--- a/net/http/http_network_transaction_spdy21_unittest.cc
+++ b/net/http/http_network_transaction_spdy21_unittest.cc
@@ -691,9 +691,9 @@ TEST_F(HttpNetworkTransactionSpdy21Test, SingleContentDispositionHeader) {
   EXPECT_EQ("Hello", out.response_data);
 }
 
-// Checks that two identical Content-Disposition headers result in an error.
+// Checks that two identical Content-Disposition headers result in no error.
 TEST_F(HttpNetworkTransactionSpdy21Test,
-       DuplicateIdenticalContentDispositionHeaders) {
+       TwoIdenticalContentDispositionHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 200 OK\r\n"),
     MockRead("Content-Disposition: attachment;filename=\"greetings.txt\"r\n"),
@@ -703,12 +703,13 @@ TEST_F(HttpNetworkTransactionSpdy21Test,
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads,
                                               arraysize(data_reads));
-  EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION, out.rv);
+  EXPECT_EQ(OK, out.rv);
+  EXPECT_EQ("HTTP/1.1 200 OK", out.status_line);
+  EXPECT_EQ("Hello", out.response_data);
 }
 
 // Checks that two distinct Content-Disposition headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy21Test,
-       DuplicateDistinctContentDispositionHeaders) {
+TEST_F(HttpNetworkTransactionSpdy21Test, TwoDistinctContentDispositionHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 200 OK\r\n"),
     MockRead("Content-Disposition: attachment;filename=\"greetings.txt\"r\n"),
@@ -721,11 +722,13 @@ TEST_F(HttpNetworkTransactionSpdy21Test,
   EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION, out.rv);
 }
 
-// Checks the behavior of a single Location header.
-TEST_F(HttpNetworkTransactionSpdy21Test, SingleLocationHeader) {
+// Checks that two identical Location headers result in no error.
+// Also tests Location header behavior.
+TEST_F(HttpNetworkTransactionSpdy21Test, TwoIdenticalLocationHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 302 Redirect\r\n"),
     MockRead("Location: http://good.com/\r\n"),
+    MockRead("Location: http://good.com/\r\n"),
     MockRead("Content-Length: 0\r\n\r\n"),
     MockRead(SYNCHRONOUS, OK),
   };
@@ -757,22 +760,8 @@ TEST_F(HttpNetworkTransactionSpdy21Test, SingleLocationHeader) {
   EXPECT_EQ("http://good.com/", url);
 }
 
-// Checks that two identical Location headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy21Test, DuplicateIdenticalLocationHeaders) {
-  MockRead data_reads[] = {
-    MockRead("HTTP/1.1 302 Redirect\r\n"),
-    MockRead("Location: http://good.com/\r\n"),
-    MockRead("Location: http://good.com/\r\n"),
-    MockRead("Content-Length: 0\r\n\r\n"),
-    MockRead(SYNCHRONOUS, OK),
-  };
-  SimpleGetHelperResult out = SimpleGetHelper(data_reads,
-                                              arraysize(data_reads));
-  EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_LOCATION, out.rv);
-}
-
 // Checks that two distinct Location headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy21Test, DuplicateDistinctLocationHeaders) {
+TEST_F(HttpNetworkTransactionSpdy21Test, TwoDistinctLocationHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 302 Redirect\r\n"),
     MockRead("Location: http://good.com/\r\n"),
diff --git a/net/http/http_network_transaction_spdy2_unittest.cc b/net/http/http_network_transaction_spdy2_unittest.cc
index 0912dee..aeab6e5 100644
--- a/net/http/http_network_transaction_spdy2_unittest.cc
+++ b/net/http/http_network_transaction_spdy2_unittest.cc
@@ -691,9 +691,9 @@ TEST_F(HttpNetworkTransactionSpdy2Test, SingleContentDispositionHeader) {
   EXPECT_EQ("Hello", out.response_data);
 }
 
-// Checks that two identical Content-Disposition headers result in an error.
+// Checks that two identical Content-Disposition headers result in no error.
 TEST_F(HttpNetworkTransactionSpdy2Test,
-       DuplicateIdenticalContentDispositionHeaders) {
+       TwoIdenticalContentDispositionHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 200 OK\r\n"),
     MockRead("Content-Disposition: attachment;filename=\"greetings.txt\"r\n"),
@@ -703,12 +703,13 @@ TEST_F(HttpNetworkTransactionSpdy2Test,
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads,
                                               arraysize(data_reads));
-  EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION, out.rv);
+  EXPECT_EQ(OK, out.rv);
+  EXPECT_EQ("HTTP/1.1 200 OK", out.status_line);
+  EXPECT_EQ("Hello", out.response_data);
 }
 
 // Checks that two distinct Content-Disposition headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy2Test,
-       DuplicateDistinctContentDispositionHeaders) {
+TEST_F(HttpNetworkTransactionSpdy2Test, TwoDistinctContentDispositionHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 200 OK\r\n"),
     MockRead("Content-Disposition: attachment;filename=\"greetings.txt\"r\n"),
@@ -721,11 +722,13 @@ TEST_F(HttpNetworkTransactionSpdy2Test,
   EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION, out.rv);
 }
 
-// Checks the behavior of a single Location header.
-TEST_F(HttpNetworkTransactionSpdy2Test, SingleLocationHeader) {
+// Checks that two identical Location headers result in no error.
+// Also tests Location header behavior.
+TEST_F(HttpNetworkTransactionSpdy2Test, TwoIdenticalLocationHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 302 Redirect\r\n"),
     MockRead("Location: http://good.com/\r\n"),
+    MockRead("Location: http://good.com/\r\n"),
     MockRead("Content-Length: 0\r\n\r\n"),
     MockRead(SYNCHRONOUS, OK),
   };
@@ -757,22 +760,8 @@ TEST_F(HttpNetworkTransactionSpdy2Test, SingleLocationHeader) {
   EXPECT_EQ("http://good.com/", url);
 }
 
-// Checks that two identical Location headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy2Test, DuplicateIdenticalLocationHeaders) {
-  MockRead data_reads[] = {
-    MockRead("HTTP/1.1 302 Redirect\r\n"),
-    MockRead("Location: http://good.com/\r\n"),
-    MockRead("Location: http://good.com/\r\n"),
-    MockRead("Content-Length: 0\r\n\r\n"),
-    MockRead(SYNCHRONOUS, OK),
-  };
-  SimpleGetHelperResult out = SimpleGetHelper(data_reads,
-                                              arraysize(data_reads));
-  EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_LOCATION, out.rv);
-}
-
 // Checks that two distinct Location headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy2Test, DuplicateDistinctLocationHeaders) {
+TEST_F(HttpNetworkTransactionSpdy2Test, TwoDistinctLocationHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 302 Redirect\r\n"),
     MockRead("Location: http://good.com/\r\n"),
diff --git a/net/http/http_network_transaction_spdy3_unittest.cc b/net/http/http_network_transaction_spdy3_unittest.cc
index a34158a..b674ca9 100644
--- a/net/http/http_network_transaction_spdy3_unittest.cc
+++ b/net/http/http_network_transaction_spdy3_unittest.cc
@@ -691,9 +691,9 @@ TEST_F(HttpNetworkTransactionSpdy3Test, SingleContentDispositionHeader) {
   EXPECT_EQ("Hello", out.response_data);
 }
 
-// Checks that two identical Content-Disposition headers result in an error.
+// Checks that two identical Content-Disposition headers result in no error.
 TEST_F(HttpNetworkTransactionSpdy3Test,
-       DuplicateIdenticalContentDispositionHeaders) {
+       TwoIdenticalContentDispositionHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 200 OK\r\n"),
     MockRead("Content-Disposition: attachment;filename=\"greetings.txt\"r\n"),
@@ -703,12 +703,13 @@ TEST_F(HttpNetworkTransactionSpdy3Test,
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads,
                                               arraysize(data_reads));
-  EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION, out.rv);
+  EXPECT_EQ(OK, out.rv);
+  EXPECT_EQ("HTTP/1.1 200 OK", out.status_line);
+  EXPECT_EQ("Hello", out.response_data);
 }
 
 // Checks that two distinct Content-Disposition headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy3Test,
-       DuplicateDistinctContentDispositionHeaders) {
+TEST_F(HttpNetworkTransactionSpdy3Test, TwoDistinctContentDispositionHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 200 OK\r\n"),
     MockRead("Content-Disposition: attachment;filename=\"greetings.txt\"r\n"),
@@ -721,11 +722,13 @@ TEST_F(HttpNetworkTransactionSpdy3Test,
   EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION, out.rv);
 }
 
-// Checks the behavior of a single Location header.
-TEST_F(HttpNetworkTransactionSpdy3Test, SingleLocationHeader) {
+// Checks that two identical Location headers result in no error.
+// Also tests Location header behavior.
+TEST_F(HttpNetworkTransactionSpdy3Test, TwoIdenticalLocationHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 302 Redirect\r\n"),
     MockRead("Location: http://good.com/\r\n"),
+    MockRead("Location: http://good.com/\r\n"),
     MockRead("Content-Length: 0\r\n\r\n"),
     MockRead(SYNCHRONOUS, OK),
   };
@@ -757,22 +760,8 @@ TEST_F(HttpNetworkTransactionSpdy3Test, SingleLocationHeader) {
   EXPECT_EQ("http://good.com/", url);
 }
 
-// Checks that two identical Location headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy3Test, DuplicateIdenticalLocationHeaders) {
-  MockRead data_reads[] = {
-    MockRead("HTTP/1.1 302 Redirect\r\n"),
-    MockRead("Location: http://good.com/\r\n"),
-    MockRead("Location: http://good.com/\r\n"),
-    MockRead("Content-Length: 0\r\n\r\n"),
-    MockRead(SYNCHRONOUS, OK),
-  };
-  SimpleGetHelperResult out = SimpleGetHelper(data_reads,
-                                              arraysize(data_reads));
-  EXPECT_EQ(ERR_RESPONSE_HEADERS_MULTIPLE_LOCATION, out.rv);
-}
-
 // Checks that two distinct Location headers result in an error.
-TEST_F(HttpNetworkTransactionSpdy3Test, DuplicateDistinctLocationHeaders) {
+TEST_F(HttpNetworkTransactionSpdy3Test, TwoDistinctLocationHeaders) {
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 302 Redirect\r\n"),
     MockRead("Location: http://good.com/\r\n"),
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 6b81a66..8098a18 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -37,13 +37,10 @@ std::string GetResponseHeaderLines(const net::HttpResponseHeaders& headers) {
   return cr_separated_headers;
 }
 
-// Return true if |headers| contain multiple |field_name| fields.  If
-// |count_same_value| is false, returns false if all copies of the field have
-// the same value.
+// Return true if |headers| contain multiple |field_name| fields.
 bool HeadersContainMultipleCopiesOfField(
     const net::HttpResponseHeaders& headers,
-    const std::string& field_name,
-    bool count_same_value) {
+    const std::string& field_name) {
   void* it = NULL;
   std::string field_value;
   if (!headers.EnumerateHeader(&it, field_name, &field_value))
@@ -53,7 +50,7 @@ bool HeadersContainMultipleCopiesOfField(
   // |count_same_value| is true.
   std::string field_value2;
   while (headers.EnumerateHeader(&it, field_name, &field_value2)) {
-    if (count_same_value || field_value != field_value2)
+    if (field_value != field_value2)
       return true;
   }
   return false;
@@ -777,21 +774,15 @@ int HttpStreamParser::DoParseResponseHeaders(int end_offset) {
   // If they exist, and have distinct values, it's a potential response
   // smuggling attack.
   if (!headers->HasHeader("Transfer-Encoding")) {
-    if (HeadersContainMultipleCopiesOfField(*headers,
-                                            "Content-Length",
-                                            false)) {
+    if (HeadersContainMultipleCopiesOfField(*headers, "Content-Length"))
       return ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_LENGTH;
-    }
   }
 
   // Check for multiple Content-Disposition or Location headers.  If they exist,
   // it's also a potential response smuggling attack.
-  if (HeadersContainMultipleCopiesOfField(*headers,
-                                          "Content-Disposition",
-                                          true)) {
+  if (HeadersContainMultipleCopiesOfField(*headers, "Content-Disposition"))
     return ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION;
-  }
-  if (HeadersContainMultipleCopiesOfField(*headers, "Location", true))
+  if (HeadersContainMultipleCopiesOfField(*headers, "Location"))
     return ERR_RESPONSE_HEADERS_MULTIPLE_LOCATION;
 
   response_->headers = headers;