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

net/server/http_server_unittest.cc

 // Copyright 2013 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 <stdint.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 114 matching lines @@
       return false;
     while (!IsCompleteResponse(*message)) {
       std::string chunk;
       if (!Read(&chunk, 1))
         return false;
       message->append(chunk);
     }
     return true;
   }
 
+  TCPClientSocket& socket() { return *socket_; }
+
  private:
   void OnConnect(const base::Closure& quit_loop, int result) {
     connect_result_ = result;
     quit_loop.Run();
   }
 
   void Write() {
     int result = socket_->Write(
         write_buffer_.get(),
         write_buffer_->BytesRemaining(),
@@ skipping 46 matching lines @@
   HttpServerTest() : quit_after_request_count_(0) {}
 
   void SetUp() override {
     std::unique_ptr<ServerSocket> server_socket(
         new TCPServerSocket(NULL, NetLog::Source()));
     server_socket->ListenWithAddressAndPort("127.0.0.1", 0, 1);
     server_.reset(new HttpServer(std::move(server_socket), this));
     ASSERT_THAT(server_->GetLocalAddress(&server_address_), IsOk());
   }
 
-  void OnConnect(int connection_id) override {}
+  void OnConnect(int connection_id) override {
+    DCHECK(connection_map_.find(connection_id) == connection_map_.end());
+    connection_map_[connection_id] = true;
+  }
 
   void OnHttpRequest(int connection_id,
                      const HttpServerRequestInfo& info) override {
     requests_.push_back(std::make_pair(info, connection_id));
     if (requests_.size() == quit_after_request_count_)
       run_loop_quit_func_.Run();
   }
 
   void OnWebSocketRequest(int connection_id,
                           const HttpServerRequestInfo& info) override {
     NOTREACHED();
   }
 
   void OnWebSocketMessage(int connection_id, const std::string& data) override {
     NOTREACHED();
   }
 
-  void OnClose(int connection_id) override {}
+  void OnClose(int connection_id) override {
+    DCHECK(connection_map_.find(connection_id) != connection_map_.end());
+    connection_map_[connection_id] = false;
+  }
 
   bool RunUntilRequestsReceived(size_t count) {
     quit_after_request_count_ = count;
     if (requests_.size() == count)
       return true;
 
     base::RunLoop run_loop;
     run_loop_quit_func_ = run_loop.QuitClosure();
     bool success = RunLoopWithTimeout(&run_loop);
     run_loop_quit_func_.Reset();
     return success;
   }
 
   HttpServerRequestInfo GetRequest(size_t request_index) {
     return requests_[request_index].first;
   }
 
   int GetConnectionId(size_t request_index) {
     return requests_[request_index].second;
   }
 
   void HandleAcceptResult(std::unique_ptr<StreamSocket> socket) {
     server_->accepted_socket_.reset(socket.release());
     server_->HandleAcceptResult(OK);
   }
 
+  std::unordered_map<int, bool>& connection_map() { return connection_map_; }
+
  protected:
   std::unique_ptr<HttpServer> server_;
   IPEndPoint server_address_;
   base::Closure run_loop_quit_func_;
   std::vector<std::pair<HttpServerRequestInfo, int> > requests_;
+  std::unordered_map<int /* connection_id */, bool /* connected */>
+      connection_map_;
 
  private:
   size_t quit_after_request_count_;
 };
 
 namespace {
 
 class WebSocketTest : public HttpServerTest {
   void OnHttpRequest(int connection_id,
                      const HttpServerRequestInfo& info) override {
@@ skipping 204 matching lines @@
   server_->SendRaw(GetConnectionId(0), "Raw Data ");
   server_->SendRaw(GetConnectionId(0), "More Data");
   server_->SendRaw(GetConnectionId(0), "Third Piece of Data");
 
   const std::string expected_response("Raw Data More DataThird Piece of Data");
   std::string response;
   ASSERT_TRUE(client.Read(&response, expected_response.length()));
   ASSERT_EQ(expected_response, response);
 }
 
+TEST_F(HttpServerTest, WrongProtocolRequest) {
+  const char* const kBadProtocolRequests[] = {
+      "GET /test HTTP/1.0\r\n\r\n",
+      "GET /test foo\r\n\r\n",
+      "GET /test \r\n\r\n",
+  };
+
+  for (size_t i = 0; i < arraysize(kBadProtocolRequests); ++i) {
+    TestHttpClient client;
+    ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
+
+    client.Send(kBadProtocolRequests[i]);
+    ASSERT_FALSE(RunUntilRequestsReceived(1));
+
+    // Assert that the delegate was updated properly.
+    ASSERT_EQ(1u, connection_map().size());
+    ASSERT_FALSE(connection_map().begin()->second);
+
+    // Assert that the socket was opened then closed.
+    ASSERT_TRUE(client.socket().WasEverUsed());
+    ASSERT_FALSE(client.socket().IsConnected());

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

This is potentially racy - the server could exit the message loop before the
client socket sees that it's closed.

Think can just do this to fix that:

std::string response;
EXPECT_FALSE(client.socket().Read(response));
EXPECT_EQ("", response);

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

Done.

+
+    // Reset the state of the connection map.
+    connection_map().clear();
+  }
+}
+
 class MockStreamSocket : public StreamSocket {
  public:
   MockStreamSocket()
       : connected_(true),
         read_buf_(NULL),
         read_buf_len_(0) {}
 
   // StreamSocket
   int Connect(const CompletionCallback& callback) override {
     return ERR_NOT_IMPLEMENTED;
@@ skipping 148 matching lines @@
   ASSERT_TRUE(client.ReadResponse(&response3));
   ASSERT_TRUE(base::StartsWith(response3, "HTTP/1.1 200 OK",
                                base::CompareCase::SENSITIVE));
   ASSERT_TRUE(base::EndsWith(response3, "Content for /test3",
                              base::CompareCase::SENSITIVE));
 }
 
 class CloseOnConnectHttpServerTest : public HttpServerTest {
  public:
   void OnConnect(int connection_id) override {
+    HttpServerTest::OnConnect(connection_id);
     connection_ids_.push_back(connection_id);
     server_->Close(connection_id);
   }
 
  protected:
   std::vector<int> connection_ids_;
 };
 
 TEST_F(CloseOnConnectHttpServerTest, ServerImmediatelyClosesConnection) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send("GET / HTTP/1.1\r\n\r\n");
   ASSERT_FALSE(RunUntilRequestsReceived(1));
   ASSERT_EQ(1ul, connection_ids_.size());
   ASSERT_EQ(0ul, requests_.size());
 }
 
 }  // namespace
 
 }  // namespace net