diff --git a/mcpp.toml b/mcpp.toml index ec01e93..8ea45b7 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] namespace = "mcpplibs" name = "tinyhttps" -version = "0.2.9" +version = "0.2.10" description = "Minimal C++23 HTTP/HTTPS client with SSE streaming support" license = "Apache-2.0" repo = "https://github.com/mcpplibs/tinyhttps" diff --git a/src/http.cppm b/src/http.cppm index 67d700e..738d2c6 100644 --- a/src/http.cppm +++ b/src/http.cppm @@ -256,6 +256,25 @@ parse_chunk_size_line(std::string_view line) { return static_cast(value); } +// A streaming request that fails is answered with an error document, not an +// event stream, so SseParser finds no event boundary in it and yields nothing. +// The bytes are still worth keeping: without them a caller can report the +// status line but never the reason. Bounded, so a server answering 5xx with an +// endless body cannot grow the buffer without limit. +export inline constexpr std::size_t stream_error_body_limit = 1024 * 1024; + +// Appends as much of `data` as `limit` still allows. Returns false once the +// buffer is full, so a caller can stop copying without tracking sizes itself. +export bool append_within_limit(std::string& buffer, std::string_view data, + std::size_t limit) { + if (buffer.size() >= limit) return false; + const std::size_t room = limit - buffer.size(); + // Not std::min: defines a `min` macro on Windows. + const std::size_t take = data.size() < room ? data.size() : room; + buffer.append(data.substr(0, take)); + return take == data.size(); +} + // Case-insensitive string comparison static bool iequals(std::string_view a, std::string_view b) { if (a.size() != b.size()) return false; @@ -732,8 +751,15 @@ public: // Stream body incrementally, feeding chunks to SseParser SseParser parser; bool stopped = false; + // send() fills `body` on every path including failures; without this + // send_stream would be the one entry point that drops it. Capturing is + // additive — events are still parsed and dispatched exactly as before. + const bool captureBody = !response.ok(); auto dispatch = [&](std::string_view data) -> bool { + if (captureBody) { + append_within_limit(response.body, data, stream_error_body_limit); + } auto events = parser.feed(data); for (const auto& ev : events) { if (!callback(ev)) { diff --git a/tests/test_download.cpp b/tests/test_download.cpp index c862fbe..1a97184 100644 --- a/tests/test_download.cpp +++ b/tests/test_download.cpp @@ -20,6 +20,29 @@ TEST(ChunkedProtocol, AcceptsValidSizeAndTerminalChunk) { EXPECT_EQ(*https::parse_chunk_size_line("0"), 0); } +TEST(StreamErrorBody, KeepsEverythingWhileUnderLimit) { + std::string buffer; + EXPECT_TRUE(https::append_within_limit(buffer, "abc", 8)); + EXPECT_TRUE(https::append_within_limit(buffer, "de", 8)); + EXPECT_EQ(buffer, "abcde"); +} + +TEST(StreamErrorBody, TruncatesTheChunkThatCrossesTheLimit) { + std::string buffer = "abc"; + EXPECT_FALSE(https::append_within_limit(buffer, "defgh", 5)); + EXPECT_EQ(buffer, "abcde"); +} + +TEST(StreamErrorBody, RefusesFurtherDataOnceFull) { + std::string buffer = "abcde"; + EXPECT_FALSE(https::append_within_limit(buffer, "f", 5)); + EXPECT_EQ(buffer, "abcde"); + // A zero limit must not append anything, not even an empty append. + std::string empty; + EXPECT_FALSE(https::append_within_limit(empty, "a", 0)); + EXPECT_TRUE(empty.empty()); +} + TEST(DownloadResultContract, CarriesTransferAndResponseMetadata) { https::DownloadToFileResult result; result.bytesWritten = 42; @@ -36,6 +59,38 @@ TEST(DownloadResultContract, CarriesTransferAndResponseMetadata) { EXPECT_FALSE(result.lastModified.empty()); } +// Test that a failed streaming request carries its error body, against a real +// HTTPS endpoint. httpbin's /status/418 answers non-2xx with a body, which is +// exactly the shape SseParser cannot turn into events. + +class StreamErrorBodyLiveTest : public ::testing::Test { +protected: + void SetUp() override { https::Socket::platform_init(); } +}; + +TEST_F(StreamErrorBodyLiveTest, FailedStreamKeepsTheErrorBody) { + https::HttpClientConfig cfg; + cfg.connectTimeoutMs = 15000; + cfg.readTimeoutMs = 30000; + cfg.keepAlive = false; // so the server closes and the read loop ends + https::HttpClient client(cfg); + + https::HttpRequest req; + req.method = https::Method::GET; + req.url = "https://httpbin.org/status/418"; + + int events = 0; + auto res = client.send_stream(req, [&](const https::SseEvent&) { + ++events; + return true; + }); + + EXPECT_EQ(res.statusCode, 418); + EXPECT_EQ(events, 0) << "an error document is not an event stream"; + EXPECT_FALSE(res.body.empty()) << "error body was dropped"; + EXPECT_NE(res.body.find("teapot"), std::string::npos); +} + // Test download_to_file against a real HTTPS endpoint. // Uses httpbin.org which returns known-size responses.