Frame response bodies in one place

A content provider that wrote past the length it was given put the
extra bytes after the framed body, where a keep-alive peer read them as
the next message. Write only what fits and fail the write, on both the
server and the client.

Content-Length and Transfer-Encoding a handler set were sent next to
the ones the library worked out, for ranges and compression, and a
post-routing handler could change the body after Content-Length was
set. Work both out in one place, from the body about to be written,
again after the post-routing handler; a HEAD response keeps the length
its handler gave it. A 1xx, 204 or 304 response sends no body and,
following RFC 9110 8.6, no Content-Length.

The client still sends a Content-Length or Transfer-Encoding its caller
passes, as curl and urllib3 do, and the README says these have to match
the body.

README.md

@@ -790,6 +790,8 @@ svr.Get("/stream", [&](const Request &req, Response &res) {
 });
 ```
 
+Write at most `length` bytes per call. Anything past it is not sent, and the response fails.
+
 Without content length:
 
 ```cpp
@@ -1127,6 +1129,8 @@ cli.set_default_headers({
 auto res = cli.Get("/hi");
 ```
 
+A `Content-Length` or `Transfer-Encoding` among these headers is sent in place of the one the library would send, so it has to describe the body as written. Leave both out when passing on the headers of another message, as a proxy does.
+
 ### POST
 
 ```c++

httplib.h

@@ -8857,6 +8857,12 @@ inline bool write_content_with_progress(Stream &strm,
 
   data_sink.write = [&](const char *d, size_t l) -> bool {
     if (ok) {
+      // Bytes past `length` would follow the framed body, where the peer reads
+      // them as the next message. Write only what fits and fail the rest.
+      if (l > end_offset - offset) {
+        l = end_offset - offset;
+        ok = false;
+      }
       if (write_data(strm, d, l)) {
         offset += l;
 
@@ -12060,6 +12066,58 @@ inline void prefer_assigned_body(Response &res) {
   }
 }
 
+// RFC 9112 6.3: a 1xx, 204 or 304 response ends with its header section.
+inline bool body_allowed_for_status(int status) {
+  return status >= 200 && status != StatusCode::NoContent_204 &&
+         status != StatusCode::NotModified_304;
+}
+
+// Only a 206 response is served as a partial representation. range_error()
+// validates `req.ranges` only for a 2xx status, so slicing under any other
+// status would start from an unchecked offset.
+inline bool is_partial_content(const Request &req, const Response &res) {
+  return !req.ranges.empty() && res.status == StatusCode::PartialContent_206;
+}
+
+// Sets the Content-Length or Transfer-Encoding framing the body that
+// Server::write_response_core() writes. Only the library sets them: one from a
+// handler would go out next to it, or describe some other body.
+inline void set_framing_headers(const Request &req, Response &res,
+                                const std::string &boundary,
+                                const std::string &content_type) {
+  // A HEAD response may give the length a GET would have had (RFC 9110 9.3.2).
+  auto head_length = req.method == "HEAD"
+                         ? res.get_header_value("Content-Length")
+                         : std::string();
+  res.headers.erase("Content-Length");
+  res.headers.erase("Transfer-Encoding");
+
+  if (!body_allowed_for_status(res.status)) { return; }
+
+  if (res.content_provider_) {
+    if (res.content_length_ == 0) {
+      if (res.is_chunked_content_provider_) {
+        res.set_header("Transfer-Encoding", "chunked");
+      }
+      return;
+    }
+
+    // The part Server::write_content_with_provider() writes.
+    auto length = res.content_length_;
+    if (is_partial_content(req, res)) {
+      length = req.ranges.size() == 1
+                   ? get_range_offset_and_length(req.ranges[0], length).second
+                   : get_multipart_ranges_data_length(req, boundary,
+                                                      content_type, length);
+    }
+    res.set_header("Content-Length", std::to_string(length));
+  } else if (!res.body.empty()) {
+    res.set_header("Content-Length", std::to_string(res.body.size()));
+  } else {
+    res.set_header("Content-Length", head_length.empty() ? "0" : head_length);
+  }
+}
+
 } // namespace detail
 
 inline size_t Response::get_header_value_u64(const std::string &key, size_t def,
@@ -13777,6 +13835,9 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
     need_apply_ranges = true;
   }
 
+  if (!detail::body_allowed_for_status(res.status)) {
+    detail::clear_content(res);
+  }
   detail::prefer_assigned_body(res);
 
   std::string content_type;
@@ -13807,16 +13868,17 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
     res.set_header("Content-Type", "text/plain");
   }
 
-  if (res.body.empty() && !res.content_length_ && !res.content_provider_ &&
-      !res.has_header("Content-Length")) {
-    res.set_header("Content-Length", "0");
-  }
+  detail::set_framing_headers(req, res, boundary, content_type);
 
   if (req.method == "HEAD" && !res.has_header("Accept-Ranges")) {
     res.set_header("Accept-Ranges", "bytes");
   }
 
-  if (post_routing_handler_) { post_routing_handler_(req, res); }
+  if (post_routing_handler_) {
+    post_routing_handler_(req, res);
+    // Again, for a body or framing headers the handler changed.
+    detail::set_framing_headers(req, res, boundary, content_type);
+  }
 
   // Response line and headers
   detail::BufferStream bstrm;
@@ -13826,7 +13888,8 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
   // Combine a small body with the headers so the whole response leaves in a
   // single write. A large body is written on its own instead: a copy of it
   // costs more than the extra write saves.
-  auto send_body = req.method != "HEAD";
+  auto send_body =
+      req.method != "HEAD" && detail::body_allowed_for_status(res.status);
   auto body_is_separate = false;
   auto provider_done = false;
   if (send_body && !res.body.empty() && !res.content_provider_) {
@@ -13879,15 +13942,7 @@ Server::write_content_with_provider(Stream &strm, const Request &req,
   };
 
   if (res.content_length_ > 0) {
-    // Only a 206 response is served as a partial representation, matching the
-    // condition `apply_ranges()` used to decide the Content-Length and the
-    // multipart boundary. Since `detail::range_error()` validates `req.ranges`
-    // only for a 2xx status, slicing under any other status would write a body
-    // that disagrees with the header already sent, from an unchecked offset.
-    auto is_partial =
-        !req.ranges.empty() && res.status == StatusCode::PartialContent_206;
-
-    if (!is_partial) {
+    if (!detail::is_partial_content(req, res)) {
       return detail::write_content(strm, res.content_provider_, 0,
                                    res.content_length_, is_shutting_down);
     } else if (req.ranges.size() == 1) {
@@ -14588,8 +14643,8 @@ Server::static_file_encoding(const Request &req, const Response &res,
   return detail::encoding_type(req, res, content_type);
 }
 
-// Compresses a file-backed content provider into `res.body` and takes over the
-// framing headers. Returns false when the response is left untouched.
+// Compresses a file-backed content provider into `res.body`. Returns false when
+// the response is left untouched.
 inline bool Server::apply_static_file_compression(const Request &req,
                                                   Response &res) const {
   auto type = res.content_coding_;
@@ -14617,7 +14672,6 @@ inline bool Server::apply_static_file_compression(const Request &req,
 
   res.set_header("Content-Encoding", detail::encoding_name(type));
   res.set_header("Vary", "Accept-Encoding");
-  res.set_header("Content-Length", std::to_string(res.body.size()));
 
   return true;
 }
@@ -14631,7 +14685,7 @@ inline void Server::apply_ranges(const Request &req, Response &res,
   // ordinary body: `Content-Length` and HEAD keep working, and the response
   // takes the same path as `set_content()` from here on. Range requests never
   // get a content coding, so `Content-Range` still names identity bytes and
-  // none of the framing below applies.
+  // none of the range handling below applies.
   if (apply_static_file_compression(req, res)) { return; }
 
   if (req.ranges.size() > 1 && res.status == StatusCode::PartialContent_206) {
@@ -14650,34 +14704,19 @@ inline void Server::apply_ranges(const Request &req, Response &res,
   auto type = detail::encoding_type(req, res);
 
   if (res.body.empty()) {
-    if (res.content_length_ > 0) {
-      size_t length = 0;
-      if (req.ranges.empty() || res.status != StatusCode::PartialContent_206) {
-        length = res.content_length_;
-      } else if (req.ranges.size() == 1) {
-        auto offset_and_length = detail::get_range_offset_and_length(
-            req.ranges[0], res.content_length_);
-
-        length = offset_and_length.second;
-
-        auto content_range = detail::make_content_range_header_field(
-            offset_and_length, res.content_length_);
-        res.set_header("Content-Range", content_range);
-      } else {
-        length = detail::get_multipart_ranges_data_length(
-            req, boundary, content_type, res.content_length_);
-      }
-      res.set_header("Content-Length", std::to_string(length));
-    } else {
-      if (res.content_provider_) {
-        if (res.is_chunked_content_provider_) {
-          res.set_header("Transfer-Encoding", "chunked");
-          res.content_coding_ = type;
-          if (type != detail::EncodingType::None) {
-            res.set_header("Content-Encoding", detail::encoding_name(type));
-            res.set_header("Vary", "Accept-Encoding");
-          }
-        }
+    if (res.content_length_ > 0 && req.ranges.size() == 1 &&
+        res.status == StatusCode::PartialContent_206) {
+      auto offset_and_length = detail::get_range_offset_and_length(
+          req.ranges[0], res.content_length_);
+
+      auto content_range = detail::make_content_range_header_field(
+          offset_and_length, res.content_length_);
+      res.set_header("Content-Range", content_range);
+    } else if (res.is_chunked_content_provider_) {
+      res.content_coding_ = type;
+      if (type != detail::EncodingType::None) {
+        res.set_header("Content-Encoding", detail::encoding_name(type));
+        res.set_header("Vary", "Accept-Encoding");
       }
     }
   } else {
@@ -14720,7 +14759,6 @@ inline void Server::apply_ranges(const Request &req, Response &res,
     }
 
     res.content_length_ = res.body.size();
-    res.set_header("Content-Length", std::to_string(res.content_length_));
   }
 }
 

test/test.cc

@@ -5355,10 +5355,99 @@ TEST(NoContentTest, ContentLength) {
     auto res = cli.Get("/hi");
     ASSERT_TRUE(res) << "Error: " << to_string(res.error());
     EXPECT_EQ(StatusCode::NoContent_204, res->status);
-    EXPECT_EQ("0", res->get_header_value("Content-Length"));
+    // RFC 9110 8.6: none in a 204 response
+    EXPECT_FALSE(res->has_header("Content-Length"));
   }
 }
 
+TEST(ResponseFramingTest, HandlerFramingHeadersAreReplaced) {
+  Server svr;
+
+  svr.Get("/body", [](const Request & /*req*/, Response &res) {
+    res.set_header("Content-Length", "5");
+    res.set_content("hello world", "text/plain");
+  });
+  svr.Get("/chunked", [](const Request & /*req*/, Response &res) {
+    // A body that reads as the last chunk, were it taken as chunked
+    res.set_header("Transfer-Encoding", "chunked");
+    res.set_content("0\r\n\r\n", "text/plain");
+  });
+  svr.Get("/provider", [](const Request & /*req*/, Response &res) {
+    res.set_header("Content-Length", "2");
+    res.set_content_provider(5, "text/plain",
+                             [](size_t offset, size_t length, DataSink &sink) {
+                               sink.write(&"hello"[offset], length);
+                               return true;
+                             });
+  });
+  svr.Get("/empty", [](const Request & /*req*/, Response &res) {
+    res.set_header("Content-Length", "1234");
+  });
+
+  thread t = thread([&] { svr.listen(HOST, PORT); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  Client cli(HOST, PORT);
+  cli.set_read_timeout(1, 0);
+
+  auto expect_body = [&](const Result &res, const std::string &body) {
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(body, res->body);
+    EXPECT_EQ(1u, res->get_header_value_count("Content-Length"));
+    EXPECT_FALSE(res->has_header("Transfer-Encoding"));
+  };
+
+  Headers identity{{"Accept-Encoding", ""}};
+  expect_body(cli.Get("/body", identity), "hello world");
+  expect_body(cli.Get("/chunked", identity), "0\r\n\r\n");
+  expect_body(cli.Get("/body",
+                      Headers{{"Accept-Encoding", ""}, {"Range", "bytes=0-1"}}),
+              "he");
+  expect_body(cli.Get("/provider", identity), "hello");
+  expect_body(cli.Get("/empty", identity), "");
+
+  // A HEAD response may give the length of the body a GET would get.
+  auto res = cli.Head("/empty");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ("1234", res->get_header_value("Content-Length"));
+}
+
+TEST(ResponseFramingTest, PostRoutingHandlerChangesAreFramed) {
+  Server svr;
+
+  svr.Get("/", [](const Request & /*req*/, Response &res) {
+    res.set_content("short", "text/plain");
+  });
+  svr.set_post_routing_handler([](const Request & /*req*/, Response &res) {
+    res.body += " with a footer";
+    res.set_header("Transfer-Encoding", "chunked");
+  });
+
+  thread t = thread([&] { svr.listen(HOST, PORT); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  Client cli(HOST, PORT);
+  cli.set_read_timeout(1, 0);
+
+  auto res = cli.Get("/", Headers{{"Accept-Encoding", ""}});
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ("short with a footer", res->body);
+  EXPECT_EQ("19", res->get_header_value("Content-Length"));
+  EXPECT_FALSE(res->has_header("Transfer-Encoding"));
+}
+
 TEST(RoutingHandlerTest, PreAndPostRoutingHandlers) {
 #ifdef CPPHTTPLIB_SSL_ENABLED
   SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE);
@@ -11604,7 +11693,7 @@ TEST(UploadProgressTest, PostContentProviderProgress) {
       },
       [](Client &cli, UploadProgress progress_callback) {
         return cli.Post(
-            "/test", 10,
+            "/test", 9,
             [](size_t /*offset*/, size_t /*length*/, DataSink &sink) -> bool {
               sink.os << "test data";
               return true;
@@ -12617,6 +12706,115 @@ TEST(DataSinkTest, LengthFramedProviderThatFinishesEarlyFailsTheResponse) {
   EXPECT_FALSE(res);
 }
 
+// Read as the next response by a peer that sees it after a framed body.
+static const std::string smuggled_response =
+    "HTTP/1.1 200 OK\r\nContent-Length: 8\r\n\r\nSMUGGLED";
+
+TEST(DataSinkTest, LengthFramedProviderCannotWritePastTheLength) {
+  Server svr;
+
+  svr.Get("/", [](const Request & /*req*/, Response &res) {
+    res.set_content_provider(
+        5, "text/plain",
+        [](size_t /*offset*/, size_t /*length*/, DataSink &sink) {
+          auto data = "AAAAA" + smuggled_response;
+          sink.write(data.data(), data.size());
+          return true;
+        });
+  });
+
+  thread t = thread([&] { svr.listen(HOST, PORT); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  std::string resp;
+  ASSERT_TRUE(send_request(1, "GET / HTTP/1.1\r\nHost: x\r\n\r\n", &resp));
+  EXPECT_EQ("AAAAA", resp.substr(resp.find("\r\n\r\n") + 4));
+
+  resp.clear();
+  ASSERT_TRUE(send_request(
+      1, "GET / HTTP/1.1\r\nHost: x\r\nRange: bytes=0-1\r\n\r\n", &resp));
+  EXPECT_EQ("AA", resp.substr(resp.find("\r\n\r\n") + 4));
+}
+
+TEST(NoContentTest, BodyIsNotSent) {
+  Server svr;
+
+  svr.Get("/204", [](const Request & /*req*/, Response &res) {
+    res.status = StatusCode::NoContent_204;
+    res.set_content(smuggled_response, "text/plain");
+  });
+  svr.Get("/304", [](const Request & /*req*/, Response &res) {
+    res.status = StatusCode::NotModified_304;
+    res.set_chunked_content_provider(
+        "text/plain", [](size_t /*offset*/, DataSink &sink) {
+          sink.write(smuggled_response.data(), smuggled_response.size());
+          sink.done();
+          return true;
+        });
+  });
+
+  thread t = thread([&] { svr.listen(HOST, PORT); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  for (auto path : {"/204", "/304"}) {
+    std::string resp;
+    ASSERT_TRUE(send_request(
+        1, std::string("GET ") + path + " HTTP/1.1\r\nHost: x\r\n\r\n", &resp));
+    EXPECT_EQ(resp.size(), resp.find("\r\n\r\n") + 4) << path;
+    EXPECT_EQ(std::string::npos, resp.find("Content-Length")) << path;
+    EXPECT_EQ(std::string::npos, resp.find("Transfer-Encoding")) << path;
+  }
+}
+
+TEST(DataSinkTest, RequestProviderCannotWritePastTheLength) {
+  Server svr;
+
+  std::atomic<bool> smuggled(false);
+  svr.Post("/", [](const Request & /*req*/, Response &res) {
+    res.set_content("ok", "text/plain");
+  });
+  svr.Get("/smuggled", [&](const Request & /*req*/, Response & /*res*/) {
+    smuggled = true;
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto listen_thread = std::thread([&svr]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    listen_thread.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  // What follows the body would be read as the next request.
+  Client cli(HOST, port);
+  auto res = cli.Post(
+      "/", 5,
+      [](size_t /*offset*/, size_t /*length*/, DataSink &sink) {
+        std::string data = "helloGET /smuggled HTTP/1.1\r\nHost: x\r\n\r\n";
+        sink.write(data.data(), data.size());
+        return true;
+      },
+      "text/plain");
+
+  ASSERT_FALSE(res);
+  EXPECT_EQ(Error::Write, res.error());
+  EXPECT_FALSE(smuggled);
+}
+
 #ifdef CPPHTTPLIB_ZLIB_SUPPORT
 TEST(DataSinkTest, CompressedRequestProviderThatFinishesEarlyFails) {
   // The compressed path builds the whole body before connecting, so this