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

Handling Range header #1744

Closed
SuperCuber opened this issue Jul 4, 2021 · 1 comment
Closed

Handling Range header #1744

SuperCuber opened this issue Jul 4, 2021 · 1 comment
Labels
duplicate This issue or pull request already exists

Comments

@SuperCuber
Copy link

Is your feature request motivated by a concrete problem? Please describe.

I'm opening this as a continuation of #439 (I don't really know the github etiquette for reopening someone else's issue so sorry if I should've commented there)

The conclusion there was this:

I don't think Rocket should handle range requests transparently, by default, for everything. As far as I know, no web frameworks or servers do this for every incoming request/response except when handling static files. Indeed, I do thing Rocket should handle range requests by default when serving static files. For this reason, I'm going to decline this particular request but note that we should implement automatic range request/response handling in a supplied static file handling library. Let's track that in #239.

Looking at #239 and the discussion after this, it seems the only result of that "tracking" was a library that @Ekranos created - here it is.
The crate was last updated 3 years ago and the repository no longer exists. Even if the crate works today, I think it's not acceptable for that to be the only implementation of Range support.

Why this feature can't or shouldn't live outside of Rocket
I think partial support is very important - I don't know if it should be inside rocket or in another related crate, but it should be supported.

Ideal Solution
I think this should either be implemented in <File as Responder>::respond_to or there should be a PartialFile struct similar to NamedFile (bad name but you get my idea).
Perhaps there could be a wrapping responder for this? Although that's probably not possible for example for streaming responses.

Alternatives Considered
Currently I'm using warp but I'm very enthusiastic about switching my project to Rocket - this is basically the only "turn-off" for me right now. I'm not very experienced with Rocket but if I had to implement this I would probably make my own Responder and copy-paste most of warp's code for it.

Additional Context
The only popular rust web server I've seen that implements this is warp - code linked above.

@SuperCuber SuperCuber added the request Request for new functionality label Jul 4, 2021
@SergioBenitez
Copy link
Member

This is #95. I'd love to do this, but it needs to be done correctly and with all security implications considered. The linked warp code does neither, and so we cannot transfer it wholesale.

@SergioBenitez SergioBenitez added duplicate This issue or pull request already exists and removed request Request for new functionality labels Jul 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants