From c58061ea8114718e1a18c6e05c11ff5daf811c99 Mon Sep 17 00:00:00 2001 From: yhirose Date: Wed, 26 Aug 2026 22:58:40 -0400 Subject: [PATCH] Fail a content provider that makes no progress write_content_with_progress() advances its offset only by what the provider writes, so a provider that reported success without writing anything and without calling done() was handed the same offset and length again on the next pass. With the peer still connected it spun there, re-entering the provider as fast as the loop could run. make_file_body()'s provider was one way to reach this and was fixed in #2566, but any user-supplied provider can do the same. Treat a pass that makes no progress as a short body, which is how done() called early is already handled. --- httplib.h | 11 +++++++++++ test/test.cc | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/httplib.h b/httplib.h index c63d4561..9d5d18df 100644 --- a/httplib.h +++ b/httplib.h @@ -8401,6 +8401,8 @@ inline bool write_content_with_progress(Stream &strm, data_sink.done = [&]() { finished = true; }; while (offset < end_offset && !finished && !is_shutting_down()) { + auto last_offset = offset; + if (!strm.wait_writable() || !strm.is_peer_alive()) { error = Error::Write; return false; @@ -8411,6 +8413,15 @@ inline bool write_content_with_progress(Stream &strm, error = Error::Write; return false; } + + // A provider that reports success without writing anything and without + // reporting itself done gets handed the same offset and length again on + // the next pass, so it would spin here for as long as the peer stays + // connected. Treat making no progress as a short body, like done() early. + if (!finished && offset == last_offset) { + error = Error::Write; + return false; + } } if (offset < end_offset) { // done() called early, or is_shutting_down() diff --git a/test/test.cc b/test/test.cc index 84dda3a5..0a7094b2 100644 --- a/test/test.cc +++ b/test/test.cc @@ -10948,6 +10948,43 @@ TEST(ClientProblemDetectionTest, ContentProvider) { } } +TEST(ContentProviderTest, ProviderMakingNoProgressFails) { + // A provider that reports success without writing anything and without + // calling done() used to be handed the same offset and length again on every + // pass, so it spun for as long as the peer stayed connected. + Server svr; + + svr.Post("/", [](const Request & /*req*/, Response &res) { + res.set_content("ok", "text/plain"); + }); + + 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(); + + std::atomic call_count{0}; + + Client cli(HOST, port); + auto res = cli.Post( + "/", 1024, + [&](size_t /*offset*/, size_t /*length*/, DataSink & /*sink*/) { + // Give up after enough passes to show the spin, so that losing the + // check below fails this test instead of hanging it. + return ++call_count < 100; + }, + "text/plain"); + + ASSERT_FALSE(res); + EXPECT_EQ(Error::Write, res.error()); + EXPECT_EQ(1, call_count.load()); +} + TEST(DataSinkTest, OptionalCallbacksAreCallableByDefault) { // A writer only has to assign `write`. The other three used to be left as // empty std::functions, so a provider calling one threw