Kaynağa Gözat

Send 100 Continue only when the request body is read

The server wrote 100 Continue as soon as it saw the expectation, before
pre_routing_handler, pre_request_handler, or routing ran. A request
those handlers rejected, or one that matched no route, still invited
the client to send a body the server would never read.

Defer the interim response until the body is about to be read. If the
request is answered without reading the body, 100 Continue is never
sent and the connection is closed, since whether and when the client
sends the body is unknown.

Also treat a 417 returned by expect_100_continue_handler as the final
response. It used to be written as a bare status line, after which the
request was processed and a second response was written.
yhirose 2 hafta önce
ebeveyn
işleme
09c02f1335
3 değiştirilmiş dosya ile 186 ekleme ve 11 silme
  1. 8 3
      README.md
  2. 20 8
      httplib.h
  3. 158 0
      test/test.cc

+ 8 - 3
README.md

@@ -564,14 +564,15 @@ svr.set_pre_request_handler([](const auto& req, auto& res) {
 
 ```
 Request received
+  │
+  ├─ expect_100_continue_handler  (when the request has "Expect: 100-continue")
+  │     └─ returns a status other than 100 → stop here
   │
   ├─ pre_routing_handler          route not matched yet, body not read
   │     └─ returns Handled → stop here
   │
   ├─ file_request_handler         (GET/HEAD, static file serving)
   │
-  ├─ expect_100_continue_handler  (when the request has "Expect: 100-continue")
-  │
   ├─ route matching → req.matched_route is set
   │
   ├─ pre_request_handler          route matched, body NOT read yet
@@ -587,6 +588,8 @@ Request received
 
 Use `pre_routing_handler` to reject a request as early as possible, before the route is known. Use `pre_request_handler` for route-specific checks, since `req.matched_route` is available and the body has not been read yet.
 
+For a request with `Expect: 100-continue`, the `100 Continue` response is not sent until the body is about to be read. A request rejected before that point (by `pre_routing_handler`, `pre_request_handler`, or because no route matched) gets its final response without `100 Continue`, so the client never sends the body.
+
 A WebSocket upgrade request that matches a route registered with `svr.WebSocket()` takes a shorter path: `pre_routing_handler`, then route matching (`req.matched_route` is set), then `pre_request_handler`, then the WebSocket handler. If either hook returns `Handled`, its response is sent as a regular HTTP response and the connection is not upgraded. Once the connection is upgraded, `post_routing_handler` does not run.
 
 ### Response user data
@@ -848,7 +851,9 @@ svr.Get("/content", [&](const Request &req, Response &res) {
 
 ### 'Expect: 100-continue' handler
 
-By default, the server sends a `100 Continue` response for an `Expect: 100-continue` header.
+By default, the server accepts an `Expect: 100-continue` header and sends a `100 Continue` response when it starts reading the request body. If the request is answered without reading the body, `100 Continue` is not sent and the connection is closed after the response.
+
+The handler runs before `pre_routing_handler`. Returning `100` lets the request proceed; returning any other status sends that status as the final response and closes the connection.
 
 ```cpp
 // Send a '417 Expectation Failed' response.

+ 20 - 8
httplib.h

@@ -1756,6 +1756,7 @@ struct Request {
 
   // private members...
   bool body_consumed_ = false;
+  bool expect_100_continue_pending_ = false;
   size_t redirect_count_ = CPPHTTPLIB_REDIRECT_MAX_COUNT;
   size_t content_length_ = 0;
   ContentProvider content_provider_;
@@ -13298,7 +13299,10 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
   // Prepare additional headers
   if (close_connection ||
       detail::has_header_token(req.headers, "Connection", "close") ||
-      400 <= res.status) { // Don't leave connections open after errors
+      400 <= res.status || // Don't leave connections open after errors
+      // The client withholds the body until `100 Continue`, which was never
+      // sent, so whether and when the body follows is unknown.
+      (req.expect_100_continue_pending_ && detail::has_framed_body(req))) {
     res.set_header("Connection", "close");
   } else {
     std::string s = "timeout=";
@@ -13549,6 +13553,13 @@ inline bool Server::read_content_core(
   }
 #endif
 
+  // The client is waiting for this before it sends the body.
+  if (req.expect_100_continue_pending_) {
+    req.expect_100_continue_pending_ = false;
+    detail::write_response_line(strm, StatusCode::Continue_100);
+    strm.write("\r\n");
+  }
+
   if (!detail::read_content(strm, req, payload_max_length_, res.status, nullptr,
                             out, true)) {
     return false;
@@ -14386,19 +14397,20 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
   // case-insensitive, and a 100-continue expectation in an HTTP/1.0 request
   // must be ignored. An expectation we do not recognize is left alone; the
   // 417 the section allows for one is a MAY, not a requirement.
+  //
+  // `100 Continue` itself is deferred until the body is actually read (see
+  // read_content_core), so a request rejected by a later handler never
+  // invites the client to send a body nobody will read.
   if (req.version != "HTTP/1.0" &&
       detail::has_header_token(req.headers, "Expect", "100-continue")) {
     int status = StatusCode::Continue_100;
     if (expect_100_continue_handler_) {
       status = expect_100_continue_handler_(req, res);
     }
-    switch (status) {
-    case StatusCode::Continue_100:
-    case StatusCode::ExpectationFailed_417:
-      detail::write_response_line(strm, status);
-      strm.write("\r\n");
-      break;
-    default:
+    if (status == StatusCode::Continue_100) {
+      req.expect_100_continue_pending_ = true;
+    } else {
+      if (res.status == -1) { res.status = status; }
       connection_closed = true;
       return write_response(strm, true, req, res);
     }

+ 158 - 0
test/test.cc

@@ -18057,6 +18057,164 @@ TEST_F(ExpectTokenTest, ExpectationAmongOthersIsRecognized) {
   EXPECT_TRUE(got_100);
 }
 
+// `100 Continue` is sent only when the server starts reading the body, so a
+// request rejected before that never invites the client to send it. The
+// requests below carry the expectation but withhold the body, as a client
+// waiting for `100 Continue` would.
+// A POST that expects `100 Continue` and withholds its two-byte body.
+static std::string expect_headers_only(const std::string &path) {
+  return "POST " + path +
+         " HTTP/1.1\r\n"
+         "Host: localhost\r\n"
+         "Content-Length: 2\r\n"
+         "Expect: 100-continue\r\n"
+         "\r\n";
+}
+
+class ExpectLazyContinueTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+    svr_.set_pre_routing_handler([](const Request &req, Response &res) {
+      if (req.path == "/pre-routing") {
+        res.status = StatusCode::Unauthorized_401;
+        return Server::HandlerResponse::Handled;
+      }
+      return Server::HandlerResponse::Unhandled;
+    });
+    svr_.set_pre_request_handler([](const Request &req, Response &res) {
+      if (req.matched_route == "/pre-request") {
+        res.status = StatusCode::Forbidden_403;
+        return Server::HandlerResponse::Handled;
+      }
+      return Server::HandlerResponse::Unhandled;
+    });
+    svr_.Post("/pre-routing", [](const Request &, Response &res) {
+      res.set_content("ok", "text/plain");
+    });
+    svr_.Post("/pre-request", [](const Request &, Response &res) {
+      res.set_content("ok", "text/plain");
+    });
+    svr_.Post("/reader-used", [](const Request &, Response &res,
+                                 const ContentReader &content_reader) {
+      std::string body;
+      content_reader([&](const char *data, size_t len) {
+        body.append(data, len);
+        return true;
+      });
+      res.set_content(body, "text/plain");
+    });
+    svr_.Post("/reader-unused",
+              [](const Request &, Response &res, const ContentReader &) {
+                res.set_content("ignored", "text/plain");
+              });
+    port_ = svr_.bind_to_any_port(HOST);
+    thread_ = thread([&]() { svr_.listen_after_bind(); });
+    svr_.wait_until_ready();
+  }
+
+  void TearDown() override {
+    svr_.stop();
+    if (thread_.joinable()) { thread_.join(); }
+  }
+
+  // Sends `req` and reads until the server closes the connection. Returns
+  // false if the read had to wait for the client-side timeout instead.
+  bool send_until_closed(const std::string &req, std::string *resp) const {
+    auto start = std::chrono::steady_clock::now();
+    if (!send_request(3, req, resp, port_)) { return false; }
+    auto elapsed = std::chrono::steady_clock::now() - start;
+    return elapsed < std::chrono::seconds(2);
+  }
+
+  // The final response comes without `100 Continue`, and the server closes
+  // the connection since the body may or may not follow.
+  void expect_final_without_interim(const std::string &path,
+                                    const char *status_line) const {
+    std::string resp;
+    ASSERT_TRUE(send_until_closed(expect_headers_only(path), &resp));
+    EXPECT_EQ(std::string::npos, resp.find("100 Continue"));
+    EXPECT_EQ(0u, resp.find(status_line));
+    EXPECT_NE(std::string::npos, resp.find("Connection: close"));
+  }
+
+  Server svr_;
+  int port_ = 0;
+  thread thread_;
+};
+
+TEST_F(ExpectLazyContinueTest, PreRoutingRejectsWithoutInterimResponse) {
+  expect_final_without_interim("/pre-routing", "HTTP/1.1 401");
+}
+
+TEST_F(ExpectLazyContinueTest, PreRequestRejectsWithoutInterimResponse) {
+  expect_final_without_interim("/pre-request", "HTTP/1.1 403");
+}
+
+TEST_F(ExpectLazyContinueTest, UnknownRouteRejectsWithoutInterimResponse) {
+  expect_final_without_interim("/nowhere", "HTTP/1.1 404");
+}
+
+TEST_F(ExpectLazyContinueTest, UnreadContentReaderClosesWithoutInterim) {
+  expect_final_without_interim("/reader-unused", "HTTP/1.1 200");
+}
+
+TEST_F(ExpectLazyContinueTest, ContentReaderGetsInterimResponse) {
+  // The body follows once `100 Continue` has had time to arrive.
+  auto req = expect_headers_only("/reader-used");
+  req.insert(req.size() - 2, "Connection: close\r\n");
+  std::string resp;
+  ASSERT_TRUE(send_request_in_parts(3, {req, "hi"}, &resp, port_));
+  EXPECT_EQ(0u, resp.find("HTTP/1.1 100 Continue"));
+  EXPECT_NE(std::string::npos, resp.find("HTTP/1.1 200"));
+  EXPECT_NE(std::string::npos, resp.find("hi"));
+}
+
+TEST_F(ExpectLazyContinueTest, ClientWithholdsBodyWhenRejected) {
+  // Large enough for the client to add `Expect: 100-continue` itself.
+  const size_t length = CPPHTTPLIB_EXPECT_100_THRESHOLD * 4;
+  std::atomic<bool> body_sent{false};
+
+  Client cli(HOST, port_);
+  auto res = cli.Post(
+      "/pre-request", length,
+      [&](size_t /*offset*/, size_t len, DataSink &sink) {
+        body_sent = true;
+        std::string chunk(len, 'x');
+        sink.write(chunk.data(), chunk.size());
+        return true;
+      },
+      "application/octet-stream");
+
+  ASSERT_TRUE(res);
+  EXPECT_EQ(StatusCode::Forbidden_403, res->status);
+  EXPECT_FALSE(body_sent);
+}
+
+TEST(Expect100ContinueHandlerTest, ExpectationFailedIsFinalResponse) {
+  Server svr;
+  svr.set_expect_100_continue_handler([](const Request &, Response &) {
+    return StatusCode::ExpectationFailed_417;
+  });
+  svr.Post("/p", [](const Request &, Response &res) {
+    res.set_content("ok", "text/plain");
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  thread t = thread([&] { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+  });
+  svr.wait_until_ready();
+
+  std::string resp;
+  ASSERT_TRUE(send_request(3, expect_headers_only("/p"), &resp, port));
+  EXPECT_EQ(0u, resp.find("HTTP/1.1 417"));
+  EXPECT_NE(std::string::npos, resp.find("Connection: close"));
+  // Exactly one response: the route handler must not run after the 417.
+  EXPECT_EQ(std::string::npos, resp.find("HTTP/1.1", 1));
+}
+
 #ifndef _WIN32
 TEST(Expect100ContinueTest, ServerClosesConnection) {
   static constexpr char reject[] = "Unauthorized";