net: don't process truncated headers.

net/http/http_stream_parser.cc

Index: net/http/http_stream_parser.cc
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 3c64ee67bf636834e857495bc7a215d8483285d4..7b195e7f698f11abb120c7c93642bed5871935cb 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")) {

[Ryan Sleevi, 2013/05/29 19:59:33]

Seems like we should be looking at SchemeIsSecure - that is, also counting in
wss:// (can't they set cookies during their initial Upgrade negotiation?)

I'm curious if we attempt a wss:// request and the server doesn't support it, if
it's possible to fall back to an HTTP response (with a Set-Cookie), thus
allowing this to work again.

[wtc, 2013/05/30 00:38:30]

Yes, this sounds like a good idea. In fact, I am wondering if we
should also do this for HTTP.

Nit: omit "else" after a return statement.

[agl, 2013/06/03 18:18:29]

After discussions with Darin, I only did this for HTTPS in order to limit compat
concerns.

r.e. SchemeIsSecure - should the other uses of SchemeIs("https") in this file be
changed too? (I'm guessing yes, but wanted to check.)

[wtc, 2013/06/03 18:36:47]

It occurred to me that this file, http_stream_parser.cc, is probably only used
for the "http" and "https" schemes. If so, then SchemeIs("https") vs.
SchemeIsSecure()
is just a cosmetic issue.

SchemeIsSecure is defined as:

  // If the scheme indicates a secure connection
  bool SchemeIsSecure() const {
    return SchemeIs("https") || SchemeIs("wss") ||
        (SchemeIsFileSystem() && inner_url() && inner_url()->SchemeIsSecure());
  }

So there is also a SchemeIsFileSystem() test, which I don't fully understand.

Since this file isn't using SchemeIsSecure, I now think it is fine to just
use SchemeIs("https") here.

+      // 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

[wtc, 2013/05/30 00:38:30]

By "a different error code", do you mean an error code different
from ERR_EMPTY_RESPONSE?

[agl, 2013/06/03 18:18:29]

Yes, will clarify.

+      // 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);