Skip to content

Deny leading/trailing whitespace in HeaderValue ( fixes #245 ). #256

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hannesg
Copy link

@hannesg hannesg commented Sep 22, 2018

According to rfc7230 http parsers are expected to ignore space around header values. Trying to construct a HeaderValue with leading or trailing whitespace is therefore clearly an error.

According to [rfc7230](https://tools.ietf.org/html/rfc7230#section-3.2.4)
http parsers are expected to ignore space around header values. Trying
to construct a HeaderValue with leading or trailing whitespace is
therefore clearly an error.
@carllerche
Copy link
Collaborator

I don't know if I agree that extra work should be done to make it an error. I could probably be convinced either way. Maybe @seanmonstar has thoughts on this?

@seanmonstar
Copy link
Member

I guess it depends on how the additional checks affect performance. Is it possible to see some benchmark results of before and after?

@seanmonstar seanmonstar added the B-rfc Blocked: request for comments. More discussion would help move this along. label Oct 18, 2018
@robjtede
Copy link

robjtede commented Apr 27, 2022

It's kind of understandable for from_static but sort of a shame to add this on try_from_generic (up-to-date method) since httparse already makes sure that leading and trailing spaces are not part of the header values it parses.

@DemiMarie
Copy link

@robjtede It’s the job of the library to validate the input it provides. If the input is already validated, it might make sense to add a (possibly unsafe) function that skips the validation. Not having leading and/or trailing whitespace is an invariant that must be upheld by all HTTP field values.

@DemiMarie
Copy link

@seanmonstar I don’t have benchmarks, but I expect this to be significantly cheaper than iterating through the value to check for forbidden characters. The check can be relaxed to “is the first or last byte 0x20 or less” if one does the existing validation first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-rfc Blocked: request for comments. More discussion would help move this along.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants