Skip to content

Fix for a crash in String.substring(with:) [SR-2483] #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion Foundation/NSString.swift
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,49 @@ extension NSString {
let start = _storage.utf16.startIndex
let min = start.advanced(by: range.location)
let max = start.advanced(by: range.location + range.length)
return String(_storage.utf16[min..<max])!
if let substr = String(_storage.utf16[min..<max]) {
return substr
}
//If we come here, then the range has created unpaired surrogates on either end.
//An unpaired surrogate is replaced by OXFFFD - the Unicode Replacement Character.
//The CRLF ("\r\n") sequence is also treated like a surrogate pair, but its constinuent
//characters "\r" and "\n" can exist outside the pair!

let replacementCharacter = String(describing: UnicodeScalar(0xFFFD)!)
let CR: UInt16 = 13 //carriage return
let LF: UInt16 = 10 //new line

//make sure the range is of non-zero length
guard range.length > 0 else { return "" }

//if the range is pointing to a single unpaired surrogate
if range.length == 1 {
switch _storage.utf16[min] {
case CR: return "\r"
case LF: return "\n"
default: return replacementCharacter
}
}

//set the prefix and suffix characters
let prefix = _storage.utf16[min] == LF ? "\n" : replacementCharacter
let suffix = _storage.utf16[max.advanced(by: -1)] == CR ? "\r" : replacementCharacter

//if the range breaks a surrogate pair at the beginning of the string
if let substrSuffix = String(_storage.utf16[min.advanced(by: 1)..<max]) {
return prefix + substrSuffix
}

//if the range breaks a surrogate pair at the end of the string
if let substrPrefix = String(_storage.utf16[min..<max.advanced(by: -1)]) {
return substrPrefix + suffix
}

//the range probably breaks surrogate pairs at both the ends
guard min.advanced(by: 1) <= max.advanced(by: -1) else { return prefix + suffix }

let substr = String(_storage.utf16[min.advanced(by: 1)..<max.advanced(by: -1)])!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After all the prior checks, I think this can be safely unwrapped.

return prefix + substr + suffix
} else {
let buff = UnsafeMutablePointer<unichar>.allocate(capacity: range.length)
getCharacters(buff, range: range)
Expand Down
38 changes: 38 additions & 0 deletions TestFoundation/TestNSString.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class TestNSString : XCTestCase {
("test_reflection", { _ in test_reflection }),
("test_replacingOccurrences", test_replacingOccurrences),
("test_getLineStart", test_getLineStart),
("test_substringWithRange", test_substringWithRange),
]
}

Expand Down Expand Up @@ -1063,6 +1064,43 @@ class TestNSString : XCTestCase {
XCTAssertTrue(testString.hasPrefix(""))
XCTAssertTrue(testString.hasSuffix(""))
}

func test_substringWithRange() {
let trivial = NSString(string: "swift.org")
XCTAssertEqual(trivial.substring(with: NSMakeRange(0, 5)), "swift")

let surrogatePairSuffix = NSString(string: "Hurray🎉")
XCTAssertEqual(surrogatePairSuffix.substring(with: NSMakeRange(0, 7)), "Hurray�")

let surrogatePairPrefix = NSString(string: "🐱Cat")
XCTAssertEqual(surrogatePairPrefix.substring(with: NSMakeRange(1, 4)), "�Cat")

let singleChar = NSString(string: "😹")
XCTAssertEqual(singleChar.substring(with: NSMakeRange(0,1)), "�")

let crlf = NSString(string: "\r\n")
XCTAssertEqual(crlf.substring(with: NSMakeRange(0,1)), "\r")
XCTAssertEqual(crlf.substring(with: NSMakeRange(1,1)), "\n")
XCTAssertEqual(crlf.substring(with: NSMakeRange(1,0)), "")

let bothEnds1 = NSString(string: "😺😺")
XCTAssertEqual(bothEnds1.substring(with: NSMakeRange(1,2)), "��")

let s1 = NSString(string: "😺\r\n")
XCTAssertEqual(s1.substring(with: NSMakeRange(1,2)), "�\r")

let s2 = NSString(string: "\r\n😺")
XCTAssertEqual(s2.substring(with: NSMakeRange(1,2)), "\n�")

let s3 = NSString(string: "😺cats😺")
XCTAssertEqual(s3.substring(with: NSMakeRange(1,6)), "�cats�")

let s4 = NSString(string: "😺cats\r\n")
XCTAssertEqual(s4.substring(with: NSMakeRange(1,6)), "�cats\r")

let s5 = NSString(string: "\r\ncats😺")
XCTAssertEqual(s5.substring(with: NSMakeRange(1,6)), "\ncats�")
}
}

struct ComparisonTest {
Expand Down