Skip to content

http::header::HeaderValue rejects bytes that WHATWG specs and WPT tests now allow #376

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
pshaughn opened this issue Dec 21, 2019 · 6 comments · May be fixed by #444
Open

http::header::HeaderValue rejects bytes that WHATWG specs and WPT tests now allow #376

pshaughn opened this issue Dec 21, 2019 · 6 comments · May be fixed by #444

Comments

@pshaughn
Copy link

https://fetch.spec.whatwg.org/#concept-header-value currently allows most bytes in the 1-31 range or 127, and https://github.com/web-platform-tests/wpt/blob/master/fetch/api/headers/header-values.html expects these bytes to roundtrip safely through request and response headers. The required code change is probably just to

http/src/header/value.rs

Lines 557 to 560 in 455f33e

#[inline]
fn is_valid(b: u8) -> bool {
b >= 32 && b != 127 || b == b'\t'
}
with more documentation lines to change than code lines.

@seanmonstar
Copy link
Member

Is there a reference somewhere when that change was made? RFC 7230 requires header values to be "VCHAR or obs-text".

@pshaughn
Copy link
Author

whatwg/fetch#332 is referenced in the commit that added the WPT test.

@jonathanKingston
Copy link

@seanmonstar is there any changes that are required to the PR? Are there issues or concerns with landing this?
Thanks!

@seanmonstar
Copy link
Member

I prefix this with "my opinion may be wrong".

I'm not a fan of the Fetch spec re-defining parts of the HTTP spec. I realize that WHATWG is basically the browser makers defining the reality of how web browsers already operate, but there's more to the Internet than just browsers. The IETF makes the HTTP specs, and it makes more sense to me if WHATWG were to get the HTTP specs updated, instead of having a competing section as part of the JavaScript fetch function spec. (There's even more confusion in all the specs around media/mime types, which suffers from similar problems, but that's also slightly off-topic).

@pshaughn
Copy link
Author

@seanmonstar may be right as to the overall general case. It does make sense to think of the non-conformant things that browsers have to allow for backwards compatibility as a special case, and since Hyper isn't making a browser, there's no inherent reason why Hyper has to be the one going out of their way to support that exceptional case. It would be nice if there were a feature flag or something, but I do can see value in enforcing RFC validity rules as a default, since someone writing e.g. a Web service client isn't going to want to get tripped up by browser legacy weirdness.

@hax
Copy link

hax commented Oct 1, 2021

Recent HTTP spec has been changed:

Field values containing other CTL characters are also invalid; however, recipients may retain such characters for the sake of robustness when they appear within a safe context (e.g., an application-specific quoted string that will not be processed by any downstream HTTP parser).

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.

4 participants