Fix #1772

yhirose · Feb 6, 2024 · 9d6f537 · 9d6f537
1 parent f06fd93
commit 9d6f537
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 22 deletions.
diff --git a/httplib.h b/httplib.h
@@ -961,7 +961,7 @@ class Server {
   bool parse_request_line(const char *s, Request &req) const;
   void apply_ranges(const Request &req, Response &res,
                     std::string &content_type, std::string &boundary) const;
-  bool write_response(Stream &strm, bool close_connection, const Request &req,
+  bool write_response(Stream &strm, bool close_connection, Request &req,
                       Response &res);
   bool write_response_with_content(Stream &strm, bool close_connection,
                                    const Request &req, Response &res);
@@ -4728,21 +4728,21 @@ serialize_multipart_formdata(const MultipartFormDataItems &items,
   return body;
 }
 
-inline bool normalize_ranges(Request &req, Response &res) {
-  ssize_t contant_len = static_cast<ssize_t>(
-      res.content_length_ ? res.content_length_ : res.body.size());
+inline bool range_error(Request &req, Response &res) {
+  if (!req.ranges.empty() && 200 <= res.status && res.status < 300) {
+    ssize_t contant_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;
-  size_t overwrapping_count = 0;
+    ssize_t prev_first_pos = -1;
+    ssize_t prev_last_pos = -1;
+    size_t overwrapping_count = 0;
 
-  if (!req.ranges.empty()) {
     // NOTE: The following Range check is based on '14.2. Range' in RFC 9110
     // 'HTTP Semantics' to avoid potential denial-of-service attacks.
     // https://www.rfc-editor.org/rfc/rfc9110#section-14.2
 
     // Too many ranges
-    if (req.ranges.size() > CPPHTTPLIB_RANGE_MAX_COUNT) { return false; }
+    if (req.ranges.size() > CPPHTTPLIB_RANGE_MAX_COUNT) { return true; }
 
     for (auto &r : req.ranges) {
       auto &first_pos = r.first;
@@ -4763,24 +4763,24 @@ inline bool normalize_ranges(Request &req, Response &res) {
       // Range must be within content length
       if (!(0 <= first_pos && first_pos <= last_pos &&
             last_pos <= contant_len - 1)) {
-        return false;
+        return true;
       }
 
       // Ranges must be in ascending order
-      if (first_pos <= prev_first_pos) { return false; }
+      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 false; }
+        if (overwrapping_count > 2) { return true; }
       }
 
       prev_first_pos = (std::max)(prev_first_pos, first_pos);
       prev_last_pos = (std::max)(prev_last_pos, last_pos);
     }
   }
 
-  return true;
+  return false;
 }
 
 inline std::pair<size_t, size_t>
@@ -5987,7 +5987,10 @@ inline bool Server::parse_request_line(const char *s, Request &req) const {
 }
 
 inline bool Server::write_response(Stream &strm, bool close_connection,
-                                   const Request &req, Response &res) {
+                                   Request &req, Response &res) {
+  // NOTE: `req.ranges` should be empty, otherwise it will be applied
+  // incorrectly to the error content.
+  req.ranges.clear();
   return write_response_core(strm, close_connection, req, res, false);
 }
 
@@ -6694,7 +6697,7 @@ Server::process_request(Stream &strm, bool close_connection,
     }
   }
 
-  // Rounting
+  // Routing
   auto routed = false;
 #ifdef CPPHTTPLIB_NO_EXCEPTIONS
   routed = routing(req, res, strm);
@@ -6731,21 +6734,23 @@ Server::process_request(Stream &strm, bool close_connection,
   }
 #endif
   if (routed) {
-    if (detail::normalize_ranges(req, res)) {
-      if (res.status == -1) {
-        res.status = req.ranges.empty() ? StatusCode::OK_200
-                                        : StatusCode::PartialContent_206;
-      }
-      return write_response_with_content(strm, close_connection, req, res);
-    } else {
+    if (res.status == -1) {
+      res.status = req.ranges.empty() ? StatusCode::OK_200
+                                      : StatusCode::PartialContent_206;
+    }
+
+    if (detail::range_error(req, res)) {
       res.body.clear();
       res.content_length_ = 0;
       res.content_provider_ = nullptr;
       res.status = StatusCode::RangeNotSatisfiable_416;
       return write_response(strm, close_connection, req, res);
     }
+
+    return write_response_with_content(strm, close_connection, req, res);
   } else {
     if (res.status == -1) { res.status = StatusCode::NotFound_404; }
+
     return write_response(strm, close_connection, req, res);
   }
 }

diff --git a/test/test.cc b/test/test.cc
@@ -2166,6 +2166,11 @@ class ServerTest : public ::testing::Test {
                   EXPECT_EQ("4", req.get_header_value("Content-Length"));
                   res.set_content(req.body, "application/octet-stream");
                 })
+        .Get("/issue1772",
+             [&](const Request & /*req*/, Response &res) {
+               res.status = 401;
+               res.set_header("WWW-Authenticate", "Basic realm=123456");
+             })
 #if defined(CPPHTTPLIB_ZLIB_SUPPORT) || defined(CPPHTTPLIB_BROTLI_SUPPORT)
         .Get("/compress",
              [&](const Request & /*req*/, Response &res) {
@@ -3139,6 +3144,12 @@ TEST_F(ServerTest, GetWithRangeMultipartOffsetGreaterThanContent) {
   EXPECT_EQ(StatusCode::RangeNotSatisfiable_416, res->status);
 }
 
+TEST_F(ServerTest, Issue1772) {
+  auto res = cli_.Get("/issue1772", {{make_range_header({{1000, -1}})}});
+  ASSERT_TRUE(res);
+  EXPECT_EQ(StatusCode::Unauthorized_401, res->status);
+}
+
 TEST_F(ServerTest, GetStreamedChunked) {
   auto res = cli_.Get("/streamed-chunked");
   ASSERT_TRUE(res);