Skip to content
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.

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

range: New http_GetContentRange() semantics #4091

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dridi
Copy link
Member

@dridi dridi commented Mar 27, 2024

This function returns the content length and puts range indices in lo and hi arguments. A content length of -1 used to be interpreted as something to ignore, but there is a case where the content length may be unknown.

Since we can't represent a zero-length range, because both bounds are inclusive, zero now denotes the lack of content-range header.

Unknown range units are treated as errors as they wouldn't pass the busyobj check for the range header, even for pass transactions. The only way to use "extension" range units is to turn http_range_support off.

The calling convention for http_GetContentRange() remains otherwise the same, and for good measure http_GetContentLength() received a similar description.

Fixes #4089

dridi added 2 commits March 27, 2024 10:27
This function returns the content length and puts range indices in
lo and hi arguments. A content length of -1 used to be interpreted
as something to ignore, but there is a case where the content length
may be unknown.

Since we can't represent a zero-length range, because both bounds are
inclusive, zero now denotes the lack of content-range header.

Unknown range units are treated as errors as they wouldn't pass the
busyobj check for the range header, even for pass transactions. The
only way to use "extension" range units is to turn http_range_support
off.

The calling convention for http_GetContentRange() remains otherwise
the same, and for good measure http_GetContentLength() received a
similar description.

Fixes varnishcache#4089
Copy link
Member

@nigoroll nigoroll left a comment

Choose a reason for hiding this comment

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

lgtm

@dridi
Copy link
Member Author

dridi commented May 26, 2024

I haven't had a chance to look at your suggestion from #4089 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

503 Invalid content-range header on partial response with start-end/*
2 participants