Update/relax str/String utf8 safety docs by zachs18 · Pull Request #134598 · rust-lang/rust

zachs18 · 2024-12-21T04:07:03Z

Relax UTF-8 requirements of String as well as some str/String-related functions from "must be UTF-8 or UB" to "invalid UTF-8 is not immediate UB but is unsafe" to match the validity requirement for str.

cc @joshtriplett #71033 (comment)

I wonder if we might be able, in the future, to carefully exclude a few of str's functions from the "library UB" requirements.

Current state:

str currently documents that it's UTF-8 requirement is not a language-level requirement (i.e. having a non-UTF-8 str is not immediately UB, but it can cause later UB because other code can assume that str's are valid UTF-8), though std::str::from_utf8_unchecked currently still has a stronger requirement.

String does not currently have such docs, though some of its associated functions' docs imply it¹.

(first commit) This PR relaxes (str,std::str)::from_utf8_unchecked(_mut)'s and str::as_bytes_mut's UTF-8 requirements to match that of str itself. Updates the safety comments and implementation in (str,std::str)::from_utf8_unchecked(_mut). Adds # Safety documentation to *_mut that previously just deferred to the immutable versions.

(second commit) This PR also adds an "Invariant" section to String's docs to match str, which documents that

Rust libraries may assume that Strings are always valid UTF-8, just like strs.

Constructing a non-UTF-8 String is not immediate undefined behavior, but any function called on a String may assume that it is valid UTF-8, which means that a non-UTF-8 String can lead to undefined behavior down the road.

(third commit) This PR also documents str::as_bytes_mut and String::as_mut_vec as "exceptions" to str/String's "Invariant" sections, that they are not UB to call on invalid-UTF-8 str/Strings.

As this relaxes stable API requirements, I think this needs a T-libs-api FCP?

@rustbot label T-libs-api

r? @joshtriplett (or other T-libs-api)

String::from_utf8_unchecked's docs say "This function is unsafe because it does not check that the bytes passed to it are valid UTF-8. If this constraint is violated, it may cause memory unsafety issues with future users of the String, as the rest of the standard library assumes that Strings are valid UTF-8." Note that it mentions only future users, implying that calling String::from_utf8_unchecked with invalid UTF-8 alone is not immediate UB. ↩

scottmcm · 2024-12-21T05:54:38Z

+/// * `String::as_bytes`
+/// * `String::as_bytes_mut`
+/// * `String::as_str`
+/// * `String::as_mut_str`
+/// * `String::as_ptr`
+/// * `String::as_mut_ptr`
+/// * `String::as_mut_vec`
+/// * `String::capacity`
+/// * `String::len`
+/// * `String::clear`
+/// * `<String as Drop>::drop` (i.e. dropping a `String` that contains invalid UTF-8 does not alone cause UB)
+/// * `String::leak`
+/// * `String::into_boxed_str`
+/// * `String::reserve`
+/// * `String::reserve_exact`
+/// * `String::try_reserve`
+/// * `String::try_reserve_exact`
+/// * `<String as Deref>::deref`
+/// * `<String as DerefMut>::deref_mut`
+/// * `<String as AsRef<str>>::as_ref`
+/// * `<String as AsMut<str>>::as_mut`
+/// * `<String as Borrow<str>>::borrow`
+/// * `<String as BorrowMut<str>>::borrow_mut`
+/// * `<String as Clone>::clone`
+/// * `<String as Clone>::clone_from`


I think this list should be cut way down.

There should be a couple of things that are the gateways to using it in ways that temporarily break invariants that make this promise, but everything else should be allowed to check if it feels like.

TBH, I'd be tempted to say that the only thing that explicitly allows it is as_mut_vec, since you can always do whatever you wanted on that Vec<u8>.

(Let others make the case why they need anything else.)

The specific list is not set in stone, I mostly just chose functions that don't do any string manipulation, and intended for T-libs-api to chime in here too.

I don't really see a reason to make conceptually non-"string manipulation" methods require the String to be valid UTF-8 on pain of immediate UB. At the very least I would expect that methods which conceptually don't "read" the heap allocation at all would be valid (definitely len and capacity, probably clear and dropping). Also the methods explicitly meant for accessing the raw bytes (as_mut_vec like you mentioned, into_bytes, ~~as_mut_bytes, and as_mut_ptr~~ which I just realized are actually strs methods via DerefMut).

I personally think this list is entirely unhelpful, honestly. Instead of documenting this here, we should say that the expectation is absolute and will only be documented if there are exceptions, in specific methods.

So, for example, I think it would be fair to say in the docs for as_mut_vec and as_bytes_mut that it's okay to violate the invariant as long as you fix it before you use any String methods. I don't think that offering this massive list is reasonable as an API guarantee, though.

Like, it's very likely that calling len and is_empty will always be fine, but, these methods also exist on &mut [u8] and &mut Vec<u8>, so, why even bother specifying? Additionally, "don't call any methods on String" is a much easier invariant to potentially automatically scan for, e.g. via a clippy lint, than verifying that a list of String methods aren't called. You can more easily detect when the borrow for as_mut_vec ends, for example, than when any method is called in the meantime.

I personally think this list is entirely unhelpful, honestly. Instead of documenting this here, we should say that the expectation is absolute and will only be documented if there are exceptions, in specific methods.

Done in latest push.

So, for example, I think it would be fair to say in the docs for as_mut_vec and as_bytes_mut that it's okay to violate the invariant as long as you fix it before you use any String methods.

We already say this for these.

This PR does relax str::as_bytes_mut's requirements from "must be UTF-8 when the borrow expires" to "invalid UTF-8 after the borrow expires is not immediate UB but is unsafe" to match String::as_mut_vec's.

This PR also relaxes both of them to not be UB on invalid UTF-8 on their input so that they can be used to "fix" invalid UTF-8.

I don't think that offering this massive list is reasonable as an API guarantee, though.

I disagree, but I've reduced the list of exceptions for this PR to only str::as_bytes_mut and String::as_mut_vec, and left other functions to later PRs to not bikeshed on it here.

scottmcm · 2024-12-21T05:58:42Z

 ///
 /// # Safety
 ///
-/// The bytes passed in must be valid UTF-8.


I think we should keep this requirement, to preserve the freedom for tooling (miri, ubchecks, kani, whatever) to validate it, even if we don't in release builds.

People can always pointer-cast if they want to do turn a &[u8] into &str without involving any standard library methods.

It's true that you can use pointer casting to get a &str with invalid UTF-8 from a &[u8] even if this function requires valid UTF-8 on pain of immediate UB, but String::from_utf8_unchecked currently does not require valid UTF-8 on pain of immediate UB (only on pain of later UB if the String is misused), so there's an asymmetry here.

So, we can either:

Relax str::from_utf8_unchecked(_mut) to not be immediate UB on invalid UTF-8 (what this PR does)

Restrict String::from_utf8_unchecked to be immediate UB on invalid UTF-8 (probably a breaking change)

Status quo with str::from_utf8_unchecked and String::from_utf8_unchecked having different preconditions w.r.t. UTF-8

Something else? (maybe declare calling _::from_utf8_unchecked on invalid UTF-8 "library erroneous behaviour"?)

alex-semenyuk · 2025-04-23T21:45:20Z

Triage: comments are not answered
@rustbot label: +S-waiting-on-author, -S-waiting-on-review

zachs18 · 2025-09-15T16:46:12Z

Responded to comments @rustbot ready

clarfonthey · 2026-05-18T14:11:07Z

Triage: don't think that this is suitable to merge as-is, left additional comments.

@rustbot author

rustbot · 2026-05-18T14:11:14Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

zachs18 · 2026-07-09T21:30:48Z

I've pared down the exceptions from the third commit to only str::as_bytes_mut and String::as_mut_vec, which is at least enough to "fix" invalid UTF-8 in an owned/uniquely-borrowed str/String before passing it to other code or ending the unique borrow. I'll leave other functions from the original list of exceptions to later PRs since which particular functions should be exceptions has proven controversial.

@rustbot ready

* (str,core::str)::from_utf8_unchecked: doc: do not declare that invalid UTF-8 is immediate UB, analogous to `String::from_utf8_unchecked`. * (str,core::str)::from_utf8_unchecked: update internal safety comments, and use pointer cast instead of transmute * (str,core::str)::from_utf8_unchecked_mut: add `# Safety` section mirroring `from_utf8_unchecked`, instead of just referring to it. * str::as_bytes_mut: doc: do not declare that invalid UTF-8 after borrow ends is immediate UB, analogous to `String::as_mut_vec`.

Documents that invalid UTF-8 in a `String` is not immediate UB, but other code may assume that `String`s are valid UTF-8, so it can lead to UB later.

…o str/String's UTF-8 requirements.

rustbot · 2026-07-10T00:11:20Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

zachs18 · 2026-07-10T00:13:51Z

(I think the mir-opt changes are due to changing from_utf8_unchecked from a transmute to a pointer cast and reborrow)

rustbot assigned joshtriplett Dec 21, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 21, 2024

This comment has been minimized.

Sign in to view

scottmcm reviewed Dec 21, 2024

View reviewed changes

Comment thread library/core/src/str/converts.rs

scottmcm reviewed Dec 21, 2024

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 23, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 15, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 18, 2026

zachs18 force-pushed the string-utf8-safety-docs branch from a29cb96 to 732cc42 Compare July 9, 2026 21:20

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 9, 2026

This comment has been minimized.

Sign in to view

zachs18 added 3 commits July 9, 2026 18:42

Add Invariant section to String's docs, analogous to str.

43252a0

Documents that invalid UTF-8 in a `String` is not immediate UB, but other code may assume that `String`s are valid UTF-8, so it can lead to UB later.

Document str::as_bytes_mut and String::as_mut_vec as exceptions t…

3b27e71

…o str/String's UTF-8 requirements.

zachs18 force-pushed the string-utf8-safety-docs branch from 56deaae to 3b27e71 Compare July 10, 2026 00:11

Uh oh!

Uh oh!

Conversation

zachs18 commented Dec 21, 2024 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

This comment has been minimized.

scottmcm Dec 21, 2024

Choose a reason for hiding this comment

Uh oh!

zachs18 Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

clarfonthey May 18, 2026

Choose a reason for hiding this comment

Uh oh!

zachs18 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scottmcm Dec 21, 2024

Choose a reason for hiding this comment

Uh oh!

zachs18 Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-semenyuk commented Apr 23, 2025

Uh oh!

zachs18 commented Sep 15, 2025

Uh oh!

clarfonthey commented May 18, 2026

Uh oh!

rustbot commented May 18, 2026

Uh oh!

This comment has been minimized.

zachs18 commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Jul 10, 2026

Uh oh!

zachs18 commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

zachs18 commented Dec 21, 2024 •

edited by rustbot

Loading

zachs18 Apr 23, 2025 •

edited

Loading

zachs18 commented Jul 9, 2026 •

edited

Loading

zachs18 commented Jul 10, 2026 •

edited

Loading