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

Content-Range is incorrectly specified. #778

Merged
merged 2 commits into from
May 10, 2022
Merged

Content-Range is incorrectly specified. #778

merged 2 commits into from
May 10, 2022

Conversation

yuki2006
Copy link
Contributor

@yuki2006 yuki2006 commented May 9, 2022

For example, a file of 22709217 bytes content is

should be 0-22709216/22709217 instead of 0-22709217/22709217.

Chrome will not handle this correctly and videos and other files will not play.

image

RFC:
https://httpwg.org/specs/rfc7233.html#header.content-range

@fsouza fsouza added bug and removed bug labels May 9, 2022
@fsouza
Copy link
Owner

fsouza commented May 9, 2022

Thanks for contributing. Would you be able to add a test that fails with the previous code and passes with the fix. Ideally something using the SDK rather than asserting on the value of the headers.

@yuki2006
Copy link
Contributor Author

@fsouza thx comment.

I don't know how to use the SDK, but I wrote some unit tests.

Please feel free to make any changes.

@fsouza fsouza merged commit ae04303 into fsouza:main May 10, 2022
@@ -686,7 +687,7 @@ func (s *Server) handleRange(obj Object, r *http.Request) (ranged bool, start in
if end > int64(len(obj.Content)) {
end = int64(len(obj.Content))
}
return true, start, end, obj.Content[start:end]
return true, start, end - 1, obj.Content[start:end]
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be:

return true, start, end - 1, obj.Content[start:end-1]

?

Copy link
Owner

Choose a reason for hiding this comment

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

Nope, in the slice expression the upper bound is non-inclusive: https://go.dev/ref/spec#Slice_expressions

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

Successfully merging this pull request may close these issues.

3 participants