Skip to content

Commit d6cb682

Browse files
yhiroseyhirose
committed
Fix #2157 
1 parent 3a1f379 commit d6cb682

File tree

2 files changed

+135
-15
lines changed

2 files changed

+135
-15
lines changed

‎httplib.h‎

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5329,13 +5329,66 @@ serialize_multipart_formdata(const MultipartFormDataItems &items,
53295329
return body;
53305330
}
53315331

5332+
inline void coalesce_ranges(Ranges &ranges, size_t content_length) {
5333+
if (ranges.size() <= 1) return;
5334+
5335+
// Sort ranges by start position
5336+
std::sort(ranges.begin(), ranges.end(),
5337+
[](const Range &a, const Range &b) { return a.first < b.first; });
5338+
5339+
Ranges coalesced;
5340+
coalesced.reserve(ranges.size());
5341+
5342+
for (auto &r : ranges) {
5343+
auto first_pos = r.first;
5344+
auto last_pos = r.second;
5345+
5346+
// Handle special cases like in range_error
5347+
if (first_pos == -1 && last_pos == -1) {
5348+
first_pos = 0;
5349+
last_pos = static_cast<ssize_t>(content_length);
5350+
}
5351+
5352+
if (first_pos == -1) {
5353+
first_pos = static_cast<ssize_t>(content_length) - last_pos;
5354+
last_pos = static_cast<ssize_t>(content_length) - 1;
5355+
}
5356+
5357+
if (last_pos == -1 || last_pos >= static_cast<ssize_t>(content_length)) {
5358+
last_pos = static_cast<ssize_t>(content_length) - 1;
5359+
}
5360+
5361+
// Skip invalid ranges
5362+
if (!(0 <= first_pos && first_pos <= last_pos &&
5363+
last_pos < static_cast<ssize_t>(content_length))) {
5364+
continue;
5365+
}
5366+
5367+
// Coalesce with previous range if overlapping or adjacent (but not identical)
5368+
if (!coalesced.empty()) {
5369+
auto &prev = coalesced.back();
5370+
// Check if current range overlaps or is adjacent to previous range
5371+
// but don't coalesce identical ranges (allow duplicates)
5372+
if (first_pos <= prev.second + 1 && !(first_pos == prev.first && last_pos == prev.second)) {
5373+
// Extend the previous range
5374+
prev.second = std::max(prev.second, last_pos);
5375+
continue;
5376+
}
5377+
}
5378+
5379+
// Add new range
5380+
coalesced.emplace_back(first_pos, last_pos);
5381+
}
5382+
5383+
ranges = std::move(coalesced);
5384+
}
5385+
53325386
inline bool range_error(Request &req, Response &res) {
53335387
if (!req.ranges.empty() && 200 <= res.status && res.status < 300) {
53345388
ssize_t content_len = static_cast<ssize_t>(
53355389
res.content_length_ ? res.content_length_ : res.body.size());
53365390

5337-
ssize_t prev_first_pos = -1;
5338-
ssize_t prev_last_pos = -1;
5391+
std::vector<std::pair<ssize_t, ssize_t>> processed_ranges;
53395392
size_t overwrapping_count = 0;
53405393

53415394
// NOTE: The following Range check is based on '14.2. Range' in RFC 9110
@@ -5378,18 +5431,22 @@ inline bool range_error(Request &req, Response &res) {
53785431
return true;
53795432
}
53805433

5381-
// Ranges must be in ascending order
5382-
if (first_pos <= prev_first_pos) { return true; }
5434+
53835435

53845436
// Request must not have more than two overlapping ranges
5385-
if (first_pos <= prev_last_pos) {
5386-
overwrapping_count++;
5387-
if (overwrapping_count > 2) { return true; }
5437+
for (const auto &processed_range : processed_ranges) {
5438+
if (!(last_pos < processed_range.first || first_pos > processed_range.second)) {
5439+
overwrapping_count++;
5440+
if (overwrapping_count > 2) { return true; }
5441+
break; // Only count once per range
5442+
}
53885443
}
53895444

5390-
prev_first_pos = (std::max)(prev_first_pos, first_pos);
5391-
prev_last_pos = (std::max)(prev_last_pos, last_pos);
5445+
processed_ranges.emplace_back(first_pos, last_pos);
53925446
}
5447+
5448+
// After validation, coalesce overlapping ranges as per RFC 9110
5449+
coalesce_ranges(req.ranges, static_cast<size_t>(content_len));
53935450
}
53945451

53955452
return false;

‎test/test.cc‎

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3992,6 +3992,14 @@ TEST_F(ServerTest, GetStreamedWithRangeMultipart) {
39923992
EXPECT_EQ("267", res->get_header_value("Content-Length"));
39933993
EXPECT_EQ(false, res->has_header("Content-Range"));
39943994
EXPECT_EQ(267U, res->body.size());
3995+
3996+
// Check that both range contents are present
3997+
EXPECT_TRUE(res->body.find("bc\r\n") != std::string::npos);
3998+
EXPECT_TRUE(res->body.find("ef\r\n") != std::string::npos);
3999+
4000+
// Check that Content-Range headers are present for both ranges
4001+
EXPECT_TRUE(res->body.find("Content-Range: bytes 1-2/7") != std::string::npos);
4002+
EXPECT_TRUE(res->body.find("Content-Range: bytes 4-5/7") != std::string::npos);
39954003
}
39964004

39974005
TEST_F(ServerTest, GetStreamedWithTooManyRanges) {
@@ -4009,14 +4017,56 @@ TEST_F(ServerTest, GetStreamedWithTooManyRanges) {
40094017
EXPECT_EQ(0U, res->body.size());
40104018
}
40114019

4020+
TEST_F(ServerTest, GetStreamedWithOverwrapping) {
4021+
auto res = cli_.Get("/streamed-with-range",
4022+
{{make_range_header({{1, 4}, {2, 5}})}});
4023+
ASSERT_TRUE(res);
4024+
EXPECT_EQ(StatusCode::PartialContent_206, res->status);
4025+
EXPECT_EQ(5U, res->body.size());
4026+
4027+
// Check that overlapping ranges are coalesced into a single range
4028+
EXPECT_EQ("bcdef", res->body);
4029+
EXPECT_EQ("bytes 1-5/7", res->get_header_value("Content-Range"));
4030+
4031+
// Should be single range, not multipart
4032+
EXPECT_TRUE(res->has_header("Content-Range"));
4033+
EXPECT_EQ("text/plain", res->get_header_value("Content-Type"));
4034+
}
4035+
40124036
TEST_F(ServerTest, GetStreamedWithNonAscendingRanges) {
4013-
auto res = cli_.Get("/streamed-with-range?error",
4014-
{{make_range_header({{0, -1}, {0, -1}})}});
4037+
auto res = cli_.Get("/streamed-with-range",
4038+
{{make_range_header({{4, 5}, {0, 2}})}});
40154039
ASSERT_TRUE(res);
4016-
EXPECT_EQ(StatusCode::RangeNotSatisfiable_416, res->status);
4017-
EXPECT_EQ("0", res->get_header_value("Content-Length"));
4018-
EXPECT_EQ(false, res->has_header("Content-Range"));
4019-
EXPECT_EQ(0U, res->body.size());
4040+
EXPECT_EQ(StatusCode::PartialContent_206, res->status);
4041+
EXPECT_EQ(268U, res->body.size());
4042+
4043+
// Check that both range contents are present
4044+
EXPECT_TRUE(res->body.find("ef\r\n") != std::string::npos);
4045+
EXPECT_TRUE(res->body.find("abc\r\n") != std::string::npos);
4046+
4047+
// Check that Content-Range headers are present for both ranges
4048+
EXPECT_TRUE(res->body.find("Content-Range: bytes 4-5/7") != std::string::npos);
4049+
EXPECT_TRUE(res->body.find("Content-Range: bytes 0-2/7") != std::string::npos);
4050+
}
4051+
4052+
TEST_F(ServerTest, GetStreamedWithDuplicateRanges) {
4053+
auto res = cli_.Get("/streamed-with-range",
4054+
{{make_range_header({{0, 2}, {0, 2}})}});
4055+
ASSERT_TRUE(res);
4056+
EXPECT_EQ(StatusCode::PartialContent_206, res->status);
4057+
EXPECT_EQ(269U, res->body.size());
4058+
4059+
// Check that both duplicate range contents are present
4060+
size_t first_abc = res->body.find("abc\r\n");
4061+
EXPECT_TRUE(first_abc != std::string::npos);
4062+
size_t second_abc = res->body.find("abc\r\n", first_abc + 1);
4063+
EXPECT_TRUE(second_abc != std::string::npos);
4064+
4065+
// Check that Content-Range headers are present for both ranges
4066+
size_t first_range = res->body.find("Content-Range: bytes 0-2/7");
4067+
EXPECT_TRUE(first_range != std::string::npos);
4068+
size_t second_range = res->body.find("Content-Range: bytes 0-2/7", first_range + 1);
4069+
EXPECT_TRUE(second_range != std::string::npos);
40204070
}
40214071

40224072
TEST_F(ServerTest, GetStreamedWithRangesMoreThanTwoOverwrapping) {
@@ -4122,6 +4172,19 @@ TEST_F(ServerTest, GetWithRange4) {
41224172
EXPECT_EQ(std::string("fg"), res->body);
41234173
}
41244174

4175+
TEST_F(ServerTest, GetWithRange5) {
4176+
auto res = cli_.Get("/with-range", {
4177+
make_range_header({{0, 5}}),
4178+
{"Accept-Encoding", ""},
4179+
});
4180+
ASSERT_TRUE(res);
4181+
EXPECT_EQ(StatusCode::PartialContent_206, res->status);
4182+
EXPECT_EQ("6", res->get_header_value("Content-Length"));
4183+
EXPECT_EQ(true, res->has_header("Content-Range"));
4184+
EXPECT_EQ("bytes 0-5/7", res->get_header_value("Content-Range"));
4185+
EXPECT_EQ(std::string("abcdef"), res->body);
4186+
}
4187+
41254188
TEST_F(ServerTest, GetWithRangeOffsetGreaterThanContent) {
41264189
auto res = cli_.Get("/with-range", {{make_range_header({{10000, 20000}})}});
41274190
ASSERT_TRUE(res);

0 commit comments

Comments
 (0)