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

Don't return file content for HEAD requests in ServeFile, ServeDir #113

Closed
jplatte opened this issue Aug 3, 2021 · 5 comments · Fixed by #169
Closed

Don't return file content for HEAD requests in ServeFile, ServeDir #113

jplatte opened this issue Aug 3, 2021 · 5 comments · Fixed by #169
Labels
C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue.
Milestone

Comments

@jplatte
Copy link
Collaborator

jplatte commented Aug 3, 2021

As discussed in tokio-rs/axum#84, ServeFile and ServeDir could be improved re. handling of HEAD requests. Responses to HEAD requests should be identical to the corresponding GETs response, but without a body.

The implementation should probably use fs::metadata instead of opening the file to fulfill HEAD requests more efficiently.

@jplatte
Copy link
Collaborator Author

jplatte commented Aug 3, 2021

I wonder whether it makes sense to even support all methods by default. Just ignoring POST & PUT bodies and returning the file contents on TRACE / DELETE / OPTIONS seems kind of weird. This probably applies to other services in tower-http as well.

@davidpdrsn davidpdrsn added the C-bug Category: This is a bug. label Aug 3, 2021
@davidpdrsn
Copy link
Member

Fixing this will unfortunately be a breaking change since ServeFile implements Service for any type R rather than Service<Request<ReqBody>>.

@davidpdrsn
Copy link
Member

I wonder whether it makes sense to even support all methods by default. Just ignoring POST & PUT bodies and returning the file contents on TRACE / DELETE / OPTIONS seems kind of weird. This probably applies to other services in tower-http as well.

I agree. Method handling could be improved for ServeDir and ServeFile. Off the top of my head I don't any other services have similar issues.

@jplatte
Copy link
Collaborator Author

jplatte commented Aug 3, 2021

Oh I see, there's only one more service currently which is the redirect one (which probably should work the same regardless of HTTP method).

@davidpdrsn davidpdrsn added the E-help-wanted Call for participation: Help is requested to fix this issue. label Sep 18, 2021
@davidpdrsn davidpdrsn added this to the 0.2.0 milestone Nov 8, 2021
@Nehliin
Copy link
Collaborator

Nehliin commented Nov 12, 2021

#156 Have the same required breaking change since it needs to inspect the Accepted-Encoding headers so I could pick this up after that PR is reviewed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants