CoreFoundation: correct CreateStringFromFileSystemRepresentationByAddingPercentEscapes

On Windows,
`CreateStringFromFileSystemRepresentationByAddingPercentEscapes` would
only partially convert the slash character to %2F.  This would result in
paths being truncated.  Fix this behaviour to canonicalise the separator
properly always.  Unfortunately, this uncovered some new cases of file
path handling being broken on Windows.  Add tests to record those cases
so that we can fix them subsequently.

Author: compnerd

CoreFoundation/URL.subproj/CFURL.c

@@ -894,15 +894,34 @@ static CFStringRef CreateStringFromFileSystemRepresentationByAddingPercentEscape
         if ( idx == numBytes ) {
             if ( isDirectory ) {
                 // if it is a directory and it doesn't end with PATH_SEP, append a PATH_SEP.
-                if ( bytes[numBytes-1] != '/' ) {
-                    *bufBytePtr++ = '/';
+                if ( windowsPath ) {
+                    if ( bufBytePtr - bufStartPtr > 3 ) {
+                        if ( strncmp((const char *)(bufBytePtr - 3), "%2F", 3) ) {
+                            *bufBytePtr++ = '%';
+                            *bufBytePtr++ = '2';
+                            *bufBytePtr++ = 'F';
+                        }
+                    }
+                }
+                else {
+                    if ( bytes[numBytes-1] != '/' ) {
+                        *bufBytePtr++ = '/';
+                    }
                 }
             }
             else {
                 // it is not a directory: remove any pathDelim characters at end (leaving at least one character)
-                while ( (numBytes > 1) && (bytes[numBytes-1] == '/') ) {
-                    --bufBytePtr;
-                    --numBytes;
+                if ( windowsPath ) {
+                    while ( (numBytes > 1) && (bufBytePtr - bufStartPtr > 3) && (strncmp((const char *)(bufBytePtr - 3), "%2F", 3) == 0) ) {
+                        bufBytePtr -= 3;
+                        --numBytes;
+                    }
+                }
+                else {
+                    while ( (numBytes > 1) && (bytes[numBytes-1] == '/') ) {
+                        --bufBytePtr;
+                        --numBytes;
+                    }
                 }
             }
 

TestFoundation/TestURL.swift

@@ -47,6 +47,58 @@ private func getTestData() -> [Any]? {
 }
 
 class TestURL : XCTestCase {
+    func test_pathSeparator() {
+      // ensure that the mixed slashes are handled properly
+      // e.g. NOT file:///S:/b/u1%2/
+      let u1 = URL(fileURLWithPath: "S:\\b\\u1/")
+      XCTAssertEqual(u1.absoluteString, "file:///S:/b/u1/")
+
+      // ensure that trailing slashes are compressed
+      // e.g. NOT file:///S:/b/u2%2F%2F%2F%/
+      let u2 = URL(fileURLWithPath: "S:\\b\\u2/////")
+      XCTAssertEqual(u2.absoluteString, "file:///S:/b/u2/")
+
+      // ensure that the trailing slashes are compressed even when mixed
+      // e.g. NOT file:///S:/b/u3%2F%/%2F%2/
+      let u3 = URL(fileURLWithPath: "S:\\b\\u3//\\//")
+      // XCTAssertEqual(u3.absoluteString, "file:///S:/b/u3/%2F/")
+      XCTAssertEqual(u3.path, "S:\\b\\u3\\")
+
+      // ensure that the regular conversion works
+      let u4 = URL(fileURLWithPath: "S:\\b\\u4")
+      XCTAssertEqual(u4.absoluteString, "file:///S:/b/u4")
+
+      // ensure that the trailing slash is added
+      let u5 = URL(fileURLWithPath: "S:\\b\\u5", isDirectory: true)
+      XCTAssertEqual(u5.absoluteString, "file:///S:/b/u5/")
+
+      // ensure that the trailing slash is preserved
+      let u6 = URL(fileURLWithPath: "S:\\b\\u6\\")
+      XCTAssertEqual(u6.absoluteString, "file:///S:/b/u6/")
+
+      // ensure that we do not index beyond the start of the string
+      let u7 = URL(fileURLWithPath: "eh", relativeTo: URL(fileURLWithPath: "S:\\b"))
+      XCTAssertEqual(u7.absoluteString, "file:///S:/b/eh")
+
+      // ensure that / is handled properly
+      let u8 = URL(fileURLWithPath: "/")
+      XCTAssertEqual(u8.absoluteString, "file:///")
+    }
+
+    func test_pathSeparator2() {
+      let u1 = URL(fileURLWithPath: "S:\\b\\u1\\", isDirectory: false)
+      XCTAssertEqual(u1.absoluteString, "file:///S:/b/u1")
+
+      let u2 = URL(fileURLWithPath: "/", isDirectory: false)
+      XCTAssertEqual(u2.absoluteString, "file:///")
+
+      let u3 = URL(fileURLWithPath: "\\", isDirectory: false)
+      XCTAssertEqual(u3.absoluteString, "file:///")
+
+      let u4 = URL(fileURLWithPath: "S:\\b\\u3//\\//")
+      XCTAssertEqual(u4.absoluteString, "file:///S:/b/u3/")
+    }
+
     func test_fileURLWithPath_relativeTo() {
         let homeDirectory = NSHomeDirectory()
         let homeURL = URL(fileURLWithPath: homeDirectory, isDirectory: true)
@@ -604,7 +656,7 @@ class TestURL : XCTestCase {
     }
 
     static var allTests: [(String, (TestURL) -> () throws -> Void)] {
-        return [
+        var tests: [(String, (TestURL) -> () throws -> Void)] = [
             ("test_URLStrings", test_URLStrings),
             ("test_fileURLWithPath_relativeTo", test_fileURLWithPath_relativeTo ),
             // TODO: these tests fail on linux, more investigation is needed
@@ -619,6 +671,15 @@ class TestURL : XCTestCase {
             ("test_URLResourceValues", testExpectedToFail(test_URLResourceValues,
                 "test_URLResourceValues: Except for .nameKey, we have no testable attributes that work in the environment Swift CI uses, for now. SR-XXXX")),
         ]
+
+#if os(Windows)
+        tests.append(contentsOf: [
+            ("test_pathSeparator", test_pathSeparator),
+            ("test_pathSeparator2", test_pathSeparator2),
+        ])
+#endif
+
+        return tests
     }
 }