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

HeaderName constructors panic for names longer than 64KiB #432

Closed
acfoltzer opened this issue Jul 29, 2020 · 0 comments · Fixed by #433
Closed

HeaderName constructors panic for names longer than 64KiB #432

acfoltzer opened this issue Jul 29, 2020 · 0 comments · Fixed by #433

Comments

@acfoltzer
Copy link
Member

Attempting to create a HeaderName longer than 64KiB fails, but with a panic rather than an InvalidHeaderName error: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f43aad9f01cc5c1bd05149129310c7e7

I think this should return Err(InvalidHeaderName) instead, but if there is good cause to keep it as a panic, that panic should be documented on the various constructors so it's less of a surprise.

If you agree about switching to the Err behavior, I'd be happy to put in a PR.

acfoltzer added a commit to acfoltzer/http that referenced this issue Aug 3, 2020
Fixes hyperium#432.

This eliminates an undocumented panic from the `HeaderName` creation functions, returning instead an
`InvalidHeaderName` when the name length exceeds `MAX_HEADER_NAME_LEN`.

I considered making `InvalidHeaderName` a richer error type, but since it was already used for
another type of length error (rejecting zero-length names) I figured it was okay to reuse.

I also redefined `MAX_HEADER_NAME_LEN` slightly, so that it is equal to the largest allowed header
length, rather than one past that value. This was motivated by discovering a bug in my comparison
logic when I went to write the new test exercising the wrong-length error conditions.
seanmonstar pushed a commit that referenced this issue Aug 3, 2020
Fixes #432.

This eliminates an undocumented panic from the `HeaderName` creation functions, returning instead an
`InvalidHeaderName` when the name length exceeds `MAX_HEADER_NAME_LEN`.

I considered making `InvalidHeaderName` a richer error type, but since it was already used for
another type of length error (rejecting zero-length names) I figured it was okay to reuse.

I also redefined `MAX_HEADER_NAME_LEN` slightly, so that it is equal to the largest allowed header
length, rather than one past that value. This was motivated by discovering a bug in my comparison
logic when I went to write the new test exercising the wrong-length error conditions.
BenxiangGe pushed a commit to BenxiangGe/http that referenced this issue Jul 26, 2021
…perium#433)

Fixes hyperium#432.

This eliminates an undocumented panic from the `HeaderName` creation functions, returning instead an
`InvalidHeaderName` when the name length exceeds `MAX_HEADER_NAME_LEN`.

I considered making `InvalidHeaderName` a richer error type, but since it was already used for
another type of length error (rejecting zero-length names) I figured it was okay to reuse.

I also redefined `MAX_HEADER_NAME_LEN` slightly, so that it is equal to the largest allowed header
length, rather than one past that value. This was motivated by discovering a bug in my comparison
logic when I went to write the new test exercising the wrong-length error conditions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant