range_error function always return true!

[bobhyun]

If there's Range in the Request header, it always failed with 416 status.

In my case, using Visual Studio 2022 C++, the value -1 is the same as SIZE_MAX
in range_error function,
ssize_t prev_first_pos = -1;
ssize_t prev_last_pos = -1;
The comparision with these values always be the same or less.

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; // The value -1 is the same as SIZE_MAX
    ssize_t prev_last_pos = -1;
    size_t overwrapping_count = 0;

    // 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 true; }

    for (auto &r : req.ranges) {
      auto &first_pos = r.first;
      auto &last_pos = r.second;

      if (first_pos == -1 && last_pos == -1) {
        first_pos = 0;
        last_pos = contant_len;
      }

      if (first_pos == -1) {
        first_pos = contant_len - last_pos;
        last_pos = contant_len - 1;
      }

      if (last_pos == -1) { last_pos = contant_len - 1; }

      // Range must be within content length
      if (!(0 <= first_pos && first_pos <= last_pos &&
            last_pos <= contant_len - 1)) {
        return true;
      }

      // Ranges must be in ascending order
      if (first_pos <= prev_first_pos) { return true; } // so this condition always true

      // Request must not have more than two overlapping ranges
      if (first_pos <= prev_last_pos) {  // it's the same as here
        overwrapping_count++;
        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 false;
}

[yhirose]

@bobhyun thanks for the report. Could you show me your code calling range_error in your code? I would like to see how it gets called.

[bobhyun]

It is not called directly. This call is made by nginx.
Instead of showing the code, I'll explain it below.

• Overview of my case

1. Web browser calls GET /storage/x/0/0/0/0/721.mp4
2. Then, according to nginx.conf, nginx proxy pass GET /api/auth to myapp (using cpp-httplib).
   1. The Range header made by nginx is bytes=0-, I'm not sure this syntax is valid or not.
   2. cpp-httplib parses it as (0, 18446744073709551615), the second value assigned as SIZE_MAX.
   3. myapp returns 200 status.
   4. Then range_error function of cpp-httplib always return true.
   5. The final status is replaced with 416 by following code block.

      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);
      }

3. nginx return 500 status, server internal error.

flowchart LR

browser-->|GET /storage/x/0/0/0/0/721.mp4|nginx-->|GET /api/auth|myapp
nginx-->|static resources|/storage/x/0/0/0/0/721.mp4

Loading

• nginx.conf

http {
  server {
    listen 80;

    location / {
      root "/var/wwwroot";
      index index.html;
      add_header 'Access-Control-Allow-Origin' '*';
    }

    location ~* ^/api(.*$) {
      proxy_set_header X-Original-URI $request_uri;
      proxy_pass http://127.0.0.1:65530;
      proxy_set_header X-Remote $remote_addr;
      proxy_set_header X-Host $http_host;
      proxy_set_header Authorization $http_authorization;

      add_header 'Access-Control-Allow-Origin' '*' always;
      add_header 'Access-Control-Allow-Credentials' 'true';
      add_header 'Access-Control-Allow-Headers' 'Authorization,Accept,Origin,DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';  # this makes Range header
      add_header 'Access-Control-Allow-Methods' 'GET,POST,OPTIONS,PUT,DELETE,PATCH';

      # Preflighted requests
      if ($request_method = OPTIONS) {
        return 204;
      }
    }
  }

  # static resources
  location /storage {
    # authentication is required before accessing the resources 
    # this makes call my app using cpp-httplib
    auth_request /api/auth; 

    proxy_set_header X-Remote $remote_addr;	
    proxy_set_header X-Host $http_host;
    proxy_set_header Authorization $http_authorization;

    add_header 'Access-Control-Allow-Origin' '*' always;
    add_header 'Access-Control-Allow-Credentials' 'true';
    add_header 'Access-Control-Allow-Headers' 'Authorization,Accept,Origin,DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
    add_header 'Access-Control-Allow-Methods' 'GET,POST,OPTIONS,PUT,DELETE,PATCH';

    # Preflighted requests
    if ($request_method = OPTIONS) {
      return 204;
    }
	
    location ~ (\/storage\/)(.*)\.(mp4|mov)$ {
      alias /videos$2;
    }
  }
}

• request headers
  

• parsed range value
  

• modified range_error function
  I'm not sure this is right way to do, but it works after checking initial value(-1).

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;

    // 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 true; }

    for (auto &r : req.ranges) {
      auto &first_pos = r.first;
      auto &last_pos = r.second;

      if (first_pos == -1 && last_pos == -1) {
        first_pos = 0;
        last_pos = contant_len;
      }

      if (first_pos == -1) {
        first_pos = contant_len - last_pos;
        last_pos = contant_len - 1;
      }

      if (last_pos == -1) { last_pos = contant_len - 1; }

      // Range must be within content length
      if (!(0 <= first_pos && first_pos <= last_pos &&
            last_pos <= contant_len - 1)) {
        return true;
      }

      // Ranges must be in ascending order
      if (prev_first_pos != -1 && // Fixed: checking initial value
        first_pos <= prev_first_pos) { return true; }

      // Request must not have more than two overlapping ranges
      if (prev_last_pos != -1 && // Fixed: checking initial value
        first_pos <= prev_last_pos) {
        overwrapping_count++;
        if (overwrapping_count > 2) { return true; }
      }

      // Fixed: checking initial values
      prev_first_pos = (prev_first_pos == -1) ? first_pos : (std::max)(prev_first_pos, first_pos); 
      prev_last_pos = (prev_last_pos == -1) ? last_pos : (std::max)(prev_last_pos, last_pos);
    }
  }

  return false;
}

Please check.
Thanks.

[yhirose]

@bobhyun thanks for the details.

modified range_error function
I'm not sure this is right way to do, but it works after checking initial value(-1).

If you already have a fix for this issue, could you send a pull request? Thanks!

[bobhyun]

Ok , I sent PR, pls confirm.
#1947

[yhirose]

@bobhyun could you explain why your range.first and range.second are unsigned __int64?

They should be ssize_t (__int64 or long).
https://github.com/yhirose/cpp-httplib/blob/10d68cff5081d8c7b7cae74000e0c423107dc7b8/httplib.h#L160-L165https://github.com/yhirose/cpp-httplib/blob/996acc52537a62435769bf1f873ad0504d6cf620/httplib.h#L604

Also I added a unit test for this issue, and the current httplib.h has no problem on Windows, Mac and Ubuntu.

cpp-httplib/test/test.cc

Lines 3703 to 3711 in 10d68cf

	TEST_F(ServerTest, GetRangeWithZeroToInfinite) {
	auto res = cli_.Get("/with-range", {{"Range", "bytes=0-"}});
	ASSERT_TRUE(res);
	EXPECT_EQ(StatusCode::PartialContent_206, res->status);
	EXPECT_EQ("7", res->get_header_value("Content-Length"));
	EXPECT_EQ(true, res->has_header("Content-Range"));
	EXPECT_EQ("bytes 0-6/7", res->get_header_value("Content-Range"));
	EXPECT_EQ(std::string("abcdefg"), res->body);
	}