Skip to content

Commit

Permalink
Rollup merge of #78499 - SkiFire13:fix-string-retain, r=m-ou-se
Browse files Browse the repository at this point in the history
Prevent String::retain from creating non-utf8 strings when abusing panic

Fixes #78498

The idea is the same as `Vec::drain`, set the len to 0 so that nobody can observe the broken invariant if it escapes the function (in this case if `f` panics)
  • Loading branch information
jonas-schievink authored Oct 29, 2020
2 parents a01e5f8 + 1f6f917 commit 48c4afb
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
10 changes: 6 additions & 4 deletions library/alloc/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,10 @@ impl String {
let mut del_bytes = 0;
let mut idx = 0;

unsafe {
self.vec.set_len(0);
}

while idx < len {
let ch = unsafe { self.get_unchecked(idx..len).chars().next().unwrap() };
let ch_len = ch.len_utf8();
Expand All @@ -1255,10 +1259,8 @@ impl String {
idx += ch_len;
}

if del_bytes > 0 {
unsafe {
self.vec.set_len(len - del_bytes);
}
unsafe {
self.vec.set_len(len - del_bytes);
}
}

Expand Down
15 changes: 15 additions & 0 deletions library/alloc/tests/string.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::borrow::Cow;
use std::collections::TryReserveError::*;
use std::ops::Bound::*;
use std::panic;

pub trait IntoCow<'a, B: ?Sized>
where
Expand Down Expand Up @@ -378,6 +379,20 @@ fn test_retain() {

s.retain(|_| false);
assert_eq!(s, "");

let mut s = String::from("0è0");
let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| {
let mut count = 0;
s.retain(|_| {
count += 1;
match count {
1 => false,
2 => true,
_ => panic!(),
}
});
}));
assert!(std::str::from_utf8(s.as_bytes()).is_ok());
}

#[test]
Expand Down

0 comments on commit 48c4afb

Please sign in to comment.