Don't parse query items for authored documentation links (#1159) (#1161)

* Don't parse query items for authored documentation links

rdar://144230958

* Make the fallback implementation more robust

* Elaborate on code comment about doc links not having query items.

Author: d-ronnqvist

Sources/SwiftDocC/Utility/ValidatedURL.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2021-2022 Apple Inc. and the Swift project authors
+ Copyright (c) 2021-2025 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See https://swift.org/LICENSE.txt for license information
@@ -55,10 +55,21 @@ public struct ValidatedURL: Hashable, Equatable {
     ///
     /// Use this to parse author provided documentation links that may contain links to on-page subsections. Escaping the fragment allows authors
     /// to write links to subsections using characters that wouldn't otherwise be allowed in a fragment of a URL.
+    ///
+    /// - Important: Documentation links don't include query items but "?" may appear in the link's path.
     init?(parsingAuthoredLink string: String) {
         // Try to parse the string without escaping anything
-        if let parsed = ValidatedURL(parsingExact: string) {
-            self.components = parsed.components
+        if var parsedComponents = ValidatedURL(parsingExact: string)?.components {
+            // Documentation links don't include query items but "?" may appear in the link's path.
+            // If `URLComponents` decoded a `query`, that's correct from a general URL standpoint but incorrect from a documentation link standpoint.
+            // To create a valid documentation link, we move the `query` component and its "?" separator into the `path` component.
+            if let query = parsedComponents.query {
+                parsedComponents.path += "?\(query)"
+                parsedComponents.query = nil
+            }
+            
+            assert(parsedComponents.string != nil, "Failed to parse authored link \(string.singleQuoted)")
+            self.components = parsedComponents
             return
         }
 
@@ -85,12 +96,13 @@ public struct ValidatedURL: Hashable, Equatable {
             remainder = remainder.dropFirst("\(ResolvedTopicReference.urlScheme):".count)
 
             if remainder.hasPrefix("//") {
+                remainder = remainder.dropFirst(2) // Don't include the "//" prefix in the `host` component.
                 // The authored link includes a bundle ID
-                guard let startOfPath = remainder.dropFirst(2).firstIndex(of: "/") else {
+                guard let startOfPath = remainder.firstIndex(of: "/") else {
                     // The link started with "doc://" but didn't contain another "/" to start of the path.
                     return nil
                 }
-                components.percentEncodedHost = String(remainder[..<startOfPath]).addingPercentEncoding(withAllowedCharacters: .urlHostAllowed)
+                components.percentEncodedHost = String(remainder[..<startOfPath]).addingPercentEncodingIfNeeded(withAllowedCharacters: .urlHostAllowed)
                 remainder = remainder[startOfPath...]
             }
         }
@@ -100,19 +112,20 @@ public struct ValidatedURL: Hashable, Equatable {
         // by documentation links and symbol links.
         if let fragmentSeparatorIndex = remainder.firstIndex(of: "#") {
             // Encode the path substring and fragment substring separately
-            guard let path = String(remainder[..<fragmentSeparatorIndex]).addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else {
+            guard let path = String(remainder[..<fragmentSeparatorIndex]).addingPercentEncodingIfNeeded(withAllowedCharacters: .urlPathAllowed) else {
                 return nil
             }
             components.percentEncodedPath = path
-            components.percentEncodedFragment = String(remainder[fragmentSeparatorIndex...].dropFirst()).addingPercentEncoding(withAllowedCharacters: .urlFragmentAllowed)
+            components.percentEncodedFragment = remainder[fragmentSeparatorIndex...].dropFirst().addingPercentEncodingIfNeeded(withAllowedCharacters: .urlFragmentAllowed)
         } else {
             // Since the link didn't include a fragment, the rest of the string is the path.
-            guard let path = String(remainder).addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else {
+            guard let path = remainder.addingPercentEncodingIfNeeded(withAllowedCharacters: .urlPathAllowed) else {
                 return nil
             }
             components.percentEncodedPath = path
         }
 
+        assert(components.string != nil, "Failed to parse authored link \(string.singleQuoted)")
         self.components = components
     }
 
@@ -160,3 +173,37 @@ public struct ValidatedURL: Hashable, Equatable {
         return components.url!
     }
 }
+
+private extension StringProtocol {
+    /// Returns a percent encoded version of the string or the original string if it is already percent encoded.
+    func addingPercentEncodingIfNeeded(withAllowedCharacters allowedCharacters: CharacterSet) -> String? {
+        var needsPercentEncoding: Bool {
+            for (index, character) in unicodeScalars.indexed() where !allowedCharacters.contains(character) {
+                if character == "%" {
+                    // % isn't allowed in a URL fragment but it is also the escape character for percent encoding.
+                    let firstFollowingIndex  = unicodeScalars.index(after: index)
+                    let secondFollowingIndex = unicodeScalars.index(after: firstFollowingIndex)
+                    
+                    guard secondFollowingIndex < unicodeScalars.endIndex else {
+                        // There's not two characters after the "%". This "%" can't represent a percent encoded character.
+                        return true
+                    }
+                    // If either of the two following characters aren't hex digits, the "%" doesn't represent a
+                    return !Character(unicodeScalars[firstFollowingIndex]).isHexDigit
+                        || !Character(unicodeScalars[secondFollowingIndex]).isHexDigit
+                    
+                } else {
+                    // Any other disallowed character is an indication that this substring needs percent encoding.
+                    return true
+                }
+            }
+            return false
+        }
+        
+        return if needsPercentEncoding {
+            addingPercentEncoding(withAllowedCharacters: allowedCharacters)
+        } else {
+            String(self)
+        }
+    }
+}

Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2021 Apple Inc. and the Swift project authors
+ Copyright (c) 2021-2025 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See https://swift.org/LICENSE.txt for license information
@@ -79,4 +79,66 @@ class ValidatedURLTests: XCTestCase {
                 XCTAssertNotNil(ValidatedURL(parsingExact: url)?.components.fragment)
             }
     }
+    
+    func testQueryIsPartOfPathForAuthoredLinks() throws {
+        // Test return type disambiguation
+        for linkText in [
+            "SymbolName/memberName()->Int?",
+            "doc:SymbolName/memberName()->Int?",
+            "doc://com.example.test/SymbolName/memberName()->Int?",
+        ] {
+            let expectedPath = linkText.hasPrefix("doc://")
+                ? "/SymbolName/memberName()->Int?"
+                :  "SymbolName/memberName()->Int?"
+            
+            let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link")
+            XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items")
+            XCTAssertEqual(validated.components.path, expectedPath)
+            
+            let validatedWithHeading = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText + "#Heading-Name"), "Failed to parse '\(linkText)#Heading-Name' as authored link")
+            XCTAssertNil(validatedWithHeading.components.queryItems, "Authored documentation links don't include query items")
+            XCTAssertEqual(validatedWithHeading.components.path, expectedPath)
+            XCTAssertEqual(validatedWithHeading.components.fragment, "Heading-Name")
+        }
+        
+        // Test parameter type disambiguation
+        for linkText in [
+            "SymbolName/memberName(with:and:)-(Int?,_)",
+            "doc:SymbolName/memberName(with:and:)-(Int?,_)",
+            "doc://com.example.test/SymbolName/memberName(with:and:)-(Int?,_)",
+        ] {
+            let expectedPath = linkText.hasPrefix("doc://")
+                ? "/SymbolName/memberName(with:and:)-(Int?,_)"
+                :  "SymbolName/memberName(with:and:)-(Int?,_)"
+            
+            let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link")
+            XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items")
+            XCTAssertEqual(validated.components.path, expectedPath)
+            
+            let validatedWithHeading = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText + "#Heading-Name"), "Failed to parse '\(linkText)#Heading-Name' as authored link")
+            XCTAssertNil(validatedWithHeading.components.queryItems, "Authored documentation links don't include query items")
+            XCTAssertEqual(validatedWithHeading.components.path, expectedPath)
+            XCTAssertEqual(validatedWithHeading.components.fragment, "Heading-Name")
+        }
+    }
+    
+    func testEscapedFragment() throws {
+        let escapedFragment = try XCTUnwrap("💻".addingPercentEncoding(withAllowedCharacters: .urlFragmentAllowed))
+        XCTAssertEqual(escapedFragment, "%F0%9F%92%BB")
+        
+        for linkText in [
+            "SymbolName#\(escapedFragment)",
+            "doc:SymbolName#\(escapedFragment)",
+            "doc://com.example.test/SymbolName#\(escapedFragment)",
+        ] {
+            let expectedPath = linkText.hasPrefix("doc://")
+                ? "/SymbolName"
+                :  "SymbolName"
+            
+            let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link")
+            
+            XCTAssertEqual(validated.components.path, expectedPath)
+            XCTAssertEqual(validated.components.fragment, "💻")
+        }
+    }
 }