Bound the request head by the read timeout as a whole

The read timeout applied to each read of the request line and headers,
so a client that sent them a byte now and then held a worker for as
long as it liked, and enough such clients left no worker for anyone
else. Bound the whole head by the read timeout, as the TLS handshake
already is, through a new Stream::set_max_timeout() that the socket
streams implement with their existing max timeout.

A request body is still bounded per read, and a WebSocket holds its
worker for its whole life. The README says so, and points to a
reverse proxy and the pre-request handler for untrusted clients.

README-websocket.md

@@ -484,7 +484,7 @@ With the default ping interval of 30 seconds, `max_missed_pongs = 2` detects a d
 
 ## Threading Model
 
-WebSocket connections share the same thread pool as HTTP requests. Each WebSocket connection occupies one thread for its entire lifetime.
+WebSocket connections share the same thread pool as HTTP requests. Each WebSocket connection occupies one thread for its entire lifetime, so a client opening many of them can leave none for HTTP requests. Reject the upgrades you do not want in `set_pre_request_handler()`, before the connection takes a thread for good.
 
 The default thread pool uses dynamic scaling: it maintains a base thread count of `CPPHTTPLIB_THREAD_POOL_COUNT` (8 or `std::thread::hardware_concurrency() - 1`, whichever is greater) and can scale up to 4x that count under load (`CPPHTTPLIB_THREAD_POOL_MAX_COUNT`). When all base threads are busy, temporary threads are spawned automatically up to the maximum. These dynamic threads exit after an idle timeout (`CPPHTTPLIB_THREAD_POOL_IDLE_TIMEOUT`, default 3 seconds).
 

README.md

@@ -898,6 +898,8 @@ svr.set_read_timeout(std::chrono::seconds(5));
 svr.set_keep_alive_timeout(std::chrono::seconds(10));
 ```
 
+The read timeout bounds the request line and headers as a whole, and each read of a request body.
+
 ### Set maximum payload length for reading a request body
 
 ```c++
@@ -917,6 +919,8 @@ Please see [Server example](https://github.com/yhirose/cpp-httplib/blob/master/e
 
 When all threads are busy and a new task arrives, a temporary thread is spawned (up to the maximum). A dynamic thread exits automatically once it has waited for the idle timeout without receiving a task. The idle timeout defaults to 3 seconds, configurable via `CPPHTTPLIB_THREAD_POOL_IDLE_TIMEOUT`.
 
+A connection holds its thread while it is served: between requests up to the keep-alive timeout, through a request body sent slowly but within the read timeout, and for the whole life of a WebSocket. Enough of them leave no thread for anyone else, so in front of untrusted clients, put a reverse proxy that limits connections per client, and reject unwanted WebSocket upgrades in the pre-request handler.
+
 If you want to set the thread counts at runtime:
 
 ```cpp

httplib.h

@@ -1997,6 +1997,10 @@ public:
     (void)usec;
   }
 
+  // Bounds the reads from now on to `msec` in total, on top of the timeout of
+  // each one. 0 lifts the bound.
+  virtual void set_max_timeout(time_t msec) { (void)msec; }
+
   // Bytes already pulled off the socket and sitting in this stream's own
   // buffer. Exposing them lets a line reader scan for a terminator in one
   // pass instead of asking for a byte at a time. A stream that does no
@@ -6739,6 +6743,7 @@ public:
   socket_t socket() const override;
   time_t duration() const override;
   void set_read_timeout(time_t sec, time_t usec = 0) override;
+  void set_max_timeout(time_t msec) override;
   const char *buffered_data(size_t &size) const override;
   void consume_buffered(size_t size) override;
 
@@ -6759,7 +6764,7 @@ private:
   time_t write_timeout_sec_;
   time_t write_timeout_usec_;
   time_t max_timeout_msec_;
-  const std::chrono::time_point<std::chrono::steady_clock> start_time_;
+  std::chrono::time_point<std::chrono::steady_clock> start_time_;
 
   std::vector<char> read_buff_;
   size_t read_buff_off_ = 0;
@@ -10631,6 +10636,7 @@ public:
   socket_t socket() const override;
   time_t duration() const override;
   void set_read_timeout(time_t sec, time_t usec = 0) override;
+  void set_max_timeout(time_t msec) override;
 
   // See SocketStream::set_readable_hint().
   void set_readable_hint() { readable_hint_ = true; }
@@ -10645,7 +10651,7 @@ private:
   time_t write_timeout_sec_;
   time_t write_timeout_usec_;
   time_t max_timeout_msec_;
-  const std::chrono::time_point<std::chrono::steady_clock> start_time_;
+  std::chrono::time_point<std::chrono::steady_clock> start_time_;
   bool readable_hint_ = false;
 };
 
@@ -12594,6 +12600,11 @@ inline void SocketStream::set_read_timeout(time_t sec, time_t usec) {
   read_timeout_usec_ = usec;
 }
 
+inline void SocketStream::set_max_timeout(time_t msec) {
+  max_timeout_msec_ = msec;
+  start_time_ = std::chrono::steady_clock::now();
+}
+
 // Buffer stream implementation
 inline bool BufferStream::is_readable() const { return true; }
 
@@ -13126,6 +13137,11 @@ inline void SSLSocketStream::set_read_timeout(time_t sec, time_t usec) {
   read_timeout_usec_ = usec;
 }
 
+inline void SSLSocketStream::set_max_timeout(time_t msec) {
+  max_timeout_msec_ = msec;
+  start_time_ = std::chrono::steady_clock::now();
+}
+
 inline WebSocketSSLStream::WebSocketSSLStream(socket_t sock,
                                               tls::session_t session,
                                               time_t read_timeout_sec,
@@ -14777,6 +14793,10 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
 
   detail::stream_line_reader line_reader(strm, buf.data(), buf.size());
 
+  // The read timeout bounds the request line and headers as a whole, so a
+  // client sending them a byte at a time cannot hold the worker any longer.
+  strm.set_max_timeout(read_timeout_sec_ * 1000 + read_timeout_usec_ / 1000);
+
   // Connection has been closed on client
   if (!line_reader.getline()) { return false; }
 
@@ -14825,6 +14845,7 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
     output_error_log(Error::InvalidHeaders, &req);
     return write_response(strm, close_connection, req, res);
   }
+  strm.set_max_timeout(0);
 
   // RFC 9112 §6.3: Reject requests whose framing is invalid or ambiguous,
   // which would otherwise let an intermediary and this parser disagree on

test/test.cc

@@ -10457,6 +10457,105 @@ TEST(SSLClientServerTest, ReadTimeoutBoundsTheWholeHandshake) {
 }
 #endif
 
+#ifdef CPPHTTPLIB_OPENSSL_SUPPORT
+// send_request_in_parts() over TLS, each part in a TLS record of its own.
+static bool send_ssl_request_in_parts(time_t read_timeout_sec,
+                                      const std::vector<std::string> &parts,
+                                      std::string *resp = nullptr,
+                                      int port = PORT) {
+  auto error = Error::Success;
+  auto sock = detail::create_client_socket(
+      HOST, "", port, AF_UNSPEC, false, false, nullptr,
+      /*connection_timeout_sec=*/5, 0, read_timeout_sec, 0,
+      /*write_timeout_sec=*/5, 0, std::string(), error);
+  if (sock == INVALID_SOCKET) { return false; }
+  auto sock_se = detail::scope_exit([&] { detail::close_socket(sock); });
+
+  auto ctx = SSL_CTX_new(TLS_client_method());
+  auto ssl = SSL_new(ctx);
+  auto ssl_se = detail::scope_exit([&] {
+    SSL_free(ssl);
+    SSL_CTX_free(ctx);
+  });
+  SSL_set_fd(ssl, static_cast<int>(sock));
+  if (SSL_connect(ssl) != 1) { return false; }
+
+  for (size_t i = 0; i < parts.size(); i++) {
+    const auto &part = parts[i];
+    if (SSL_write(ssl, part.data(), static_cast<int>(part.size())) !=
+        static_cast<int>(part.size())) {
+      return false;
+    }
+    if (i + 1 < parts.size()) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(100));
+    }
+  }
+
+  char buf[512];
+  int n;
+  while ((n = SSL_read(ssl, buf, sizeof(buf))) > 0) {
+    if (resp) { resp->append(buf, static_cast<size_t>(n)); }
+  }
+  return true;
+}
+#endif
+
+// The request line, then a header value a byte at a time.
+static std::vector<std::string> slow_request_head_parts() {
+  std::vector<std::string> parts{"GET / HTTP/1.1\r\nHost: x\r\nX-Slow: "};
+  parts.resize(26, "a");
+  parts.emplace_back("\r\n\r\n");
+  return parts;
+}
+
+// `send` takes 2.5 seconds over slow_request_head_parts(), each wait well
+// within the one-second read timeout. The server gives up a second in, before
+// the head is complete.
+static void
+expect_slow_request_head_dropped(Server &svr,
+                                 const std::function<std::string()> &send) {
+  std::atomic<bool> handled(false);
+  svr.Get("/", [&](const Request &, Response &res) {
+    handled = true;
+    res.set_content("ok", "text/plain");
+  });
+  svr.set_read_timeout(1, 0);
+
+  thread t = thread([&]() { ASSERT_TRUE(svr.listen(HOST, PORT)); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  auto resp = send();
+  EXPECT_FALSE(handled);
+  EXPECT_EQ(std::string::npos, resp.find("200 OK"));
+}
+
+TEST(ServerRequestParsingTest, ReadTimeoutBoundsTheWholeRequestHead) {
+  Server svr;
+  expect_slow_request_head_dropped(svr, [] {
+    std::string resp;
+    send_request_in_parts(5, slow_request_head_parts(), &resp);
+    return resp;
+  });
+}
+
+#ifdef CPPHTTPLIB_OPENSSL_SUPPORT
+TEST(SSLClientServerTest, ReadTimeoutBoundsTheWholeRequestHead) {
+  SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE);
+  ASSERT_TRUE(svr.is_valid());
+  expect_slow_request_head_dropped(svr, [] {
+    std::string resp;
+    send_ssl_request_in_parts(5, slow_request_head_parts(), &resp);
+    return resp;
+  });
+}
+#endif
+
 TEST(ServerRequestParsingTest, TrimWhitespaceFromHeaderValues) {
   Server svr;
   std::string header_value;
@@ -12111,37 +12210,13 @@ TEST(KeepAliveTest, SSLPipelinedRequests) {
   SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE);
   std::string res;
   serve_pipelining_routes(svr, [&](int port) {
-    auto error = Error::Success;
-    auto sock = detail::create_client_socket(
-        HOST, "", port, AF_UNSPEC, false, false, nullptr,
-        /*connection_timeout_sec=*/5, 0,
-        /*read_timeout_sec=*/2, 0,
-        /*write_timeout_sec=*/5, 0, std::string(), error);
-    ASSERT_NE(INVALID_SOCKET, sock);
-    auto sock_se = detail::scope_exit([&] { detail::close_socket(sock); });
-
-    auto ctx = SSL_CTX_new(TLS_client_method());
-    auto ssl = SSL_new(ctx);
-    auto ssl_se = detail::scope_exit([&] {
-      SSL_free(ssl);
-      SSL_CTX_free(ctx);
-    });
-    SSL_set_fd(ssl, static_cast<int>(sock));
-    ASSERT_EQ(1, SSL_connect(ssl));
-
-    // One write, so all three requests arrive in one TLS record
-    const std::string req =
-        "GET /hi/1 HTTP/1.1\r\nHost: localhost\r\n\r\n"
-        "GET /hi/2 HTTP/1.1\r\nHost: localhost\r\n\r\n"
-        "GET /hi/3 HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n";
-    ASSERT_EQ(static_cast<int>(req.size()),
-              SSL_write(ssl, req.data(), static_cast<int>(req.size())));
-
-    char buf[512];
-    int n;
-    while ((n = SSL_read(ssl, buf, sizeof(buf))) > 0) {
-      res.append(buf, static_cast<size_t>(n));
-    }
+    // One part, so all three requests arrive in one TLS record
+    EXPECT_TRUE(send_ssl_request_in_parts(
+        2,
+        {"GET /hi/1 HTTP/1.1\r\nHost: localhost\r\n\r\n"
+         "GET /hi/2 HTTP/1.1\r\nHost: localhost\r\n\r\n"
+         "GET /hi/3 HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n"},
+        &res, port));
   });
   expect_in_order(res, {"hi 1", "hi 2", "hi 3"});
 }