net: don't process truncated headers on HTTPS connections.

This change causes us to not process any headers unless they are correctly
terminated with a \r\n\r\n sequence.

BUG=244260

Review URL: https://chromiumcodereview.appspot.com/15688012

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

4 files changed, 139 insertions, 15 deletions

diff --git a/net/http/http_proxy_client_socket_pool_spdy2_unittest.cc b/net/http/http_proxy_client_socket_pool_spdy2_unittest.cc
index 0d32634..50b6bf6 100644
--- a/net/http/http_proxy_client_socket_pool_spdy2_unittest.cc
+++ b/net/http/http_proxy_client_socket_pool_spdy2_unittest.cc
@@ -478,7 +478,13 @@ TEST_P(HttpProxyClientSocketPoolSpdy2Test, TunnelUnexpectedClose) {
   EXPECT_FALSE(handle_.socket());
 
   data_->RunFor(3);
-  EXPECT_EQ(ERR_CONNECTION_CLOSED, callback_.WaitForResult());
+  if (GetParam() == SPDY) {
+    // SPDY cannot process a headers block unless it's complete and so it
+    // returns ERR_CONNECTION_CLOSED in this case.
+    EXPECT_EQ(ERR_CONNECTION_CLOSED, callback_.WaitForResult());
+  } else {
+    EXPECT_EQ(ERR_HEADERS_TRUNCATED, callback_.WaitForResult());
+  }
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 }
diff --git a/net/http/http_proxy_client_socket_pool_spdy3_unittest.cc b/net/http/http_proxy_client_socket_pool_spdy3_unittest.cc
index a0edc67..a612b9d 100644
--- a/net/http/http_proxy_client_socket_pool_spdy3_unittest.cc
+++ b/net/http/http_proxy_client_socket_pool_spdy3_unittest.cc
@@ -479,7 +479,13 @@ TEST_P(HttpProxyClientSocketPoolSpdy3Test, TunnelUnexpectedClose) {
   EXPECT_FALSE(handle_.socket());
 
   data_->RunFor(3);
-  EXPECT_EQ(ERR_CONNECTION_CLOSED, callback_.WaitForResult());
+  if (GetParam() == SPDY) {
+    // SPDY cannot process a headers block unless it's complete and so it
+    // returns ERR_CONNECTION_CLOSED in this case.
+    EXPECT_EQ(ERR_CONNECTION_CLOSED, callback_.WaitForResult());
+  } else {
+    EXPECT_EQ(ERR_HEADERS_TRUNCATED, callback_.WaitForResult());
+  }
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 }
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 3c64ee6..7b195e7 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -541,26 +541,31 @@ int HttpStreamParser::DoReadHeadersComplete(int result) {
 
   if (result == ERR_CONNECTION_CLOSED) {
     // The connection closed before we detected the end of the headers.
-    // parse things as well as we can and let the caller decide what to do.
     if (read_buf_->offset() == 0) {
       // The connection was closed before any data was sent. Likely an error
       // rather than empty HTTP/0.9 response.
       io_state_ = STATE_DONE;
       return ERR_EMPTY_RESPONSE;
+    } else if (request_->url.SchemeIs("https")) {
+      // The connection was closed in the middle of the headers. For HTTPS we
+      // don't parse partial headers. Return a different error code so that we
+      // know that we shouldn't attempt to retry the request.
+      io_state_ = STATE_DONE;
+      return ERR_HEADERS_TRUNCATED;
+    }
+    // Parse things as well as we can and let the caller decide what to do.
+    int end_offset;
+    if (response_header_start_offset_ >= 0) {
+      io_state_ = STATE_READ_BODY_COMPLETE;
+      end_offset = read_buf_->offset();
     } else {
-      int end_offset;
-      if (response_header_start_offset_ >= 0) {
-        io_state_ = STATE_READ_BODY_COMPLETE;
-        end_offset = read_buf_->offset();
-      } else {
-        io_state_ = STATE_BODY_PENDING;
-        end_offset = 0;
-      }
-      int rv = DoParseResponseHeaders(end_offset);
-      if (rv < 0)
-        return rv;
-      return result;
+      io_state_ = STATE_BODY_PENDING;
+      end_offset = 0;
     }
+    int rv = DoParseResponseHeaders(end_offset);
+    if (rv < 0)
+      return rv;
+    return result;
   }
 
   read_buf_->set_offset(read_buf_->offset() + result);
diff --git a/net/http/http_stream_parser_unittest.cc b/net/http/http_stream_parser_unittest.cc
index 323e3ae..9637bad 100644
--- a/net/http/http_stream_parser_unittest.cc
+++ b/net/http/http_stream_parser_unittest.cc
@@ -301,4 +301,111 @@ TEST(HttpStreamParser, AsyncChunkAndAsyncSocket) {
   ASSERT_EQ(kBodySize, rv);
 }
 
+TEST(HttpStreamParser, TruncatedHeaders) {
+  MockRead truncated_status_reads[] = {
+    MockRead(SYNCHRONOUS, 1, "HTTP/1.1 20"),
+    MockRead(SYNCHRONOUS, 0, 2),  // EOF
+  };
+
+  MockRead truncated_after_status_reads[] = {
+    MockRead(SYNCHRONOUS, 1, "HTTP/1.1 200 Ok\r\n"),
+    MockRead(SYNCHRONOUS, 0, 2),  // EOF
+  };
+
+  MockRead truncated_in_header_reads[] = {
+    MockRead(SYNCHRONOUS, 1, "HTTP/1.1 200 Ok\r\nHead"),
+    MockRead(SYNCHRONOUS, 0, 2),  // EOF
+  };
+
+  MockRead truncated_after_header_reads[] = {
+    MockRead(SYNCHRONOUS, 1, "HTTP/1.1 200 Ok\r\nHeader: foo\r\n"),
+    MockRead(SYNCHRONOUS, 0, 2),  // EOF
+  };
+
+  MockRead truncated_after_final_newline_reads[] = {
+    MockRead(SYNCHRONOUS, 1, "HTTP/1.1 200 Ok\r\nHeader: foo\r\n\r"),
+    MockRead(SYNCHRONOUS, 0, 2),  // EOF
+  };
+
+  MockRead not_truncated_reads[] = {
+    MockRead(SYNCHRONOUS, 1, "HTTP/1.1 200 Ok\r\nHeader: foo\r\n\r\n"),
+    MockRead(SYNCHRONOUS, 0, 2),  // EOF
+  };
+
+  MockRead* reads[] = {
+    truncated_status_reads,
+    truncated_after_status_reads,
+    truncated_in_header_reads,
+    truncated_after_header_reads,
+    truncated_after_final_newline_reads,
+    not_truncated_reads,
+  };
+
+  MockWrite writes[] = {
+    MockWrite(SYNCHRONOUS, 0, "GET / HTTP/1.1\r\n\r\n"),
+  };
+
+  enum {
+    HTTP = 0,
+    HTTPS,
+    NUM_PROTOCOLS,
+  };
+
+  for (size_t protocol = 0; protocol < NUM_PROTOCOLS; protocol++) {
+    SCOPED_TRACE(protocol);
+
+    for (size_t i = 0; i < arraysize(reads); i++) {
+      SCOPED_TRACE(i);
+      DeterministicSocketData data(reads[i], 2, writes, arraysize(writes));
+      data.set_connect_data(MockConnect(SYNCHRONOUS, OK));
+      data.SetStop(3);
+
+      scoped_ptr<DeterministicMockTCPClientSocket> transport(
+          new DeterministicMockTCPClientSocket(NULL, &data));
+      data.set_delegate(transport->AsWeakPtr());
+
+      TestCompletionCallback callback;
+      int rv = transport->Connect(callback.callback());
+      rv = callback.GetResult(rv);
+      ASSERT_EQ(OK, rv);
+
+      scoped_ptr<ClientSocketHandle> socket_handle(new ClientSocketHandle);
+      socket_handle->set_socket(transport.release());
+
+      HttpRequestInfo request_info;
+      request_info.method = "GET";
+      if (protocol == HTTP) {
+        request_info.url = GURL("http://localhost");
+      } else {
+        request_info.url = GURL("https://localhost");
+      }
+      request_info.load_flags = LOAD_NORMAL;
+
+      scoped_refptr<GrowableIOBuffer> read_buffer(new GrowableIOBuffer);
+      HttpStreamParser parser(socket_handle.get(), &request_info, read_buffer,
+                              BoundNetLog());
+
+      HttpRequestHeaders request_headers;
+      HttpResponseInfo response_info;
+      rv = parser.SendRequest("GET / HTTP/1.1\r\n", request_headers,
+                              &response_info, callback.callback());
+      ASSERT_EQ(OK, rv);
+
+      rv = parser.ReadResponseHeaders(callback.callback());
+      if (i == arraysize(reads) - 1) {
+        EXPECT_EQ(OK, rv);
+        EXPECT_TRUE(response_info.headers.get());
+      } else {
+        if (protocol == HTTP) {
+          EXPECT_EQ(ERR_CONNECTION_CLOSED, rv);
+          EXPECT_TRUE(response_info.headers.get());
+        } else {
+          EXPECT_EQ(ERR_HEADERS_TRUNCATED, rv);
+          EXPECT_FALSE(response_info.headers.get());
+        }
+      }
+    }
+  }
+}
+
 }  // namespace net