Handle non-HTTP/1.1 requests more gracefully in net::HttpServer.

net/server/http_server.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/server/http_server.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 214 matching lines @@
       delegate_->OnWebSocketMessage(connection->id(), message);
       if (HasClosedConnection(connection))
         return ERR_CONNECTION_CLOSED;
       continue;
     }
 
     HttpServerRequestInfo request;
     size_t pos = 0;
     if (!ParseHeaders(read_buf->StartOfBuffer(), read_buf->GetSize(),
                       &request, &pos)) {
+      // An error has occured. Close the connection.
+      Close(connection->id());
+      return ERR_CONNECTION_CLOSED;
+    } else if (!pos) {
+      // If pos is 0, we read all the data in read_buf, but haven't yet finished

[mmenke, 2016/09/15 17:40:12]

nit:  avoid "we" in comments, due to ambiguity ("We the object", "We the
developers", "We the user", "We the television network", etc).  Prefer passive
voice.

[slan, 2016/09/16 22:35:30]

Done.

+      // parsing the headers. Continue parsing when more data rolls in.
       break;
     }
 
     // Sets peer address if exists.
     connection->socket()->GetPeerAddress(&request.peer);
 
     if (request.HasHeaderValue("connection", "upgrade")) {
       connection->SetWebSocket(base::MakeUnique<WebSocket>(this, connection));
       read_buf->DidConsume(pos);
       delegate_->OnWebSocketRequest(connection->id(), request);
@@ skipping 153 matching lines @@
       switch (state) {
         case ST_METHOD:
           info->method = buffer;
           buffer.clear();
           break;
         case ST_URL:
           info->path = buffer;
           buffer.clear();
           break;
         case ST_PROTO:
-          // TODO(mbelshe): Deal better with parsing protocol.
-          DCHECK(buffer == "HTTP/1.1");
+          if (buffer != "HTTP/1.1") {
+            LOG(ERROR) << "Cannot handle request with protocol: " << buffer;
+            next_state = ST_ERR;
+          }
           buffer.clear();
           break;
         case ST_NAME:
           header_name = base::ToLowerASCII(buffer);
           buffer.clear();
           break;
         case ST_VALUE:
           base::TrimWhitespaceASCII(buffer, base::TRIM_LEADING, &header_value);
           it = info->headers.find(header_name);
           // See the second paragraph ("A sender MUST NOT generate multiple
@@ skipping 21 matching lines @@
           buffer.append(&ch, 1);
           break;
         case ST_DONE:
           DCHECK(input == INPUT_LF);
           return true;
         case ST_ERR:
           return false;
       }
     }
   }
-  // No more characters, but we haven't finished parsing yet.
-  return false;
+  // No more characters, but we haven't finished parsing yet. Signal this to
+  // the caller by setting |pos| to zero.
+  pos = 0;
+  return true;
 }
 
 HttpConnection* HttpServer::FindConnection(int connection_id) {
   IdToConnectionMap::iterator it = id_to_connection_.find(connection_id);
   if (it == id_to_connection_.end())
     return NULL;
   return it->second;
 }
 
 // This is called after any delegate callbacks are called to check if Close()
 // has been called during callback processing. Using the pointer of connection,
 // |connection| is safe here because Close() deletes the connection in next run
 // loop.
 bool HttpServer::HasClosedConnection(HttpConnection* connection) {
   return FindConnection(connection->id()) != connection;
 }
 
 }  // namespace net