(132421368) URL: Don't percent-encode an appended component as a first path segment (#766)

Author: jrflat

Sources/FoundationEssentials/URL/URL.swift

@@ -2188,27 +2188,37 @@ extension URL {
 
     private func appending<S: StringProtocol>(path: S, directoryHint: DirectoryHint, encodingSlashes: Bool) -> URL {
         #if os(Windows)
-        let path = path.replacing(UInt8(ascii: "\\"), with: UInt8(ascii: "/"))
+        var path = path.replacing(._backslash, with: ._slash)
+        #else
+        var path = String(path)
         #endif
+
+        var newPath = relativePath()
+        var insertedSlash = false
+        if !newPath.isEmpty && path.utf8.first != ._slash {
+            // Don't treat as first path segment when encoding
+            path = "/" + path
+            insertedSlash = true
+        }
+
         guard var pathToAppend = Parser.percentEncode(path, component: .path) else {
             return self
         }
         if encodingSlashes {
             var utf8 = Array(pathToAppend.utf8)
-            utf8.replace([UInt8(ascii: "/")], with: [UInt8(ascii: "%"), UInt8(ascii: "2"), UInt8(ascii: "F")])
+            utf8[(insertedSlash ? 1 : 0)...].replace([._slash], with: [UInt8(ascii: "%"), UInt8(ascii: "2"), UInt8(ascii: "F")])
             pathToAppend = String(decoding: utf8, as: UTF8.self)
         }
 
-        let slash = UInt8(ascii: "/")
-        var newPath = relativePath()
-        if newPath.utf8.last != slash && pathToAppend.utf8.first != slash {
+        if newPath.utf8.last != ._slash && pathToAppend.utf8.first != ._slash {
             newPath += "/"
-        } else if newPath.utf8.last == slash && pathToAppend.utf8.first == slash {
+        } else if newPath.utf8.last == ._slash && pathToAppend.utf8.first == ._slash {
             _ = newPath.popLast()
         }
 
         newPath += pathToAppend
-        let hasTrailingSlash = newPath.utf8.last == slash
+
+        let hasTrailingSlash = newPath.utf8.last == ._slash
         let isDirectory: Bool
         switch directoryHint {
         case .isDirectory:
@@ -2220,7 +2230,7 @@ extension URL {
             // We can only check file system if the URL is a file URL
             if isFileURL {
                 let filePath: String
-                if newPath.utf8.first == slash {
+                if newPath.utf8.first == ._slash {
                     filePath = URL.fileSystemPath(for: newPath)
                 } else {
                     filePath = URL.fileSystemPath(for: mergedPath(for: newPath))
@@ -2236,7 +2246,7 @@ extension URL {
         case .inferFromPath:
             isDirectory = hasTrailingSlash
         }
-        if isDirectory && newPath.utf8.last != slash {
+        if isDirectory && newPath.utf8.last != ._slash {
             newPath += "/"
         }
 

Tests/FoundationEssentialsTests/URLTests.swift

@@ -514,6 +514,46 @@ final class URLTests : XCTestCase {
         }
     }
 
+    func testURLAppendingPathDoesNotEncodeColon() throws {
+        let baseURL = URL(string: "file:///var/mobile/")!
+        let url = URL(string: "relative", relativeTo: baseURL)!
+        let component = "no:slash"
+        let slashComponent = "/with:slash"
+
+        // Make sure we don't encode ":" since `component` is not the first path segment
+        var appended = url.appending(path: component, directoryHint: .notDirectory)
+        XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/no:slash")
+        XCTAssertEqual(appended.relativePath, "relative/no:slash")
+
+        appended = url.appending(path: slashComponent, directoryHint: .notDirectory)
+        XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/with:slash")
+        XCTAssertEqual(appended.relativePath, "relative/with:slash")
+
+        appended = url.appending(component: component, directoryHint: .notDirectory)
+        XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/no:slash")
+        XCTAssertEqual(appended.relativePath, "relative/no:slash")
+
+        // `appending(component:)` should explicitly treat `component` as a single
+        // path component, meaning "/" should be encoded to "%2F" before appending
+        appended = url.appending(component: slashComponent, directoryHint: .notDirectory)
+        #if FOUNDATION_FRAMEWORK_NSURL
+        XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/with:slash")
+        XCTAssertEqual(appended.relativePath, "relative/with:slash")
+        #else
+        XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/%2Fwith:slash")
+        XCTAssertEqual(appended.relativePath, "relative/%2Fwith:slash")
+        #endif
+
+        appended = url.appendingPathComponent(component, isDirectory: false)
+        XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/no:slash")
+        XCTAssertEqual(appended.relativePath, "relative/no:slash")
+
+        // Test deprecated API, which acts like `appending(path:)`
+        appended = url.appendingPathComponent(slashComponent, isDirectory: false)
+        XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/with:slash")
+        XCTAssertEqual(appended.relativePath, "relative/with:slash")
+    }
+
     func testURLComponentsPercentEncodedUnencodedProperties() throws {
         var comp = URLComponents()