Skip to content

docs: Correct comments in HeaderValue::from_maybe_shared_unchecked.#822

Open
PiotrSikora wants to merge 1 commit intohyperium:masterfrom
PiotrSikora:unchecked_not_utf8
Open

docs: Correct comments in HeaderValue::from_maybe_shared_unchecked.#822
PiotrSikora wants to merge 1 commit intohyperium:masterfrom
PiotrSikora:unchecked_not_utf8

Conversation

@PiotrSikora
Copy link

No description provided.

Signed-off-by: Piotr Sikora <code@piotrsikora.dev>
/// `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.
/// `src` must contain valid HTTP field value. In a release build it is
/// undefined behaviour to call this with `src` that is not valid.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tottoto while we're here, could you clarify what did you mean by "undefined behaviour" (added in 63e7d63)?

While the HeaderValue that's created from invalid HTTP field value would indeed contain non-compliant HTTP field value, it would produce an error in to_str() or when used with other "checked" variants, so it's not clear what kind of "undefined behaviour" did you mean here.

Perhaps we could change it to something more descriptive? This function is already unnecessarily marked as unsafe, so using "undefined behaviour" in this context raises a bit more red flags than it should, IMHO.

Notably, majority of production deployments allow more relaxed parsing of HTTP field values than required by RFC9110 (e.g. see WHATWG), so it would be helpful if those could be stored in HeaderValue without worrying about "undefined behaviour".

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.

1 participant