tide icon indicating copy to clipboard operation
tide copied to clipboard

Serve content with support for HTTP conditionals and range requests

Open ririsoft opened this issue 4 years ago • 4 comments

This PR introduces new utilities to serve content with support for HTTP conditionals as defined in RFC7232. It will allow to return 304 Not Modified (for instance) and handle HTTP Range requests (RFC7233) transparently.

Motivation and Context

Implementing RFC7232 and RFC7233 is not trivial: I believe it should be implemented at Tide level instead of leaving the users implementing it themselves. At the time I checked a few weeks ago Rocket, warp and actix all had limited (if any) support for such conditional requests (this might have changed). Support for conditional requests is provided by Go's standard http library.

This will help closing #63. Also the current implementation of tide::Route::serve_dir could benefit from it to handle last modification date and range requests.

I am already running a full implementation in production to serve a bunch of video files for internal use (building a media center for home).

This draft PR depends on changes that are not yet released (some not even merged) in http-types, see dependencies section below. I will update this PR as much as the situation evolves on http-types side. I will then commit the implementation, testing and more documentation.

Finally i will move out of draft status once everything is released on http-types side and implementation, tests and docs completed.

For now I am requesting comments for interest in such feature within Tide or if you prefer that I release this in an external crate. Also I would like you comments on the API, please.

Thanks in advance ! Cheers.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [X] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [X] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [X] I have read the CONTRIBUTING document.
  • [ ] I have added tests to cover my changes.
  • [ ] All new and existing tests passed.

Dependencies:

  • [x] http-rs/http-types#211 to be released
  • [x] http-rs/http-types#222 to be merged
  • [x] http-rs/http-types#224 to be merged
  • [ ] http-rs/http-types#180 to be merged

ririsoft avatar Aug 26 '20 08:08 ririsoft

I think if we have this, then ModState probably belongs in http-types?

I am not sure if that would be confusing or not, given most of the other things there represent http headers directly, but it feels like the correct place.

Fishrock123 avatar Aug 26 '20 18:08 Fishrock123

I think if we have this, then ModState probably belongs in http-types?

I am not sure if that would be confusing or not, given most of the other things there represent http headers directly, but it feels like the correct place.

This is indeed an open question. It could be renamed in something like http_types::conditional::CondState, or a better name that suite more HTTP wording. But personally I am more on the it would be confusing side: I believe the ModState is closer to the application logic, than HTTP logic (the border is thin). It is just a convenient utility provided to the user to pass either a time or a version string associated to its resource.

Looking at what Go does they just allow a time in their serve_content function, and they force users to set the ETag if any at the response header level. This is where Rust enums are superior and allow to do more for the user.

Otherwise I have no strong opinion about it.

ririsoft avatar Aug 26 '20 19:08 ririsoft

This is indeed an open question. It could be renamed in something like http_types::conditional::CondState, or a better name that suite more HTTP wording. But personally I am more on the it would be confusing side: I believe the ModState is closer to the application logic, than HTTP logic (the border is thin). It is just a convenient utility provided to the user to pass either a time or a version string associated to its resource.

I might change my opinion here. When you look at the If-Range header implementation I have submitted in http-rs/http-types#180 it has exactly the same need. Having a shared type to handle both cases might be interesting, and in such case it would belong to http-types utils.

ririsoft avatar Nov 25 '20 09:11 ririsoft

@Fishrock123 @ririsoft

Hi, I’m interested that conditional headers in tide framework. I tried to implements the middleware which can only verify about If-None-Match and content ETag. if they match, it returns the 304 NotModified. https://github.com/u5surf/tide-condition

serve_file directive must know about file modified information through from_file_with_path of http-types (see also https://github.com/http-rs/http-types/blob/main/src/body.rs#L429 Fortunately, async_std::fs::Metadata has file modification time.

https://docs.rs/async-std/0.99.12/async_std/fs/struct.Metadata.html#method.modified

It is possible to be use this. Although, it has not implemented about this yet. So now, it is hard to use modified information to serve the static file. Moreover, how can be treated in case of dynamic generated contents? That’s why I gave up implements the period verification such as using If-Modified-Since and Last-Modified

u5surf avatar Feb 14 '21 11:02 u5surf

I am closing this PR as I am not using/working on tide anymore and will thus not be able to work on this further on.

ririsoft avatar May 15 '24 14:05 ririsoft