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

Author: yhirose

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<ssize_t>(content_length);
+    }
+
+    if (first_pos == -1) {
+      first_pos = static_cast<ssize_t>(content_length) - last_pos;
+      last_pos = static_cast<ssize_t>(content_length) - 1;
+    }
+
+    if (last_pos == -1 || last_pos >= static_cast<ssize_t>(content_length)) {
+      last_pos = static_cast<ssize_t>(content_length) - 1;
+    }
+
+    // Skip invalid ranges
+    if (!(0 <= first_pos && first_pos <= last_pos &&
+          last_pos < static_cast<ssize_t>(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<ssize_t>(
         res.content_length_ ? res.content_length_ : res.body.size());
 
-    ssize_t prev_first_pos = -1;
-    ssize_t prev_last_pos = -1;
+    std::vector<std::pair<ssize_t, ssize_t>> 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<size_t>(content_len));
   }
 
   return false;

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, GetStreamedWithOverwrapping) {
+  auto res =
+      cli_.Get("/streamed-with-range", {{make_range_header({{1, 4}, {2, 5}})}});
+  ASSERT_TRUE(res);
+  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?error",
-                      {{make_range_header({{0, -1}, {0, -1}})}});
+  auto res =
+      cli_.Get("/streamed-with-range", {{make_range_header({{4, 5}, {0, 2}})}});
   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(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);