Rewrite String::replace_range

This simplifies the code, provides better panic messages,
and avoids an integer overflow.
This commit is contained in:
Theemathas Chirananthavat 2025-11-29 14:42:43 +07:00
parent 467250ddb2
commit 5f5286beb2
2 changed files with 28 additions and 30 deletions

View file

@ -50,8 +50,6 @@ use core::iter::from_fn;
use core::ops::Add;
#[cfg(not(no_global_oom_handling))]
use core::ops::AddAssign;
#[cfg(not(no_global_oom_handling))]
use core::ops::Bound::{Excluded, Included, Unbounded};
use core::ops::{self, Range, RangeBounds};
use core::str::pattern::{Pattern, Utf8Pattern};
use core::{fmt, hash, ptr, slice};
@ -2049,30 +2047,19 @@ impl String {
where
R: RangeBounds<usize>,
{
// Memory safety
//
// Replace_range does not have the memory safety issues of a vector Splice.
// of the vector version. The data is just plain bytes.
// We avoid #81138 (nondeterministic RangeBounds impls) because we only use `range` once, here.
let checked_range = slice::range(range, ..self.len());
// WARNING: Inlining this variable would be unsound (#81138)
let start = range.start_bound();
match start {
Included(&n) => assert!(self.is_char_boundary(n)),
Excluded(&n) => assert!(self.is_char_boundary(n + 1)),
Unbounded => {}
};
// WARNING: Inlining this variable would be unsound (#81138)
let end = range.end_bound();
match end {
Included(&n) => assert!(self.is_char_boundary(n + 1)),
Excluded(&n) => assert!(self.is_char_boundary(n)),
Unbounded => {}
};
assert!(
self.is_char_boundary(checked_range.start),
"start of range should be a character boundary"
);
assert!(
self.is_char_boundary(checked_range.end),
"end of range should be a character boundary"
);
// Using `range` again would be unsound (#81138)
// We assume the bounds reported by `range` remain the same, but
// an adversarial implementation could change between calls
unsafe { self.as_mut_vec() }.splice((start, end), replace_with.bytes());
unsafe { self.as_mut_vec() }.splice(checked_range, replace_with.bytes());
}
/// Replaces the leftmost occurrence of a pattern with another string, in-place.

View file

@ -616,8 +616,15 @@ fn test_replace_range() {
}
#[test]
#[should_panic]
fn test_replace_range_char_boundary() {
#[should_panic = "start of range should be a character boundary"]
fn test_replace_range_start_char_boundary() {
let mut s = "Hello, 世界!".to_owned();
s.replace_range(8.., "");
}
#[test]
#[should_panic = "end of range should be a character boundary"]
fn test_replace_range_end_char_boundary() {
let mut s = "Hello, 世界!".to_owned();
s.replace_range(..8, "");
}
@ -632,28 +639,32 @@ fn test_replace_range_inclusive_range() {
}
#[test]
#[should_panic]
#[should_panic = "range end index 6 out of range for slice of length 5"]
fn test_replace_range_out_of_bounds() {
let mut s = String::from("12345");
s.replace_range(5..6, "789");
}
#[test]
#[should_panic]
#[should_panic = "range end index 5 out of range for slice of length 5"]
fn test_replace_range_inclusive_out_of_bounds() {
let mut s = String::from("12345");
s.replace_range(5..=5, "789");
}
// The overflowed index value is target-dependent,
// so we don't check for its exact value in the panic message
#[test]
#[should_panic]
#[should_panic = "out of range for slice of length 3"]
fn test_replace_range_start_overflow() {
let mut s = String::from("123");
s.replace_range((Excluded(usize::MAX), Included(0)), "");
}
// The overflowed index value is target-dependent,
// so we don't check for its exact value in the panic message
#[test]
#[should_panic]
#[should_panic = "out of range for slice of length 3"]
fn test_replace_range_end_overflow() {
let mut s = String::from("456");
s.replace_range((Included(0), Included(usize::MAX)), "");