From 18612b21d05ce0021f9ef995592b158e181ffd90 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 4 Jun 2017 01:47:21 -0700 Subject: [PATCH 1/2] Delegate str:Index(Mut) to SliceIndex Move any extra logic that the former had into the latter, so they're consistent. --- src/libcore/str/mod.rs | 55 ++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 547a4899c7118..76a5d0c0b50b1 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -1617,12 +1617,7 @@ mod traits { #[inline] fn index(&self, index: ops::RangeTo) -> &str { - // is_char_boundary checks that the index is in [0, .len()] - if self.is_char_boundary(index.end) { - unsafe { self.slice_unchecked(0, index.end) } - } else { - super::slice_error_fail(self, 0, index.end) - } + index.index(self) } } @@ -1636,12 +1631,7 @@ mod traits { impl ops::IndexMut> for str { #[inline] fn index_mut(&mut self, index: ops::RangeTo) -> &mut str { - // is_char_boundary checks that the index is in [0, .len()] - if self.is_char_boundary(index.end) { - unsafe { self.slice_mut_unchecked(0, index.end) } - } else { - super::slice_error_fail(self, 0, index.end) - } + index.index_mut(self) } } @@ -1657,12 +1647,7 @@ mod traits { #[inline] fn index(&self, index: ops::RangeFrom) -> &str { - // is_char_boundary checks that the index is in [0, .len()] - if self.is_char_boundary(index.start) { - unsafe { self.slice_unchecked(index.start, self.len()) } - } else { - super::slice_error_fail(self, index.start, self.len()) - } + index.index(self) } } @@ -1676,13 +1661,7 @@ mod traits { impl ops::IndexMut> for str { #[inline] fn index_mut(&mut self, index: ops::RangeFrom) -> &mut str { - // is_char_boundary checks that the index is in [0, .len()] - if self.is_char_boundary(index.start) { - let len = self.len(); - unsafe { self.slice_mut_unchecked(index.start, len) } - } else { - super::slice_error_fail(self, index.start, self.len()) - } + index.index_mut(self) } } @@ -1724,9 +1703,7 @@ mod traits { #[inline] fn index(&self, index: ops::RangeInclusive) -> &str { - assert!(index.end != usize::max_value(), - "attempted to index str up to maximum usize"); - self.index(index.start .. index.end+1) + index.index(self) } } @@ -1738,9 +1715,7 @@ mod traits { #[inline] fn index(&self, index: ops::RangeToInclusive) -> &str { - assert!(index.end != usize::max_value(), - "attempted to index str up to maximum usize"); - self.index(.. index.end+1) + index.index(self) } } @@ -1750,9 +1725,7 @@ mod traits { impl ops::IndexMut> for str { #[inline] fn index_mut(&mut self, index: ops::RangeInclusive) -> &mut str { - assert!(index.end != usize::max_value(), - "attempted to index str up to maximum usize"); - self.index_mut(index.start .. index.end+1) + index.index_mut(self) } } #[unstable(feature = "inclusive_range", @@ -1761,9 +1734,7 @@ mod traits { impl ops::IndexMut> for str { #[inline] fn index_mut(&mut self, index: ops::RangeToInclusive) -> &mut str { - assert!(index.end != usize::max_value(), - "attempted to index str up to maximum usize"); - self.index_mut(.. index.end+1) + index.index_mut(self) } } @@ -1886,6 +1857,7 @@ mod traits { } #[inline] fn index_mut(self, slice: &mut str) -> &mut Self::Output { + // is_char_boundary checks that the index is in [0, .len()] if slice.is_char_boundary(self.end) { unsafe { self.get_unchecked_mut(slice) } } else { @@ -1932,6 +1904,7 @@ mod traits { } #[inline] fn index_mut(self, slice: &mut str) -> &mut Self::Output { + // is_char_boundary checks that the index is in [0, .len()] if slice.is_char_boundary(self.start) { unsafe { self.get_unchecked_mut(slice) } } else { @@ -1961,10 +1934,14 @@ mod traits { } #[inline] fn index(self, slice: &str) -> &Self::Output { + assert!(self.end != usize::max_value(), + "attempted to index str up to maximum usize"); (self.start..self.end+1).index(slice) } #[inline] fn index_mut(self, slice: &mut str) -> &mut Self::Output { + assert!(self.end != usize::max_value(), + "attempted to index str up to maximum usize"); (self.start..self.end+1).index_mut(slice) } } @@ -2002,11 +1979,15 @@ mod traits { } #[inline] fn index(self, slice: &str) -> &Self::Output { + assert!(self.end != usize::max_value(), + "attempted to index str up to maximum usize"); let end = self.end + 1; self.get(slice).unwrap_or_else(|| super::slice_error_fail(slice, 0, end)) } #[inline] fn index_mut(self, slice: &mut str) -> &mut Self::Output { + assert!(self.end != usize::max_value(), + "attempted to index str up to maximum usize"); if slice.is_char_boundary(self.end) { unsafe { self.get_unchecked_mut(slice) } } else { From 808a08a363591abf754fafd93ec3f44c686486ec Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 4 Jun 2017 11:08:25 -0700 Subject: [PATCH 2/2] Add overflow checking for str::get with inclusive ranges Fixes #42401 --- src/libcollections/tests/lib.rs | 1 + src/libcollections/tests/str.rs | 42 +++++++++++++++++++++++++++++++++ src/libcore/str/mod.rs | 16 +++++++++---- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/libcollections/tests/lib.rs b/src/libcollections/tests/lib.rs index 8af8786994e40..f64a549a05d4d 100644 --- a/src/libcollections/tests/lib.rs +++ b/src/libcollections/tests/lib.rs @@ -22,6 +22,7 @@ #![feature(rand)] #![feature(slice_rotate)] #![feature(splice)] +#![feature(str_checked_slicing)] #![feature(str_escape)] #![feature(test)] #![feature(unboxed_closures)] diff --git a/src/libcollections/tests/str.rs b/src/libcollections/tests/str.rs index c9b7104fec4f0..9d8ca38b20e48 100644 --- a/src/libcollections/tests/str.rs +++ b/src/libcollections/tests/str.rs @@ -358,6 +358,48 @@ fn test_slice_fail() { &"中华Việt Nam"[0..2]; } +#[test] +#[should_panic] +fn test_str_slice_rangetoinclusive_max_panics() { + &"hello"[...usize::max_value()]; +} + +#[test] +#[should_panic] +fn test_str_slice_rangeinclusive_max_panics() { + &"hello"[1...usize::max_value()]; +} + +#[test] +#[should_panic] +fn test_str_slicemut_rangetoinclusive_max_panics() { + let mut s = "hello".to_owned(); + let s: &mut str = &mut s; + &mut s[...usize::max_value()]; +} + +#[test] +#[should_panic] +fn test_str_slicemut_rangeinclusive_max_panics() { + let mut s = "hello".to_owned(); + let s: &mut str = &mut s; + &mut s[1...usize::max_value()]; +} + +#[test] +fn test_str_get_maxinclusive() { + let mut s = "hello".to_owned(); + { + let s: &str = &s; + assert_eq!(s.get(...usize::max_value()), None); + assert_eq!(s.get(1...usize::max_value()), None); + } + { + let s: &mut str = &mut s; + assert_eq!(s.get(...usize::max_value()), None); + assert_eq!(s.get(1...usize::max_value()), None); + } +} #[test] fn test_is_char_boundary() { diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 76a5d0c0b50b1..34aca592b1e95 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -1918,11 +1918,19 @@ mod traits { type Output = str; #[inline] fn get(self, slice: &str) -> Option<&Self::Output> { - (self.start..self.end+1).get(slice) + if let Some(end) = self.end.checked_add(1) { + (self.start..end).get(slice) + } else { + None + } } #[inline] fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> { - (self.start..self.end+1).get_mut(slice) + if let Some(end) = self.end.checked_add(1) { + (self.start..end).get_mut(slice) + } else { + None + } } #[inline] unsafe fn get_unchecked(self, slice: &str) -> &Self::Output { @@ -1953,7 +1961,7 @@ mod traits { type Output = str; #[inline] fn get(self, slice: &str) -> Option<&Self::Output> { - if slice.is_char_boundary(self.end + 1) { + if self.end < usize::max_value() && slice.is_char_boundary(self.end + 1) { Some(unsafe { self.get_unchecked(slice) }) } else { None @@ -1961,7 +1969,7 @@ mod traits { } #[inline] fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> { - if slice.is_char_boundary(self.end + 1) { + if self.end < usize::max_value() && slice.is_char_boundary(self.end + 1) { Some(unsafe { self.get_unchecked_mut(slice) }) } else { None