Skip to content

Add partial support for the 'Range' header when serving static files #290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
atheriel wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add partial support for the 'Range' header when serving static files #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ httpuv 1.5.4.9000

* Allow responses to omit `body` (or set it as `NULL`) to avoid sending a body or setting the `Content-Length` header. This is intended for use with HTTP 204/304 responses. (#288)

* Resolved #259: Static files now support common range requests. (#290)

httpuv 1.5.4
============

Expand Down
22 changes: 20 additions & 2 deletions src/filedatasource-unix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ FileDataSourceResult FileDataSource::initialize(const std::string& path, bool ow
return FDS_ISDIR;
}

_length = info.st_size;
_payloadSize = info.st_size;
_fsize = info.st_size;

if (owned && unlink(path.c_str())) {
// Print this (on either main or background thread), since we're not
Expand All @@ -49,7 +50,24 @@ FileDataSourceResult FileDataSource::initialize(const std::string& path, bool ow
}

uint64_t FileDataSource::size() const {
return _length;
return _payloadSize;
}

uint64_t FileDataSource::fileSize() const {
return _fsize;
}

bool FileDataSource::setRange(uint64_t start, uint64_t end) {
ASSERT_BACKGROUND_THREAD()
if (end > _fsize || end < start) {
return false;
}
if (lseek(_fd, start, SEEK_SET) < 0) {
err_printf("Error in lseek: %d\n", errno);
return false;
}
_payloadSize = end - start + 1;
return true;
}

uv_buf_t FileDataSource::getData(size_t bytesDesired) {
Expand Down
25 changes: 23 additions & 2 deletions src/filedatasource-win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,39 @@ FileDataSourceResult FileDataSource::initialize(const std::string& path, bool ow
}


if (!GetFileSizeEx(_hFile, &_length)) {
if (!GetFileSizeEx(_hFile, &_fsize)) {
CloseHandle(_hFile);
_hFile = INVALID_HANDLE_VALUE;
_lastErrorMessage = "Error retrieving file size for " + path + ": " + toString(GetLastError()) + "\n";
return FDS_ERROR;
}
_payloadSize = _fsize.QuadPart;

return FDS_OK;
}

uint64_t FileDataSource::size() const {
return _length.QuadPart;
return _payloadSize;
}

uint64_t FileDataSource::fileSize() const {
return _fsize.QuadPart;
}

bool FileDataSource::setRange(uint64_t start, uint64_t end) {
ASSERT_BACKGROUND_THREAD()
if (end > _fsize.QuadPart || end < start) {
return false;
}
LARGE_INTEGER position = {0};
position.QuadPart = start;
// When using FILE_BEGIN, position is interpreted as an unsigned value.
if (!SetFilePointerEx(_hFile, position, NULL, FILE_BEGIN)) {
err_printf("Error in SetFilePointerEx: %d\n", GetLastError());
return false;
}
_payloadSize = _fsize.QuadPart - start + 1;
return true;
}

uv_buf_t FileDataSource::getData(size_t bytesDesired) {
Expand Down
11 changes: 9 additions & 2 deletions src/filedatasource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ enum FileDataSourceResult {
class FileDataSource : public DataSource {
#ifdef _WIN32
HANDLE _hFile;
LARGE_INTEGER _length;
uint64_t _payloadSize;
LARGE_INTEGER _fsize;
#else
int _fd;
off_t _length;
off_t _payloadSize;
off_t _fsize;
#endif
std::string _lastErrorMessage;

Expand All @@ -31,7 +33,12 @@ class FileDataSource : public DataSource {
}

FileDataSourceResult initialize(const std::string& path, bool owned);
// Total size of the data in this source, in bytes, after accounting for file
// size and range settings.
uint64_t size() const;
// Total size of the underlying file in this source, in bytes.
uint64_t fileSize() const;
bool setRange(uint64_t start, uint64_t end);
uv_buf_t getData(size_t bytesDesired);
void freeData(uv_buf_t buffer);
// Get the mtime of the file. If there's an error, return 0.
Expand Down
76 changes: 76 additions & 0 deletions src/webapplication.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,70 @@ boost::shared_ptr<HttpResponse> RWebApplication::staticFileResponse(
}
}

// Check the HTTP Range header, if it has been specified.
Copy link
Collaborator

@wch wch Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this stuff is sufficiently complex that it makes sense to pull it out into a separate function.

Copy link
Contributor Author

@atheriel atheriel Dec 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit awkward to do so, since there are currently three variables set in this section.

Copy link
Collaborator

@wch wch Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be simpler to review and maintain the code if the range header parsing was a separate function. Right now the string parsing is mixed in with the logic for setting range and generating error responses; if the string parsing were done in a separate step, that would make the code more manageable. Even if the code is correct now, I would feel uneasy about touching it in the future.

Something like this would be easier to review and maintain:

std::pair<int64_t, int64_t> parseRangeHeader(std::string rangeHeader) {
  if (rangeHeader.find("bytes=") != 0) {
    return std::make_pair<int64_t, int64_t>(-1, -1);
  }

  rangeHeader = rangeHeader.substr(6);
  int sep_offset = rangeHeader.find("-");
  if (sep_offset == std::string::npos) {
    return std::make_pair<int64_t, int64_t>(-1, -1);
  }

  std::string start_str = rangeHeader.substr(0, sep_offset+1);
  std::string end_str   = rangeHeader.substr(sep_offset+1);
  
  int64_t start;
  int64_t end;

  // TODO: Do number parsing here; return (-1, -1) if either start or end are not
  // well-formed.
  
  return std::make_pair<int64_t, int64_t>(start, end);
}

The number parsing could be done with strtoull() as you currently have, or with sscanf(). (There are a number of details to get right with those functions and I don't have the time right now to try to do it, so I didn't implement that part.) It might make sense to extract it out into a function.

//
// We only support a limited form of the HTTP Range header (something like
// 'bytes=(\\d+)-(\\d+)?'), so some client requests can't be fulfilled and we
// need to fall back on a 200 OK instead.
bool hasRange = false;
uint64_t start = 0;
uint64_t end = pDataSource->fileSize() - 1;
if (pRequest->hasHeader("Range")) {
std::string rangeHeader = pRequest->getHeader("Range");
size_t cursor = rangeHeader.find("bytes=");
size_t sep = rangeHeader.find("-", cursor + 6);
if (cursor != std::string::npos && sep != std::string::npos && sep != cursor + 6) {
// At this point we can be sure we have something like 'bytes=([^-]+)-.*'
// and we can try parsing the range itself.
char *ptr = ((char *) rangeHeader.c_str()) + cursor + 6; // Skip over 'bytes='.
char *endptr = ptr;
// strtoull() will return 0 when it can't find a number or ULLONG_MAX on
// overflow. Since we can't have a file larger than ULLONG_MAX anyway,
// this is OK because we'll just return HTTP 416 below.
start = strtoull(ptr, &endptr, 10);
if (start == 0 && (endptr == ptr || *endptr != '-')) {
// Either there was no number at all or there was garbage *after* the
// number but before the '-' separator. This is invalid range syntax
// from the client.
return error_response(pRequest, 400);
}
if (start >= pDataSource->fileSize()) {
boost::shared_ptr<HttpResponse> pResponse = error_response(pRequest, 416);
pResponse->headers().push_back(
std::make_pair("Content-Range", "bytes */" + toString(pDataSource->fileSize()))
);
return pResponse;
}
ptr = endptr + 1; // Skip the '-'.
if (*ptr != '\0') {
end = strtoull(ptr, &endptr, 10);
if (*endptr != '\0' && *endptr != ',') {
// We hit a non-digit, non-multirange character at some point.
return error_response(pRequest, 400);
}
if (end < start) {
return error_response(pRequest, 400);
}
ptr = endptr;
}
// A comma indicates we're parsing a multipart range, which we don't
// support. So we need to fallback on 200 behaviour instead when we detect
// one.
if (*ptr != ',') {
if (end > pDataSource->fileSize() - 1) {
// The client might not know the size, so the range end is supposed to
// be redefined to be the last byte in this case instead of issuing an
// error. This also catches overflow in strtoull() above, which would
// be OK.
//
// See: https://tools.ietf.org/html/rfc7233#section-2.1
end = pDataSource->fileSize() - 1;
}
hasRange = pDataSource->setRange(start, end);
}
}
}

// ==================================
// Create the HTTP response
// ==================================
Expand All @@ -585,6 +649,10 @@ boost::shared_ptr<HttpResponse> RWebApplication::staticFileResponse(
status_code = 304;
}

if (hasRange) {
status_code = 206;
}

boost::shared_ptr<HttpResponse> pResponse = boost::shared_ptr<HttpResponse>(
new HttpResponse(pRequest, status_code, getStatusDescription(status_code), pDataSource2),
auto_deleter_background<HttpResponse>
Expand Down Expand Up @@ -622,6 +690,14 @@ boost::shared_ptr<HttpResponse> RWebApplication::staticFileResponse(
respHeaders.push_back(std::make_pair("Content-Length", toString(pDataSource->size())));
respHeaders.push_back(std::make_pair("Content-Type", content_type));
respHeaders.push_back(std::make_pair("Last-Modified", http_date_string(pDataSource->getMtime())));
respHeaders.push_back(std::make_pair("Accept-Ranges", "bytes"));
}

if (status_code == 206) {
respHeaders.push_back(std::make_pair(
"Content-Range",
"bytes " + toString(start) + "-" + toString(end) + "/" + toString(pDataSource2->fileSize())
));
}

return pResponse;
Expand Down
137 changes: 137 additions & 0 deletions tests/testthat/test-static-paths.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,3 +808,140 @@ test_that("Paths with non-ASCII characters", {
expect_identical(r$status_code, 200L)
expect_identical(r$content, file_content)
})


test_that("Range headers", {
s <- startServer("127.0.0.1", randomPort(),
list(
staticPaths = list(
"/" = staticPath(test_path("apps/content"))
)
)
)
on.exit(s$stop())

file_size <- file.info(test_path("apps/content/mtcars.csv"))$size
file_content <- raw_file_content(test_path("apps/content/mtcars.csv"))

# Malformed Range headers.
h <- new_handle()
handle_setheaders(h, "Range" = "bytes=500-100")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 400L)

handle_setheaders(h, "Range" = "bytes=notanumber-500")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 400L)

handle_setheaders(h, "Range" = "bytes=0-notanumber")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 400L)

handle_setheaders(h, "Range" = "bytes=500-100, 1000-")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 400L)

# Range we can't handle (with overflow).
handle_setheaders(h, "Range" = "bytes=2147483648-2147483649")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 416L)

# Range starts beyond file length.
handle_setheaders(h, "Range" = "bytes=10000-20000")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 416L)
expect_identical(
parse_headers_list(r$headers)$`content-range`,
sprintf("bytes */%d", file_size)
)

# Range starts *exactly* one byte beyond file size.
handle_setheaders(h, "Range" = sprintf("bytes=%d-20000", file_size))
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 416L)

# Multipart ranges, which we just ignore.
handle_setheaders(h, "Range" = "bytes=0-500, 1000-")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 200L)

# Suffix length ranges, which we also ignore.
handle_setheaders(h, "Range" = "bytes=-500")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 200L)

# Garbage Range header, which we also ignore.
handle_setheaders(h, "Range" = "bytes=500")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 200L)

handle_setheaders(h, "Range" = "invalid")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 200L)

# Start of a file.
handle_setheaders(h, "Range" = "bytes=0-499")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 206L)
expect_identical(
parse_headers_list(r$headers)$`content-range`,
sprintf("bytes 0-499/%d", file_size)
)
expect_equal(length(r$content), 500)
expect_identical(r$content, file_content[1:500])

# Exactly 1 byte.
handle_setheaders(h, "Range" = "bytes=0-0")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 206L)
expect_identical(
parse_headers_list(r$headers)$`content-range`,
sprintf("bytes 0-0/%d", file_size)
)
expect_equal(length(r$content), 1)
expect_identical(r$content, file_content[1])

# Exactly all bytes.
handle_setheaders(h, "Range" = sprintf("bytes=0-%d", file_size - 1))
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 206L)
expect_identical(
parse_headers_list(r$headers)$`content-range`,
sprintf("bytes 0-%d/%d", file_size - 1, file_size)
)
expect_equal(length(r$content), file_size)
expect_identical(r$content, file_content)

# End of a file.
handle_setheaders(h, "Range" = "bytes=1000-")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 206L)
expect_identical(
parse_headers_list(r$headers)$`content-range`,
sprintf("bytes 1000-%d/%d", file_size - 1, file_size)
)
expect_equal(length(r$content), (file_size - 1000))
expect_identical(r$content, file_content[1001:file_size])

# End of a smaller file than expected.
handle_setheaders(h, "Range" = "bytes=1000-2000")
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 206L)
expect_identical(
parse_headers_list(r$headers)$`content-range`,
sprintf("bytes 1000-%d/%d", file_size - 1, file_size)
)
expect_equal(length(r$content), (file_size - 1000))
expect_identical(r$content, file_content[1001:file_size])

# The last 1 byte.
handle_setheaders(h, "Range" = sprintf("bytes=%d-2000", file_size - 1))
r <- fetch(local_url("/mtcars.csv", s$getPort()), h)
expect_identical(r$status_code, 206L)
expect_identical(
parse_headers_list(r$headers)$`content-range`,
sprintf("bytes %d-%d/%d", file_size - 1, file_size - 1, file_size)
)
expect_equal(length(r$content), 1)
expect_identical(r$content, file_content[file_size])
})