Skip to content

Commit 1f92a91

Browse files
Prevent accessing unicode scalar element with incorrect index.
This change prevents a potential DocC crash by making sure that it does not access an array element using an index that is outside of the bonds of the array. `.index(after:` can crash in some cases if the passed index belongs to the last element of the collection. with this change we ensure that the passed index is not the last index. rdar://150760264
1 parent 94eaaa3 commit 1f92a91

File tree

2 files changed

+34
-23
lines changed

2 files changed

+34
-23
lines changed

Sources/SwiftDocC/Utility/ValidatedURL.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,16 +181,16 @@ private extension StringProtocol {
181181
for (index, character) in unicodeScalars.indexed() where !allowedCharacters.contains(character) {
182182
if character == "%" {
183183
// % isn't allowed in a URL fragment but it is also the escape character for percent encoding.
184-
let firstFollowingIndex = unicodeScalars.index(after: index)
185-
let secondFollowingIndex = unicodeScalars.index(after: firstFollowingIndex)
186-
187-
guard secondFollowingIndex < unicodeScalars.endIndex else {
184+
guard self.distance(from: index, to: self.endIndex) >= 2 else {
188185
// There's not two characters after the "%". This "%" can't represent a percent encoded character.
189186
return true
190187
}
191-
// If either of the two following characters aren't hex digits, the "%" doesn't represent a
192-
return !Character(unicodeScalars[firstFollowingIndex]).isHexDigit
193-
|| !Character(unicodeScalars[secondFollowingIndex]).isHexDigit
188+
let firstFollowingIndex = self.index(after: index)
189+
let secondFollowingIndex = self.index(after: firstFollowingIndex)
190+
191+
// If either of the two following characters aren't hex digits, the "%" doesn't represent a percent encoded character.
192+
return Character(unicodeScalars[firstFollowingIndex]).isHexDigit
193+
|| Character(unicodeScalars[secondFollowingIndex]).isHexDigit
194194

195195
} else {
196196
// Any other disallowed character is an indication that this substring needs percent encoding.

Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,14 @@ class ValidatedURLTests: XCTestCase {
8181
}
8282

8383
func testQueryIsPartOfPathForAuthoredLinks() throws {
84+
85+
func validate(linkText: String, expectedPath: String, expectedFragment: String? = nil) throws {
86+
let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link")
87+
XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items")
88+
XCTAssertEqual(validated.components.path, expectedPath)
89+
XCTAssertEqual(validated.components.fragment, expectedFragment)
90+
}
91+
8492
// Test return type disambiguation
8593
for linkText in [
8694
"SymbolName/memberName()->Int?",
@@ -91,14 +99,8 @@ class ValidatedURLTests: XCTestCase {
9199
? "/SymbolName/memberName()->Int?"
92100
: "SymbolName/memberName()->Int?"
93101

94-
let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link")
95-
XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items")
96-
XCTAssertEqual(validated.components.path, expectedPath)
97-
98-
let validatedWithHeading = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText + "#Heading-Name"), "Failed to parse '\(linkText)#Heading-Name' as authored link")
99-
XCTAssertNil(validatedWithHeading.components.queryItems, "Authored documentation links don't include query items")
100-
XCTAssertEqual(validatedWithHeading.components.path, expectedPath)
101-
XCTAssertEqual(validatedWithHeading.components.fragment, "Heading-Name")
102+
try validate(linkText: linkText, expectedPath: expectedPath)
103+
try validate(linkText: linkText + "#Heading-Name", expectedPath: expectedPath, expectedFragment: "Heading-Name")
102104
}
103105

104106
// Test parameter type disambiguation
@@ -111,15 +113,24 @@ class ValidatedURLTests: XCTestCase {
111113
? "/SymbolName/memberName(with:and:)-(Int?,_)"
112114
: "SymbolName/memberName(with:and:)-(Int?,_)"
113115

114-
let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link")
115-
XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items")
116-
XCTAssertEqual(validated.components.path, expectedPath)
117-
118-
let validatedWithHeading = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText + "#Heading-Name"), "Failed to parse '\(linkText)#Heading-Name' as authored link")
119-
XCTAssertNil(validatedWithHeading.components.queryItems, "Authored documentation links don't include query items")
120-
XCTAssertEqual(validatedWithHeading.components.path, expectedPath)
121-
XCTAssertEqual(validatedWithHeading.components.fragment, "Heading-Name")
116+
try validate(linkText: linkText, expectedPath: expectedPath)
117+
try validate(linkText: linkText + "#Heading-Name", expectedPath: expectedPath, expectedFragment: "Heading-Name")
122118
}
119+
120+
// Test parameter with percent encoding
121+
var linkText = "doc://com.example.test/docc=Whats%20New&version=DocC&Title=[Update]"
122+
var expectedPath = "/docc=Whats%20New&version=DocC&Title=[Update]"
123+
try validate(linkText: linkText, expectedPath: expectedPath)
124+
125+
// Test parameter without percent encoding
126+
linkText = "doc://com.example.test/docc=WhatsNew&version=DocC&Title=[Update]"
127+
expectedPath = "/docc=WhatsNew&version=DocC&Title=[Update]"
128+
try validate(linkText: linkText, expectedPath: expectedPath)
129+
130+
// Test parameter with special characters
131+
linkText = "doc://com.example.test/テスト"
132+
expectedPath = "/テスト"
133+
try validate(linkText: linkText, expectedPath: expectedPath)
123134
}
124135

125136
func testEscapedFragment() throws {

0 commit comments

Comments
 (0)