From 27879c4874aabfc254edd25cf58385c25ac247b7 Mon Sep 17 00:00:00 2001 From: yhirose Date: Mon, 23 Jun 2025 08:35:56 -0400 Subject: [PATCH] Fix #2157 (#2158) * Fix #2157 * Fix Windows build error: wrap std::max in parentheses to avoid macro conflict - On Windows, max/min are often defined as macros by windows.h - This causes compilation errors with std::max/std::min - Solution: use (std::max) to prevent macro expansion - Fixes CI build error: error C2589: '(': illegal token on right side of '::' Fixes: error in coalesce_ranges function on line 5376 --- httplib.h | 78 ++++++++++++++++++++++++++++++++++++++++++------- test/test.cc | 82 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 143 insertions(+), 17 deletions(-) diff --git a/httplib.h b/httplib.h index d9aa8f1..217cb88 100644 --- a/httplib.h +++ b/httplib.h @@ -5329,13 +5329,68 @@ serialize_multipart_formdata(const MultipartFormDataItems &items, return body; } +inline void coalesce_ranges(Ranges &ranges, size_t content_length) { + if (ranges.size() <= 1) return; + + // Sort ranges by start position + std::sort(ranges.begin(), ranges.end(), + [](const Range &a, const Range &b) { return a.first < b.first; }); + + Ranges coalesced; + coalesced.reserve(ranges.size()); + + for (auto &r : ranges) { + auto first_pos = r.first; + auto last_pos = r.second; + + // Handle special cases like in range_error + if (first_pos == -1 && last_pos == -1) { + first_pos = 0; + last_pos = static_cast(content_length); + } + + if (first_pos == -1) { + first_pos = static_cast(content_length) - last_pos; + last_pos = static_cast(content_length) - 1; + } + + if (last_pos == -1 || last_pos >= static_cast(content_length)) { + last_pos = static_cast(content_length) - 1; + } + + // Skip invalid ranges + if (!(0 <= first_pos && first_pos <= last_pos && + last_pos < static_cast(content_length))) { + continue; + } + + // Coalesce with previous range if overlapping or adjacent (but not + // identical) + if (!coalesced.empty()) { + auto &prev = coalesced.back(); + // Check if current range overlaps or is adjacent to previous range + // but don't coalesce identical ranges (allow duplicates) + if (first_pos <= prev.second + 1 && + !(first_pos == prev.first && last_pos == prev.second)) { + // Extend the previous range + prev.second = (std::max)(prev.second, last_pos); + continue; + } + } + + // Add new range + coalesced.emplace_back(first_pos, last_pos); + } + + ranges = std::move(coalesced); +} + inline bool range_error(Request &req, Response &res) { if (!req.ranges.empty() && 200 <= res.status && res.status < 300) { ssize_t content_len = static_cast( res.content_length_ ? res.content_length_ : res.body.size()); - ssize_t prev_first_pos = -1; - ssize_t prev_last_pos = -1; + std::vector> processed_ranges; size_t overwrapping_count = 0; // NOTE: The following Range check is based on '14.2. Range' in RFC 9110 @@ -5378,18 +5433,21 @@ inline bool range_error(Request &req, Response &res) { return true; } - // Ranges must be in ascending order - if (first_pos <= prev_first_pos) { return true; } - // Request must not have more than two overlapping ranges - if (first_pos <= prev_last_pos) { - overwrapping_count++; - if (overwrapping_count > 2) { return true; } + for (const auto &processed_range : processed_ranges) { + if (!(last_pos < processed_range.first || + first_pos > processed_range.second)) { + overwrapping_count++; + if (overwrapping_count > 2) { return true; } + break; // Only count once per range + } } - prev_first_pos = (std::max)(prev_first_pos, first_pos); - prev_last_pos = (std::max)(prev_last_pos, last_pos); + processed_ranges.emplace_back(first_pos, last_pos); } + + // After validation, coalesce overlapping ranges as per RFC 9110 + coalesce_ranges(req.ranges, static_cast(content_len)); } return false; diff --git a/test/test.cc b/test/test.cc index 11f622a..b700755 100644 --- a/test/test.cc +++ b/test/test.cc @@ -3992,6 +3992,16 @@ TEST_F(ServerTest, GetStreamedWithRangeMultipart) { EXPECT_EQ("267", res->get_header_value("Content-Length")); EXPECT_EQ(false, res->has_header("Content-Range")); EXPECT_EQ(267U, res->body.size()); + + // Check that both range contents are present + EXPECT_TRUE(res->body.find("bc\r\n") != std::string::npos); + EXPECT_TRUE(res->body.find("ef\r\n") != std::string::npos); + + // Check that Content-Range headers are present for both ranges + EXPECT_TRUE(res->body.find("Content-Range: bytes 1-2/7") != + std::string::npos); + EXPECT_TRUE(res->body.find("Content-Range: bytes 4-5/7") != + std::string::npos); } TEST_F(ServerTest, GetStreamedWithTooManyRanges) { @@ -4009,14 +4019,59 @@ TEST_F(ServerTest, GetStreamedWithTooManyRanges) { EXPECT_EQ(0U, res->body.size()); } -TEST_F(ServerTest, GetStreamedWithNonAscendingRanges) { - auto res = cli_.Get("/streamed-with-range?error", - {{make_range_header({{0, -1}, {0, -1}})}}); +TEST_F(ServerTest, GetStreamedWithOverwrapping) { + auto res = + cli_.Get("/streamed-with-range", {{make_range_header({{1, 4}, {2, 5}})}}); ASSERT_TRUE(res); - EXPECT_EQ(StatusCode::RangeNotSatisfiable_416, res->status); - EXPECT_EQ("0", res->get_header_value("Content-Length")); - EXPECT_EQ(false, res->has_header("Content-Range")); - EXPECT_EQ(0U, res->body.size()); + EXPECT_EQ(StatusCode::PartialContent_206, res->status); + EXPECT_EQ(5U, res->body.size()); + + // Check that overlapping ranges are coalesced into a single range + EXPECT_EQ("bcdef", res->body); + EXPECT_EQ("bytes 1-5/7", res->get_header_value("Content-Range")); + + // Should be single range, not multipart + EXPECT_TRUE(res->has_header("Content-Range")); + EXPECT_EQ("text/plain", res->get_header_value("Content-Type")); +} + +TEST_F(ServerTest, GetStreamedWithNonAscendingRanges) { + auto res = + cli_.Get("/streamed-with-range", {{make_range_header({{4, 5}, {0, 2}})}}); + ASSERT_TRUE(res); + EXPECT_EQ(StatusCode::PartialContent_206, res->status); + EXPECT_EQ(268U, res->body.size()); + + // Check that both range contents are present + EXPECT_TRUE(res->body.find("ef\r\n") != std::string::npos); + EXPECT_TRUE(res->body.find("abc\r\n") != std::string::npos); + + // Check that Content-Range headers are present for both ranges + EXPECT_TRUE(res->body.find("Content-Range: bytes 4-5/7") != + std::string::npos); + EXPECT_TRUE(res->body.find("Content-Range: bytes 0-2/7") != + std::string::npos); +} + +TEST_F(ServerTest, GetStreamedWithDuplicateRanges) { + auto res = + cli_.Get("/streamed-with-range", {{make_range_header({{0, 2}, {0, 2}})}}); + ASSERT_TRUE(res); + EXPECT_EQ(StatusCode::PartialContent_206, res->status); + EXPECT_EQ(269U, res->body.size()); + + // Check that both duplicate range contents are present + size_t first_abc = res->body.find("abc\r\n"); + EXPECT_TRUE(first_abc != std::string::npos); + size_t second_abc = res->body.find("abc\r\n", first_abc + 1); + EXPECT_TRUE(second_abc != std::string::npos); + + // Check that Content-Range headers are present for both ranges + size_t first_range = res->body.find("Content-Range: bytes 0-2/7"); + EXPECT_TRUE(first_range != std::string::npos); + size_t second_range = + res->body.find("Content-Range: bytes 0-2/7", first_range + 1); + EXPECT_TRUE(second_range != std::string::npos); } TEST_F(ServerTest, GetStreamedWithRangesMoreThanTwoOverwrapping) { @@ -4122,6 +4177,19 @@ TEST_F(ServerTest, GetWithRange4) { EXPECT_EQ(std::string("fg"), res->body); } +TEST_F(ServerTest, GetWithRange5) { + auto res = cli_.Get("/with-range", { + make_range_header({{0, 5}}), + {"Accept-Encoding", ""}, + }); + ASSERT_TRUE(res); + EXPECT_EQ(StatusCode::PartialContent_206, res->status); + EXPECT_EQ("6", res->get_header_value("Content-Length")); + EXPECT_EQ(true, res->has_header("Content-Range")); + EXPECT_EQ("bytes 0-5/7", res->get_header_value("Content-Range")); + EXPECT_EQ(std::string("abcdef"), res->body); +} + TEST_F(ServerTest, GetWithRangeOffsetGreaterThanContent) { auto res = cli_.Get("/with-range", {{make_range_header({{10000, 20000}})}}); ASSERT_TRUE(res);