Add Partial Content Delivery (+tests) #98
Conversation
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.
|
I am still active on GitHub and I saw your PR on my fork, and I wanted to get to it this week-end but you beat me to it. It's true that lately I haven't as much time as I wished I had, because you know, life'n'stuff. Anyway I'm grateful that you're taking care of this now, I was starting to feel guilty about not implementing the tests to have this merged. About the tests themselves, if I were you I would also add some tests for the errors as well. (requesting a byterange were the minimum is higher than the file total, requesting a from -1 in the headers, things like that... but still, great work and thank you again. |
|
I should have been a bit more patient, I know how busy things can get :c I'll add a few tests for invalid requests, that's a good point. |
Also fixed a edge case bug where a FromTo range ending out of bounds was not treated as a From range (See https://tools.ietf.org/html/rfc7233, section 2.1).
src/partial_file.rs
Outdated
| } | ||
|
|
||
| /// Panics if the file doesn't exist | ||
| pub fn from_path<P: AsRef<Path>, Range>(path: P, range: Range) -> PartialFile |
There was a problem hiding this comment.
Is there a specific reason this doesn't return a Result<PartialFile, Std::io::Error>?
There was a problem hiding this comment.
The reason was originally that the file existence was already checked beforehand everytime before this method was called (via metadata). However now that I think of it this is pretty racy: if the file is deleted between the check and the file being delivered (which is unlikely but can still happen), then it will panic.
It would be better to return a Result<_, io::Error> there indeed.
|
Any update regarding the possibility/timeline of this being merged? |
|
Hey guys, this works great, we're using this pull request in a video renderer. Should be good to go in. Cheers. |
|
Ping! |
|
Maybe @untitaker would be willing to review/merge this? |
|
At this point I would recommend migrating to a web framework that is still maintained. |
|
Is this not maintained? A deprecation warning would be nice |
|
Yeah, Iron hasn't had a commit in a very long time. I think everyone uses Rocket, Actix-Web or some other frameworks now. |
this commit adds partial content delivery support borrowing @Cobrand's PR iron/staticfile#98 commit motivated by static-web-server/static-web-server#15
This is a follow up to #95. @Cobrand doesn't seem to be active on Github anymore but his pending PR adding partial content responses is very valuable to me. I added unit tests to his code, please let me know if anything else needs to happen for this to be merged!