Rollup merge of #42428 - scottmcm:str-get-overflow, r=sfackler

Add overflow checking for `str::get` with inclusive ranges

Fixes https://github.com/rust-lang/rust/issues/42401

Two commits here:

1. The first makes `str::index` just call `SliceIndex<str>::index`.  It's intended to have no behavior change, except where the two methods were inconsistent.
2. The second actually adds the overflow checking to `get(_mut)` (and tests for it)

diff --git a/src/libcollections/tests/lib.rs b/src/libcollections/tests/lib.rs
index 5f5217b73c2596ee2ff6de6e23c643cab30be58f..c6f0b4436bb3179d361e8ac3b920b4f44b6b3081 100644 (file)
--- a/src/libcollections/tests/lib.rs
+++ b/src/libcollections/tests/lib.rs
@@ -24,6 +24,7 @@
 #![feature(repr_align)]
 #![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 c9b7104fec4f0072d2256f2b0de70153fd4648a9..9d8ca38b20e48c822af8beb5f756c25c88ad5d7e 100644 (file)
--- 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 547a4899c7118c85e9bf6ceffaaab190401ac988..34aca592b1e95434d598a990930dcad2d004cd1f 100644 (file)
--- a/src/libcore/str/mod.rs
+++ b/src/libcore/str/mod.rs
@@ -1617,12 +1617,7 @@ impl ops::Index<ops::RangeTo<usize>> for str {
 
         #[inline]
         fn index(&self, index: ops::RangeTo<usize>) -> &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 @@ fn index(&self, index: ops::RangeTo<usize>) -> &str {
     impl ops::IndexMut<ops::RangeTo<usize>> for str {
         #[inline]
         fn index_mut(&mut self, index: ops::RangeTo<usize>) -> &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 @@ impl ops::Index<ops::RangeFrom<usize>> for str {
 
         #[inline]
         fn index(&self, index: ops::RangeFrom<usize>) -> &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 @@ fn index(&self, index: ops::RangeFrom<usize>) -> &str {
     impl ops::IndexMut<ops::RangeFrom<usize>> for str {
         #[inline]
         fn index_mut(&mut self, index: ops::RangeFrom<usize>) -> &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 @@ impl ops::Index<ops::RangeInclusive<usize>> for str {
 
         #[inline]
         fn index(&self, index: ops::RangeInclusive<usize>) -> &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 @@ impl ops::Index<ops::RangeToInclusive<usize>> for str {
 
         #[inline]
         fn index(&self, index: ops::RangeToInclusive<usize>) -> &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 @@ fn index(&self, index: ops::RangeToInclusive<usize>) -> &str {
     impl ops::IndexMut<ops::RangeInclusive<usize>> for str {
         #[inline]
         fn index_mut(&mut self, index: ops::RangeInclusive<usize>) -> &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 @@ fn index_mut(&mut self, index: ops::RangeInclusive<usize>) -> &mut str {
     impl ops::IndexMut<ops::RangeToInclusive<usize>> for str {
         #[inline]
         fn index_mut(&mut self, index: ops::RangeToInclusive<usize>) -> &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 @@ fn index(self, slice: &str) -> &Self::Output {
         }
         #[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 @@ fn index(self, slice: &str) -> &Self::Output {
         }
         #[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 {
@@ -1945,11 +1918,19 @@ impl SliceIndex<str> for ops::RangeInclusive<usize> {
         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 {
@@ -1961,10 +1942,14 @@ unsafe fn get_unchecked_mut(self, slice: &mut str) -> &mut Self::Output {
         }
         #[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)
         }
     }
@@ -1976,7 +1961,7 @@ impl SliceIndex<str> for ops::RangeToInclusive<usize> {
         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
@@ -1984,7 +1969,7 @@ fn get(self, slice: &str) -> Option<&Self::Output> {
         }
         #[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
@@ -2002,11 +1987,15 @@ unsafe fn get_unchecked_mut(self, slice: &mut str) -> &mut Self::Output {
         }
         #[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 {