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