Skip to content

Commit 4babd33

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 4babd33

File tree

2 files changed

+48
-26
lines changed

2 files changed

+48
-26
lines changed

Sources/SwiftDocC/Utility/ValidatedURL.swift

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -179,23 +179,29 @@ private extension StringProtocol {
179179
func addingPercentEncodingIfNeeded(withAllowedCharacters allowedCharacters: CharacterSet) -> String? {
180180
var needsPercentEncoding: Bool {
181181
for (index, character) in unicodeScalars.indexed() where !allowedCharacters.contains(character) {
182+
// Check if the character "%" represents a percent encoded URL.
183+
// Any other disallowed character is an indication that this substring needs percent encoding.
182184
if character == "%" {
183185
// % 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 {
186+
guard self.distance(from: index, to: self.endIndex) >= 2 else {
188187
// There's not two characters after the "%". This "%" can't represent a percent encoded character.
189188
return true
190189
}
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
190+
let firstFollowingIndex = self.index(after: index)
191+
let secondFollowingIndex = self.index(after: firstFollowingIndex)
194192

195-
} else {
196-
// Any other disallowed character is an indication that this substring needs percent encoding.
197-
return true
193+
// Check if the next two characthers represent a percent encoded
194+
// URL.
195+
// If either of the two following characters aren't hex digits,
196+
// the "%" doesn't represent a percent encoded character.
197+
if Character(unicodeScalars[firstFollowingIndex]).isHexDigit,
198+
Character(unicodeScalars[secondFollowingIndex]).isHexDigit
199+
{
200+
// Later characters in the string might require percentage encoding.
201+
continue
202+
}
198203
}
204+
return true
199205
}
200206
return false
201207
}

Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift

Lines changed: 32 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,file: StaticString = #filePath, line: UInt = #line) 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", file: file, line: line)
88+
XCTAssertEqual(validated.components.path, expectedPath, file: file, line: line)
89+
XCTAssertEqual(validated.components.fragment, expectedFragment, file: file, line: line)
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,29 @@ 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 with percent encoding at the end of the URL
126+
linkText = "doc://com.example.test/docc=Whats%20New&version=DocC&Title=[Update]%20"
127+
expectedPath = "/docc=Whats%20New&version=DocC&Title=[Update]%20"
128+
try validate(linkText: linkText, expectedPath: expectedPath)
129+
130+
// Test parameter without percent encoding
131+
linkText = "doc://com.example.test/docc=WhatsNew&version=DocC&Title=[Update]"
132+
expectedPath = "/docc=WhatsNew&version=DocC&Title=[Update]"
133+
try validate(linkText: linkText, expectedPath: expectedPath)
134+
135+
// Test parameter with special characters
136+
linkText = "doc://com.example.test/テスト"
137+
expectedPath = "/テスト"
138+
try validate(linkText: linkText, expectedPath: expectedPath)
123139
}
124140

125141
func testEscapedFragment() throws {

0 commit comments

Comments
 (0)