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

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)

clarfonthey · 2026-09-17T23:03:54Z

r? libs

* (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`.

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

rustbot · 2026-10-11T22:36:09Z

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.

…ec`, and `String::deref(_mut)` as exceptions to str/String's UTF-8 requirements.

…invalid UTF-8 input entirely instead of specifying that it is not UB but gives a `&(mut) str` with invalid UTF-8.

zachs18 · 2026-10-11T23:26:07Z

Okay, latest push has str::as_bytes(_mut), String::as_bytes, String::as_mut_vec, and String::deref(_mut) documented as not requiring-valid-UTF-8-input-on-pain-of-immediate-UB, and updates String's existing "UTF-8" docs section to have similar wording to str's "Invariant" docs section.

Since this version of the PR doesn't add the guarantee for (str,core::str)::from_utf8_unchecked(_mut), they still have the issue I mentioned here where their safety requirements are stricter than String::from_utf8_unchecked(_mut) with regard to UTF-8, but that can be fixed (or not) in a different PR.

I reimplemented String::deref(_mut) using pointer casts instead of their previous implementations that called String::as(_mut)_str (which call str::from_utf8_unchecked(_mut) (which used to do reference transmutes and now do pointer casts)). Other options would be to: 1. also add the relevant guarantees to those called functions, or 2. add private non-doc comments of the guarantees to those called functions (effectively add the guarantee as a private implementation detail, not a public guarantee).

@rustbot ready

jhpratt · 2026-10-17T06:45:47Z

r=me with completed FCP.

For FCP, this is proposing to explicitly guarantee that the following do not and will never rely on the str/String being UTF-8.

str::as_bytes
str::as_bytes_mut
String::as_bytes
String::as_bytes_mut
<String as Deref>::deref (Target=str)
<String as DerefMut>::deref_mut

@rustbot fcp merge libs

jhpratt · 2026-10-17T06:47:05Z

@rfcbot fcp merge libs

(see previous comment for info)

rust-rfcbot · 2026-10-17T06:47:08Z

@jhpratt has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/libs-ping: FCP proposed for libs, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

Mark-Simulacrum · 2026-10-20T00:38:48Z

    #[inline]
    fn deref(&self) -> &str {
-        self.as_str()
+        // Note: cannot use `String::as_str` as that may not allow invalid UTF-8.


It feels odd to bless String::deref but not the explicit form (as_str). Is there a reason we opted for that? It looks like this also has the effect of increasing noise in MIR (if I'm reading the differences right) which seems not great.

View changes since the review

I originally proposed including deref, as_str and many other functions. This proved to be controversial, so I pared the list down to only str::as_mut_bytes and String::as_mut_vec. I was then asked to add specifically as_bytes (it wasn't specified str::as_bytes or String::as_bytes, so I did both) and deref (and I inferred that deref should imply deref_mut), so I added those. I can add more, but FCP has already started on the specific list currently implemented, so that should probably be a follow-up PR? (though as of right now no-one has checked their box, so 🤷 IDK)

The mir-opt test differences aren't from this change; they're from changing str::from_utf8_unchecked from transmute::<&[u8], &str> to a pointer cast and reborrow. The mir-opt test that changed doesn't actually call String::deref, it calls <&String as PartialEq<&str>>::eq which calls <String as PartialEq<str>>::eq which calls <String as Index<RangeFull>>::index (not String::deref). The mir-opt test change is also a wash IMO (see other comment thread).

I think the explicit forms of deref should be okay to guarantee as being equivalent. The main concern is that because deref happens automatically it should be clarified as okay.

To be clear, the reason I bring it up is that I think having &**s be "valid" but s.as_str() be unsound is a bad idea. If we want to bless something here, we should bless the explicit form, and it seems fine to also bless the implicit form in that case.

I don't personally know that I feel strongly about expanding beyond just as_mut_vec/as_mut_bytes, but I don't feel great about just including Deref.

(This is a longer form of what Clar and you said...)

Right, for reference my proposal is specifically to add it to the list for the FCP and make sure everyone who's checked their boxes is okay with that.

clarfonthey · 2026-10-20T12:59:46Z

So, one potential additional valid concern: not all bytes are valid UTF-8 since there are a few prefixes that cannot show up. I think it might be desirable to retain this property for str such that, for example, the user never writes e.g. 0xFF to the buffer at all.

Mark-Simulacrum · 2026-10-22T01:19:53Z

I do not think we should have a half-way condition like that. For example, I think something like a memcpy or rotate might in theory write arbitrary garbage (or so I'd expect) from not-actually-in-the-string bytes in the tail of some vector, to be later overwritten in the same function with initialized, correct bytes. That's probably rare but I'm not sure ruling it out fully makes sense. And I'm not sure what it buys us.

clarfonthey · 2026-10-22T04:04:26Z

Note that for the purposes of the guaranties, memcpy and rotate cannot actually cause UB even if the intermediate state is incorrect, since they're intrinsics. Although this would mean that we open up the option for adding range information on the bytes which would be part of provenance under tools like miri and would be detectable UB.

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 [DEPRECATED; DO NOT USE] 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 force-pushed the string-utf8-safety-docs branch from 56deaae to 3b27e71 Compare July 10, 2026 00:11

This comment has been minimized.

Sign in to view

clarfonthey removed the T-libs-api [DEPRECATED; DO NOT USE] label Aug 12, 2026

rustbot assigned jhpratt Sep 17, 2026

This was referenced Oct 1, 2026

String::retain: convert only the untouched UTF-8 tail #163624

Open

[Rust] Add specs for str::get_unchecked, str::chars, Chars::next, char::len_utf8 and char::encode_utf8 verifast/verifast#1026

Merged

zachs18 added 2 commits October 11, 2026 16:38

Update "UTF-8" section in String's docs to link to str's "Invariant" …

c427077

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

zachs18 force-pushed the string-utf8-safety-docs branch from 73420c8 to 223a3b9 Compare October 11, 2026 22:36

This comment has been minimized.

Sign in to view

zachs18 force-pushed the string-utf8-safety-docs branch 2 times, most recently from 0470431 to 679b11c Compare October 11, 2026 22:52

zachs18 added 2 commits October 11, 2026 17:54

Document str::as_bytes(_mut), String::as_bytes, `String::as_mut_v…

09f2f03

…ec`, and `String::deref(_mut)` as exceptions to str/String's UTF-8 requirements.

Update (str,core::str)::from_utf8_unchecked(_mut)'s docs to disallow …

fd99c21

…invalid UTF-8 input entirely instead of specifying that it is not UB but gives a `&(mut) str` with invalid UTF-8.

zachs18 force-pushed the string-utf8-safety-docs branch from 679b11c to fd99c21 Compare October 11, 2026 22:54

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 Oct 11, 2026

This comment was marked as resolved.

Sign in to view

rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 17, 2026

Mark-Simulacrum reviewed Oct 20, 2026

View reviewed changes

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

theemathas mentioned this pull request Oct 20, 2026

Relax safety documentation of FnPtr::from_ptr(). #163821

Open

jhpratt added S-waiting-on-t-libs Status: Awaiting decision from T-libs and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 25, 2026

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.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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.

This comment has been minimized.

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

Uh oh!

Uh oh!

clarfonthey commented Sep 17, 2026

Uh oh!

rustbot commented Oct 11, 2026

Uh oh!

This comment has been minimized.

zachs18 commented Oct 11, 2026

Uh oh!

jhpratt commented Oct 17, 2026

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

jhpratt commented Oct 17, 2026

Uh oh!

rust-rfcbot commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mark-Simulacrum Oct 20, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clarfonthey commented Oct 20, 2026

Uh oh!

Mark-Simulacrum commented Oct 22, 2026

Uh oh!

clarfonthey commented Oct 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

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

rust-rfcbot commented Oct 17, 2026 •

edited

Loading

Mark-Simulacrum Oct 20, 2026 •

edited by rustbot

Loading