From 5539a66c63a73c54f2b384c8459574f2a056e776 Mon Sep 17 00:00:00 2001 From: yhirose Date: Sat, 1 Aug 2026 21:47:35 -0400 Subject: [PATCH] Cover the non-206 Range paths and drop the duplicated test route The added test only exercised a 403 with a single range, leaving three paths that were equally broken untested: a suffix range, whose first_pos is still -1 when the bounds asserts are compiled out under NDEBUG; two ranges, where apply_ranges() makes no boundary for a non-206 response and the multipart branch wrote an empty one; and a 2xx that is not a 206, which was validated but still announced the full content length. Fold the copied route back into /streamed-with-range with a status query parameter, following the ?error idiom already used there. --- httplib.h | 15 +++++----- test/test.cc | 78 ++++++++++++++++++++++++++++------------------------ 2 files changed, 49 insertions(+), 44 deletions(-) diff --git a/httplib.h b/httplib.h index 81855f47..57479c6f 100644 --- a/httplib.h +++ b/httplib.h @@ -11845,16 +11845,15 @@ 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. That is the - // condition `apply_ranges()` used when it decided the Content-Length and - // the multipart boundary, and `detail::range_error()` only validates - // `req.ranges` against the content length for a 2xx status. Honouring the - // ranges here under any other status would write a body that disagrees - // with the header we already sent, from an offset nothing has checked. - auto partial = + // 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 (!partial) { + if (!is_partial) { return detail::write_content(strm, res.content_provider_, 0, res.content_length_, is_shutting_down); } else if (req.ranges.size() == 1) { diff --git a/test/test.cc b/test/test.cc index 51697594..388e0cf9 100644 --- a/test/test.cc +++ b/test/test.cc @@ -4540,6 +4540,15 @@ protected: .Get("/streamed-with-range", [&](const Request &req, Response &res) { auto data = new std::string("abcdefg"); + // An explicit status keeps the server from picking the 206 it + // would otherwise choose for a ranged request, so the response + // is not a partial representation. + auto status = req.get_param_value("status"); + if (status == "200") { + res.status = StatusCode::OK_200; + } else if (status == "403") { + res.status = StatusCode::Forbidden_403; + } res.set_content_provider( data->size(), "text/plain", [data](size_t offset, size_t length, DataSink &sink) { @@ -4557,24 +4566,6 @@ protected: delete data; }); }) - .Get("/streamed-with-range-and-error-status", - [&](const Request & /*req*/, Response &res) { - auto data = new std::string("abcdefg"); - res.status = StatusCode::Forbidden_403; - res.set_content_provider( - data->size(), "text/plain", - [data](size_t offset, size_t length, DataSink &sink) { - size_t DATA_CHUNK_SIZE = 4; - const auto &d = *data; - auto out_len = - std::min(static_cast(length), DATA_CHUNK_SIZE); - auto ret = - sink.write(&d[static_cast(offset)], out_len); - EXPECT_TRUE(ret); - return true; - }, - [data](bool /*success*/) { delete data; }); - }) .Get("/streamed-cancel", [&](const Request & /*req*/, Response &res) { res.set_content_provider( @@ -6088,25 +6079,40 @@ TEST_F(ServerTest, GetStreamedWithRangeSuffix2) { EXPECT_EQ(0U, res->body.size()); } -TEST_F(ServerTest, GetStreamedWithRangeAndErrorStatus) { - // The handler answers with a non-2xx status, so `detail::range_error()` - // never validated the ranges and `apply_ranges()` reported the full - // content length. The body must match that header and the content provider - // must not be asked for an offset outside the representation. - auto res = cli_.Get("/streamed-with-range-and-error-status", - Headers{{make_range_header({{3, 5}})}}); - ASSERT_TRUE(res) << "Error: " << to_string(res.error()); - EXPECT_EQ(StatusCode::Forbidden_403, res->status); - EXPECT_EQ("7", res->get_header_value("Content-Length")); - EXPECT_EQ(false, res->has_header("Content-Range")); - EXPECT_EQ(std::string("abcdefg"), res->body); +TEST_F(ServerTest, GetStreamedWithRangeAndNonPartialStatus) { + // Only a 206 is served as a partial representation. Under any other status + // `apply_ranges()` reported the full content length, so the body must match + // that header, and the content provider must never be asked for an offset + // outside the representation. + auto check = [&](int status, const char *range) { + auto path = + std::string("/streamed-with-range?status=") + std::to_string(status); + auto ctx = path + " Range: " + range; - auto res2 = cli_.Get("/streamed-with-range-and-error-status", - Headers{{"Range", "bytes=100000-100200"}}); - ASSERT_TRUE(res2) << "Error: " << to_string(res2.error()); - EXPECT_EQ(StatusCode::Forbidden_403, res2->status); - EXPECT_EQ("7", res2->get_header_value("Content-Length")); - EXPECT_EQ(std::string("abcdefg"), res2->body); + auto res = cli_.Get(path, Headers{{"Range", range}}); + ASSERT_TRUE(res) << ctx << " Error: " << to_string(res.error()); + EXPECT_EQ(status, res->status) << ctx; + EXPECT_EQ("7", res->get_header_value("Content-Length")) << ctx; + EXPECT_EQ("text/plain", res->get_header_value("Content-Type")) << ctx; + EXPECT_FALSE(res->has_header("Content-Range")) << ctx; + EXPECT_EQ(std::string("abcdefg"), res->body) << ctx; + }; + + // Non-2xx: `detail::range_error()` never validated these ranges at all. + check(403, "bytes=3-5"); + // The offset is far past the representation. + check(403, "bytes=100000-100200"); + // `first_pos` is still -1 here, and the bounds asserts in + // `get_range_offset_and_length()` are compiled out under NDEBUG. + check(403, "bytes=-3"); + // `apply_ranges()` makes no boundary for a non-206 response, so the + // multipart branch would have written one that is empty. + check(403, "bytes=1-2, 4-5"); + + // 2xx but not 206: the ranges were validated, yet the Content-Length still + // covers the whole representation. + check(200, "bytes=3-5"); + check(200, "bytes=1-2, 4-5"); } TEST_F(ServerTest, GetStreamedWithRangeError) {