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

Add Partial Content Delivery #95

Closed
wants to merge 1 commit into from
Closed

Conversation

Cobrand
Copy link

@Cobrand Cobrand commented Feb 7, 2017

This commits adds Partial Content Delivery, as asked in issue #93. This
enables the "Accept-Ranges" header on all files delivered via
static. It is possible to ask only a certain portion of bytes like
explicitely described in RFC 7233. Of the 3 ways to ask for a range of
bytes (byte x to byte y, everything from byte x to end of file, last y
bytes from end of file), every one of them is implemented, but if a
request is to ask multi ranges in a same request, this implementation
will only account the first range.

---- end of commit ----

Quite a big PR I have here, I hope it's not too much hassle. If you think there are better ways to structure my code, please tell me I'll fix that right away ! What is good coding to me might not be for iron, and I'm fine with that !

So basically it closes partially #93 . It works for a single range, but multiple ranges are not yet supported (that's why it doesn't close #93 totally in my opinion). Video seeking works fine though, in firefox, chrome or mpv, or whatever.

I'm missing tests as well, and I'd like support on that part. I must say that I'm not familiar with iron_test and your way of testing, and I don't really know what to test either. So if someone could comment on that part, that'd be nice.


This change is Reviewable

This commits adds Partial Content Delivery, as asked in issue iron#93. This
enables the "Accept-Ranges" header on all files delivered via
static. It is possible to ask only a certain portion of bytes like
explicitely described in RFC 7233. Of the 3 ways to ask for a range of
bytes (byte x to byte y, everything from byte x to end of file, last y
bytes from end of file), every one of them is implemented, but if a
request is to ask multi ranges in a same request, this implementation
will only account the first range.
Copy link
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

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

One small comment, also please add tests.


};
if let Some(range) = range {
let content_range = ContentRange(ContentRangeSpec::Bytes {
Copy link
Member

Choose a reason for hiding this comment

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

Setting content range can be done at the very beginning to deduplicate the code here. If the file length is None we can then set RangeNotSatisfiable and return early.

@Hoverbear
Copy link

Anything we can do to help support you in landing this PR?

@Cobrand
Copy link
Author

Cobrand commented Apr 15, 2017

If someone could help me on the test part of this PR, that's be great. I don't ask for anyone else to implement the tests themselves (although that'd be great), but if someone could give me a hint as to what to use in iron_test precisely, and which use cases require tests for this PR to land, that'd be a huge help.

@untitaker
Copy link
Member

Right, so I think you should be able to use ProjectBuilder like in the other tests (e.g. here: https://github.com/iron/staticfile/blob/master/tests/static.rs#L16) to create an app, a file and send a request that only accesses a range of that file and asserts that it only gets the range back.

@Hoverbear
Copy link

It looks like #98 (comment) follows up on this so closing in favor of it.

@Hoverbear Hoverbear closed this Jul 9, 2017
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.

Support HTTP Byte Serving via Ranges header
3 participants