Merge pull request #41598 from lorentey/string-index-validation

[stdlib] String.index(before:): Fix index validation issues

Author: lorentey

stdlib/public/core/StringCharacterView.swift

@@ -68,10 +68,16 @@ extension String: BidirectionalCollection {
   ///   `startIndex`.
   /// - Returns: The index value immediately before `i`.
   public func index(before i: Index) -> Index {
-    _precondition(i > startIndex, "String index is out of bounds")
-
     // TODO: known-ASCII fast path, single-scalar-grapheme fast path, etc.
+
+    // 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")
+
     let stride = _characterStride(endingAt: i)
     let priorOffset = i._encodedOffset &- stride
     return Index(

test/stdlib/StringTraps.swift

@@ -56,6 +56,35 @@ StringTraps.test("subscript(_:)/endIndex")
   _ = s[i]
 }
 
+StringTraps.test("String.index(before:) 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 = String(repeating: "X", count: 1024)
+  let short = "This is a short string"
+  expectCrashLater()
+  let i = short.index(before: long.endIndex)
+  print(i)
+}
+
+StringTraps.test("String.index(before:) trap on i == 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 = "🥯 Bagel with schmear"
+  let i = s.utf8.index(after: s.utf8.startIndex)
+  expectCrashLater()
+  // `i` is equivalent to `s.startIndex` as far as `String` is concerned
+  let j = s.index(before: i)
+  print(j)
+}
+
 StringTraps.test("UTF8ViewSubscript/endIndexSuccessor")
   .skip(.custom(
     { _isFastAssertConfiguration() },