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

Return an error rather than panicking when HeaderName is too long #433

Merged
merged 1 commit into from Aug 3, 2020

Conversation

@acfoltzer
Copy link
Contributor

@acfoltzer acfoltzer commented 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.

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.
Copy link
Member

@seanmonstar seanmonstar left a comment

Seems fine to me, thanks!

@seanmonstar seanmonstar merged commit b00473c into hyperium:master Aug 3, 2020
5 checks passed
5 checks passed
Test stable
Details
Test beta
Details
Test nightly
Details
Test 1.39.0
Details
WASM
Details
@acfoltzer acfoltzer deleted the acfoltzer:header-name-len-error branch Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.