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

Non ASCII header values causes panic #519

Open
x1125 opened this issue Nov 12, 2023 · 2 comments
Open

Non ASCII header values causes panic #519

x1125 opened this issue Nov 12, 2023 · 2 comments

Comments

@x1125
Copy link

x1125 commented Nov 12, 2023

I came here via an unexpected panic in my tide webserver, but I think the problem belongs here.
When I'm using a cookie that might be a bit "odd" like password=Â , the calling library (async-h1 or tide) is calling append_header from request.rs, then append and insert within the headers/headers.rs file to set that header. On line 54 it says:
let values: HeaderValues = values.to_header_values().unwrap().collect();
which fails and panics with a Result::unwrap() on an Err value: String slice should be valid ASCII.
I think those functions should have a result of type Result<T, E> if these functions can fail in such circumstances.

@Fishrock123
Copy link
Member

In the past, http-rs held tight to disallowing things disallowed by existing http rfcs.

However in RFC 9110 "HTTP Semantics" (June 2022) under the "Field Values" (5.5) section it says:

Specifications for newly defined fields SHOULD limit their values to visible US-ASCII octets (VCHAR), SP, and HTAB. A recipient SHOULD treat other allowed octets in field content (i.e., obs-text) as opaque data.

To me this sounds as if http-rs treats this incorrectly. Instead of causing an error, the RFC indicated non-ascii octets should generally be preserved in some way, at least as i read it.

@OneOfOne
Copy link

OneOfOne commented Apr 8, 2024

this is fixed in OneOfOne@82270e7

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

No branches or pull requests

3 participants