Merge pull request #41599 from lorentey/string-unicodescalarview-index-validation

[stdlib] String.UnicodeScalarView: Fix some index validation issues

Author: lorentey

stdlib/public/core/StringUnicodeScalarView.swift

@@ -106,10 +106,11 @@ extension String.UnicodeScalarView: BidirectionalCollection {
   /// - Precondition: The next location exists.
   @inlinable @inline(__always)
   public func index(after i: Index) -> Index {
-    let i = _guts.scalarAlign(i)
-    _internalInvariant(i < endIndex)
     // TODO(String performance): isASCII fast-path
 
+    _precondition(i < endIndex, "String index is out of bounds")
+    let i = _guts.scalarAlign(i)
+
     if _fastPath(_guts.isFastUTF8) {
       let len = _guts.fastUTF8ScalarLength(startingAt: i._encodedOffset)
       return i.encoded(offsetBy: len)._scalarAligned
@@ -128,10 +129,16 @@ extension String.UnicodeScalarView: BidirectionalCollection {
   /// - Precondition: The previous location exists.
   @inlinable @inline(__always)
   public func index(before i: Index) -> Index {
-    let i = _guts.scalarAlign(i)
-    _precondition(i._encodedOffset > 0)
     // TODO(String performance): isASCII fast-path
 
+    // Note: bounds checking in `index(before:)` is tricky as scalar aligning an
+    // index may need to access storage, but it may also move it closer towards
+    // the `startIndex`. Therefore, we must check against the `endIndex` before
+    // aligning, but we need to delay the `i > startIndex` check until after.
+    _precondition(i <= endIndex, "String index is out of bounds")
+    let i = _guts.scalarAlign(i)
+    _precondition(i > startIndex, "String index is out of bounds")
+
     if _fastPath(_guts.isFastUTF8) {
       let len = _guts.withFastUTF8 { utf8 -> Int in
         return _utf8ScalarLength(utf8, endingAt: i._encodedOffset)

test/stdlib/StringTraps.swift

@@ -198,5 +198,67 @@ StringTraps.test("UTF8ViewIndex/offsetCrash")
   _ = s8.utf8[i]
 }
 
+StringTraps.test("UnicodeScalarView index(before:) trap on startIndex")
+.skip(
+  .custom({ _isFastAssertConfiguration() },
+  reason: "trap is not guaranteed to happen in -Ounchecked"))
+.code {
+  guard #available(SwiftStdlib 5.7, *) else { return }
+
+  let s = "abc"
+  var i = s.unicodeScalars.endIndex
+  i = s.unicodeScalars.index(before: i)
+  i = s.unicodeScalars.index(before: i)
+  i = s.unicodeScalars.index(before: i)
+  expectCrashLater()
+  i = s.unicodeScalars.index(before: i)
+}
+
+StringTraps.test("UnicodeScalarView index(before:) trap on startIndex after scalar alignment")
+.skip(
+  .custom({ _isFastAssertConfiguration() },
+  reason: "trap is not guaranteed to happen in -Ounchecked"))
+.code {
+  guard #available(SwiftStdlib 5.7, *) else { return }
+
+  let s = "🥦 Floret of broccoli"
+  var i = s.utf8.index(after: s.utf8.startIndex)
+  expectCrashLater()
+  // `i` is equivalent to `s.startIndex` as far as `String.UnicodeScalarView` is
+  // concerned
+  i = s.unicodeScalars.index(before: i)
+}
+
+StringTraps.test("UnicodeScalarView index(after:) trap on endIndex")
+.skip(
+  .custom({ _isFastAssertConfiguration() },
+  reason: "trap is not guaranteed to happen in -Ounchecked"))
+.code {
+  guard #available(SwiftStdlib 5.7, *) else { return }
+
+  let s = "abc"
+  var i = s.unicodeScalars.startIndex
+  i = s.unicodeScalars.index(after: i)
+  i = s.unicodeScalars.index(after: i)
+  i = s.unicodeScalars.index(after: i)
+  expectCrashLater()
+  i = s.unicodeScalars.index(after: i)
+}
+
+StringTraps.test("UnicodeScalarView index(after:) trap on i > endIndex")
+.skip(
+  .custom({ _isFastAssertConfiguration() },
+  reason: "trap is not guaranteed to happen in -Ounchecked"))
+.code {
+  guard #available(SwiftStdlib 5.7, *) else { return }
+
+  let long = "abcd"
+  var i = long.unicodeScalars.endIndex
+
+  let s = "abc"
+  expectCrashLater()
+  i = s.unicodeScalars.index(after: i)
+}
+
 runAllTests()