Skip to content

Mark HeaderValue::from_maybe_shared_unchecked() as safe. - #871

Closed
orium wants to merge 1 commit into
hyperium:masterfrom
orium:safety-header-values
Closed

Mark HeaderValue::from_maybe_shared_unchecked() as safe.#871
orium wants to merge 1 commit into
hyperium:masterfrom
orium:safety-header-values

Conversation

@orium

@orium orium commented Sep 9, 2026

Copy link
Copy Markdown

This method was always safe.

@seanmonstar

Copy link
Copy Markdown
Member

Is there a very strong reason to?

I know it doesn't follow the official guidelines, but I also don't think they are always right.

@orium

orium commented Sep 9, 2026

Copy link
Copy Markdown
Author

unsafe signals that UB is possible when the pre-conditions are violated. In my case it lead me to look at alternatives (for longer than I would like to admit) because I thought code that I maintain had UB. Took me a while to realize there's actually no UB and that the method is safe as far as "rust safety" is concerned (the method safety section said "src must contain valid UTF-8" which is incorrect). It's just not recommended in most cases because of the header values it allows, but that's signaled with the _unchecked method name suffix.

@seanmonstar

Copy link
Copy Markdown
Member

We've been aware of the official meaning of unsafe, and purposefully chose that this should be anyways. While it doesn't currently allow UB to happen, unsafe characters can cause message splitting, which is also very dangerous. Also, we could conceivably in the future adjust the inner storage to be more string-like and require closer to the official definition of unsafe. So in that sense, there's nothing stopping a function from claiming to be unsafe in the name of backwards-compatibility.

@seanmonstar seanmonstar closed this Sep 9, 2026
@orium

orium commented Sep 9, 2026

Copy link
Copy Markdown
Author

@seanmonstar can I at least fix the documentation? It currently says:

    /// ## Safety
    /// `src` must contain valid UTF-8. In a release build it is undefined
    /// behaviour to call this with `src` that is not valid UTF-8.

but there's no UB. I can keep the safety section an explain what the risk is.

@orium

orium commented Sep 9, 2026

Copy link
Copy Markdown
Author

Another issue that the function rejects non-utf-8 sequences in debug mode, but not in release. That would make sense if the goal was actually to reject non-utf-8 sequences, but HeaderValue::from_bytes() accepts them, so that's not an invariant HeaderValue maintains.

I can also fix that in a follow-up PR.

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 this pull request may close these issues.

2 participants