Skip to content

HEAD request with empty body but set content-length triggers panic #3794

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.

Sign up for GitHub

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

Jump to bottom
Closed
23doors opened this issue Nov 25, 2024 · 1 comment · Fixed by #3795
Closed

HEAD request with empty body but set content-length triggers panic #3794

23doors opened this issue Nov 25, 2024 · 1 comment · Fixed by #3795
Labels
A-http1 Area: HTTP/1 specific. A-server Area: server. C-bug Category: bug. Something is wrong. This is bad! E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@23doors
Copy link
Contributor

23doors commented Nov 25, 2024

Version
hyper 1.5.0

Platform
MacOS

Description

With HEAD requests it seems it is often the case that content-length is set to relevant GET body response, but payload is empty.
That's what for example tower_http ServeDir does. In debug mode this triggers panic:
https://github.com/hyperium/hyper/blob/master/src/proto/h1/role.rs#L704C36-L704C89

I guess this assertion should skip HEAD method completely or maybe it's more of a tower_http bug and body should define different sizehints here?

Release version works as expected, as assertion is skipped.
@23doors 23doors added the C-bug Category: bug. Something is wrong. This is bad! label Nov 25, 2024
@seanmonstar
Copy link
Member

Hm, Yea, that check should probably include checking if an actual body is expected. I imagine we have a function for that elsewhere in this file...

@seanmonstar seanmonstar added A-server Area: server. E-easy Effort: easy. A task that would be a great starting point for a new contributor. A-http1 Area: HTTP/1 specific. labels Nov 26, 2024
@23doors 23doors mentioned this issue Nov 26, 2024
Merged
GlenDC added a commit to plabayo/rama that referenced this issue Dec 27, 2024
@GlenDC
sync w/ upstream(hyper): v1.5.1 => v1.5.2

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
Loading
Loading status checks…
39708e1 
Changelog:

### v1.5.2 (2024-12-16)

#### Bug Fixes

* **http1:**
  * fix intermitent panic parsing partial headers (#3812) ([a131111f](hyperium/hyper@a131111), closes [#3811](hyperium/hyper#3811))
  * skip debug assertion of content length for HEAD responses (#3795) ([eaf2267c](hyperium/hyper@eaf2267), closes [#3794](hyperium/hyper#3794))

Release url: <https://github.com/hyperium/hyper/releases/tag/v1.5.2>
Raw changelog: <hyperium/hyper@v1.5.1...v1.5.2>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-http1 Area: HTTP/1 specific. A-server Area: server. C-bug Category: bug. Something is wrong. This is bad! E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: content length assertion for HEAD 23doors/hyper
2 participants
@seanmonstar @23doors