Ver código fonte

Do not let a zero-length write end a chunked body (#2563)

write_content_chunked()'s sink treated "the provider wrote nothing" as
"the provider has finished":

    data_available = l > 0;

so sink.write(p, 0) ended the loop. Only done()/done_with_trailer()
emit the terminating zero-length chunk, so the body was left
unterminated - and the function still returned Success, because the
post-loop check only reports the is_shutting_down() case. The peer waits
for a last chunk that never arrives, and on a keep-alive connection
anything written next is parsed as a chunk-size line.

A provider reaching a pass with nothing to hand over is ordinary:
popping an empty buffer off a queue, or a compressor that has consumed
its input without producing output yet. It is not the end of the
message.

Ignore zero-length writes instead. A zero-length chunk is the terminator
in chunked coding, so it must never be emitted mid-body either way, and
data_available is now controlled only by done()/done_with_trailer().
This matches write_content_without_length(), where the sink's write
never ends the body.

The old behaviour cannot have been relied on: it produced an
unterminated response, so a provider using it never worked in the first
place.
yhirose 3 dias atrás
pai
commit
ae417b405a
2 arquivos alterados com 47 adições e 2 exclusões
  1. 4 2
      httplib.h
  2. 43 0
      test/test.cc

+ 4 - 2
httplib.h

@@ -8452,8 +8452,10 @@ write_content_chunked(Stream &strm, const ContentProvider &content_provider,
   DataSink data_sink;
 
   data_sink.write = [&](const char *d, size_t l) -> bool {
-    if (ok) {
-      data_available = l > 0;
+    // Only done()/done_with_trailer() end a chunked body. A pass with nothing
+    // to hand over is ordinary (an empty buffer popped off a queue), and a
+    // zero-length chunk is the terminator, so it must not be emitted here.
+    if (ok && l > 0) {
       offset += l;
 
       std::string payload;

+ 43 - 0
test/test.cc

@@ -10058,6 +10058,49 @@ TEST(DownloadProgressTest, GetWithContentReceiver) {
       "/download-receiver", 2000u);
 }
 
+TEST(StreamingTest, ChunkedZeroLengthWriteDoesNotEndTheBody) {
+  // A provider reaching a pass with nothing to hand over - an empty buffer
+  // popped off a queue, say - must not be taken to mean the body is finished.
+  // It used to end the loop with the terminating chunk unwritten while the
+  // server still considered the response complete, so the peer waited for an
+  // end that never arrived.
+  Server svr;
+
+  svr.Get("/stream", [](const Request & /*req*/, Response &res) {
+    auto step = std::make_shared<int>(0);
+    res.set_chunked_content_provider("text/plain",
+                                     [step](size_t /*offset*/, DataSink &sink) {
+                                       switch ((*step)++) {
+                                       case 0: sink.write("", 0); break;
+                                       case 1: sink.write("hello", 5); break;
+                                       default: sink.done(); break;
+                                       }
+                                       return true;
+                                     });
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto listen_thread = std::thread([&svr]() { svr.listen_after_bind(); });
+  auto listen_se = detail::scope_exit([&] {
+    svr.stop();
+    listen_thread.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  Client cli(HOST, port);
+  // Without the fix the body is never terminated, so bound the wait rather
+  // than letting the test hang on the default read timeout.
+  cli.set_read_timeout(5, 0);
+
+  auto res = cli.Get("/stream");
+
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::OK_200, res->status);
+  EXPECT_EQ("hello", res->body);
+}
+
 TEST(StreamingTest, NoContentLengthStreaming) {
   Server svr;