Skip to content
This repository has been archived by the owner on Mar 27, 2023. It is now read-only.

Range support off-by-one error #39

Closed
fasterthanlime opened this issue Jul 30, 2020 · 2 comments · Fixed by #40
Closed

Range support off-by-one error #39

fasterthanlime opened this issue Jul 30, 2020 · 2 comments · Fixed by #40
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@fasterthanlime
Copy link

When the request header Range is set to bytes=0-, the last byte is missing from the response.

Reproduction steps

Create ohno.txt with contents:

$ printf "life" > ohno.txt
$ sfz -Z &
$ curl -s http://localhost:5000/ohno.txt; echo
[30/Jul/2020 18:08:59] "GET /ohno.txt" - 200 OK
life
$ curl -s http://localhost:5000/ohno.txt -H "Range: bytes=0-"; echo
[30/Jul/2020 18:09:16] "GET /ohno.txt" - 206 Partial Content
lif

Don't use echo to create ohno.txt, it would put a newline in it. Some text
editors also automatically insert newlines

Additional notes

The HTTP Range header is annoying to implement correctly, I recommend writing a bunch of tests around it!

I think just doing end - start + 1 here would fix it:

.take(end - start)

...but it's the end of the day and I'm tired, so.

@luser
Copy link

luser commented Jul 30, 2020

hyper's ByteRangeSpec seems really useful, but the conversion to the Content-Range header using to_satisfiable_range puts the information in a less useful form. Seems like it'd be less error-prone to simply validate that the range is satisfiable, and then pass the ByteRangeSpec down into send_file_with_range?

@weihanglo weihanglo added bug Something isn't working help wanted Extra attention is needed labels Aug 28, 2020
weihanglo added a commit that referenced this issue Aug 28, 2020
weihanglo added a commit that referenced this issue Aug 28, 2020
weihanglo added a commit that referenced this issue Aug 28, 2020
@fasterthanlime
Copy link
Author

Thanks for the fix @weihanglo! ✨

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants