A simple fuzzer for HttpServer, with limited coverage of WebSocket.

net/server/http_server_fuzzer.cc

Index: net/server/http_server_fuzzer.cc
diff --git a/net/server/http_server_fuzzer.cc b/net/server/http_server_fuzzer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..bc07e4a3fc074c492c8ad24f7449021528d5fc70
--- /dev/null
+++ b/net/server/http_server_fuzzer.cc
@@ -0,0 +1,103 @@
+// Copyright 2017 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 "base/logging.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
+#include "base/test/fuzzed_data_provider.h"
+#include "net/base/net_errors.h"
+#include "net/log/test_net_log.h"
+#include "net/server/http_server.h"
+#include "net/socket/fuzzed_server_socket.h"
+
+namespace {
+
+class WaitTillHttpCloseDelegate : public net::HttpServer::Delegate {
+ public:
+  WaitTillHttpCloseDelegate(base::FuzzedDataProvider* data_provider,
+                            const base::Closure& done_closure)
+      : server_(nullptr),
+        data_provider_(data_provider),
+        done_closure_(done_closure),
+        action_flags_(data_provider_->ConsumeUint8()) {}
+
+  void set_server(net::HttpServer* server) { server_ = server; }
+
+  void OnConnect(int connection_id) override {
+    if (!(action_flags_ & ACCEPT_CONNECTION))
+      server_->Close(connection_id);
+  }
+
+  void OnHttpRequest(int connection_id,
+                     const net::HttpServerRequestInfo& info) override {
+    if (!(action_flags_ & ACCEPT_MESSAGE)) {
+      server_->Close(connection_id);
+      return;
+    }
+
+    if (action_flags_ & REPLY_TO_MESSAGE)
+      server_->Send200(connection_id,
+                       data_provider_->ConsumeRandomLengthString(64),
+                       "text/html");

[mmenke, 2017/01/27 17:08:53]

nit:  Use braces when the body of an if takes up multiple lines.

[Maks Orlovich, 2017/01/27 18:17:12]

Done.

+  }
+
+  void OnWebSocketRequest(int connection_id,
+                          const net::HttpServerRequestInfo& info) override {
+    // Randomize whether the delegate accepts web socket or not, to cover both
+    // options.
+    if (action_flags_ & ACCEPT_WEBSOCKET)
+      server_->AcceptWebSocket(connection_id, info);

[mmenke, 2017/01/27 17:08:53]

Should we close the connection if we don't accept this?

I'm not rightly sure what state we're in if we neither accept the websocket
request nor close the connection.  (Same is also true in the HTTP request...if
we get one request, and don't send a response....We just wait for another? 
That's weird).

[morlovich, 2017/01/27 17:30:29]

Well, not doing either is basically the case this addresses:
https://chromium.googlesource.com/chromium/src/net/+/62e66169d64ab6f44118a99f...

... so it definitely feels worth covering. May also be worth testing a coin-flip
of close, of course.

It's also an easy API misuse, or at least I did so myself: it's easy to provide
empty implementations of WebSocket methods when thinking of HTTP only, and
having that not crash based on remote input seems desirable...

[mmenke, 2017/01/27 17:34:59]

Hrm...Good point.  Seems like we're in an indeterminate state if we don't accept
the WebSocket...but yea, test should cover that case, as long as the API allows
it.

[mmenke, 2017/01/27 17:40:13]

Well, "indeterminant" might not be the word - we'd still be reading data, but
would return FRAME_ERROR if any data was actually received...And the consumer
could accept the connection at any time, if it so desired, until then.

+  }
+
+  void OnWebSocketMessage(int connection_id, const std::string& data) override {
+    if (!(action_flags_ & ACCEPT_MESSAGE)) {
+      server_->Close(connection_id);
+      return;
+    }
+
+    if (action_flags_ & REPLY_TO_MESSAGE)
+      server_->SendOverWebSocket(connection_id,
+                                 data_provider_->ConsumeRandomLengthString(64));

[mmenke, 2017/01/27 17:08:53]

nit:  Braces

[Maks Orlovich, 2017/01/27 18:17:11]

Done.

+  }
+
+  void OnClose(int connection_id) override { done_closure_.Run(); }
+
+ private:
+  enum {
+    ACCEPT_CONNECTION = 1,
+    ACCEPT_MESSAGE = 2,
+    REPLY_TO_MESSAGE = 4,
+    ACCEPT_WEBSOCKET = 8
+  };
+
+  net::HttpServer* server_;
+  base::FuzzedDataProvider* const data_provider_;
+  base::Closure done_closure_;
+  const uint8_t action_flags_;
+
+  DISALLOW_COPY_AND_ASSIGN(WaitTillHttpCloseDelegate);
+};
+
+}  // namespace
+
+// Fuzzer for HttpServer
+//
+// |data| is used to create a FuzzedServerSocket.
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+  net::TestNetLog test_net_log;
+  base::FuzzedDataProvider data_provider(data, size);
+
+  std::unique_ptr<net::ServerSocket> server_socket(
+      base::MakeUnique<net::FuzzedServerSocket>(&data_provider, &test_net_log));
+  CHECK_EQ(net::OK,
+           server_socket->ListenWithAddressAndPort("127.0.0.1", 80, 5));

[mmenke, 2017/01/27 17:08:53]

Should just use port 0.  This both gets us on an ephemeral port instead of a
named one, which seems like a good practice, and we have no guarantee that port
80 isn't already in use on the local device.

[morlovich, 2017/01/27 17:30:29]

This isn't actually listening, though (being a FuzzedServerSocket), just
pretends to be for purposes of GetLocalAddress() --- so does this matter?

[mmenke, 2017/01/27 17:34:59]

Oh, right, it doesn't.

+
+  base::RunLoop run_loop;
+  WaitTillHttpCloseDelegate delegate(&data_provider, run_loop.QuitClosure());
+  net::HttpServer server(std::move(server_socket), &delegate);
+  delegate.set_server(&server);
+  run_loop.Run();
+  return 0;
+}