Merge pull request #231 from apple/main

Merge main into release/1.5.0

Author: glessard

.github/workflows/pull_request.yml

@@ -0,0 +1,23 @@
+name: Pull request
+
+on:
+  pull_request:
+    types: [opened, reopened, synchronize]
+
+jobs:
+  tests:
+    name: Test
+    uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main
+    with:
+      linux_exclude_swift_versions: '[{"swift_version": "5.8"}]'
+  soundness:
+    name: Soundness
+    uses: swiftlang/github-workflows/.github/workflows/soundness.yml@main
+    with:
+      license_header_check_project_name: "Swift.org"
+      # https://github.com/apple/swift-system/issues/224
+      docs_check_enabled: false
+      unacceptable_language_check_enabled: false
+      license_header_check_enabled: false
+      format_check_enabled: false
+      python_lint_check_enabled: false

Sources/System/FilePath/FilePathTempWindows.swift

@@ -40,7 +40,7 @@ fileprivate func forEachFile(
 
   try searchPath.withPlatformString { szPath in
     var findData = WIN32_FIND_DATAW()
-    let hFind = FindFirstFileW(szPath, &findData)
+    let hFind = try szPath.withCanonicalPathRepresentation({ szPath in FindFirstFileW(szPath, &findData) })
     if hFind == INVALID_HANDLE_VALUE {
       throw Errno(windowsError: GetLastError())
     }
@@ -95,8 +95,8 @@ internal func _recursiveRemove(
     let subpath = path.appending(component)
 
     if (findData.dwFileAttributes & DWORD(FILE_ATTRIBUTE_DIRECTORY)) == 0 {
-      try subpath.withPlatformString {
-        if !DeleteFileW($0) {
+      try subpath.withPlatformString { subpath in
+        if try !subpath.withCanonicalPathRepresentation({ DeleteFileW($0) }) {
           throw Errno(windowsError: GetLastError())
         }
       }
@@ -105,7 +105,7 @@ internal func _recursiveRemove(
 
   // Finally, delete the parent
   try path.withPlatformString {
-    if !RemoveDirectoryW($0) {
+    if try !$0.withCanonicalPathRepresentation({ RemoveDirectoryW($0) }) {
       throw Errno(windowsError: GetLastError())
     }
   }

Sources/System/FilePath/FilePathWindows.swift

@@ -461,3 +461,78 @@ extension SystemString {
     return lexer.current
   }
 }
+
+#if os(Windows)
+import WinSDK
+
+// FIXME: Rather than canonicalizing the path at every call site to a Win32 API,
+// we should consider always storing absolute paths with the \\?\ prefix applied,
+// for better performance.
+extension UnsafePointer where Pointee == CInterop.PlatformChar {
+  /// Invokes `body` with a resolved and potentially `\\?\`-prefixed version of the pointee,
+  /// to ensure long paths greater than MAX_PATH (260) characters are handled correctly.
+  ///
+  /// - seealso: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
+  internal func withCanonicalPathRepresentation<Result>(_ body: (Self) throws -> Result) throws -> Result {
+    // 1. Normalize the path first.
+    // Contrary to the documentation, this works on long paths independently
+    // of the registry or process setting to enable long paths (but it will also
+    // not add the \\?\ prefix required by other functions under these conditions).
+    let dwLength: DWORD = GetFullPathNameW(self, 0, nil, nil)
+    return try withUnsafeTemporaryAllocation(of: WCHAR.self, capacity: Int(dwLength)) { pwszFullPath in
+      guard (1..<dwLength).contains(GetFullPathNameW(self, DWORD(pwszFullPath.count), pwszFullPath.baseAddress, nil)) else {
+        throw Errno(rawValue: _mapWindowsErrorToErrno(GetLastError()))
+      }
+
+      // 1.5 Leave \\.\ prefixed paths alone since device paths are already an exact representation and PathCchCanonicalizeEx will mangle these.
+      if let base = pwszFullPath.baseAddress,
+        base[0] == UInt8(ascii: "\\"),
+        base[1] == UInt8(ascii: "\\"),
+        base[2] == UInt8(ascii: "."),
+        base[3] == UInt8(ascii: "\\") {
+        return try body(base)
+      }
+
+      // 2. Canonicalize the path.
+      // This will add the \\?\ prefix if needed based on the path's length.
+      var pwszCanonicalPath: LPWSTR?
+      let flags: ULONG = numericCast(PATHCCH_ALLOW_LONG_PATHS.rawValue)
+      let result = PathAllocCanonicalize(pwszFullPath.baseAddress, flags, &pwszCanonicalPath)
+      if let pwszCanonicalPath {
+          defer { LocalFree(pwszCanonicalPath) }
+          if result == S_OK {
+            // 3. Perform the operation on the normalized path.
+            return try body(pwszCanonicalPath)
+          }
+      }
+      throw Errno(rawValue: _mapWindowsErrorToErrno(WIN32_FROM_HRESULT(result)))
+    }
+  }
+}
+
+@inline(__always)
+fileprivate func HRESULT_CODE(_ hr: HRESULT) -> DWORD {
+    DWORD(hr) & 0xffff
+}
+
+@inline(__always)
+fileprivate func HRESULT_FACILITY(_ hr: HRESULT) -> DWORD {
+    DWORD(hr << 16) & 0x1fff
+}
+
+@inline(__always)
+fileprivate func SUCCEEDED(_ hr: HRESULT) -> Bool {
+    hr >= 0
+}
+
+// This is a non-standard extension to the Windows SDK that allows us to convert
+// an HRESULT to a Win32 error code.
+@inline(__always)
+fileprivate func WIN32_FROM_HRESULT(_ hr: HRESULT) -> DWORD {
+    if SUCCEEDED(hr) { return DWORD(ERROR_SUCCESS) }
+    if HRESULT_FACILITY(hr) == FACILITY_WIN32 {
+        return HRESULT_CODE(hr)
+    }
+    return DWORD(hr)
+}
+#endif

Sources/System/Internals/Mocking.swift

@@ -130,7 +130,7 @@ internal var mockingEnabled: Bool {
 @inline(__always)
 internal var forceWindowsPaths: Bool? {
   #if !ENABLE_MOCKING
-  return false
+  return nil
   #else
   return MockingDriver.forceWindowsPaths
   #endif

Sources/System/Internals/WindowsSyscallAdapters.swift

@@ -35,17 +35,17 @@ internal func open(
     bInheritHandle: decodedFlags.bInheritHandle
   )
 
-  let hFile = CreateFileW(path,
-                          decodedFlags.dwDesiredAccess,
-                          DWORD(FILE_SHARE_DELETE
-                                  | FILE_SHARE_READ
-                                  | FILE_SHARE_WRITE),
-                          &saAttrs,
-                          decodedFlags.dwCreationDisposition,
-                          decodedFlags.dwFlagsAndAttributes,
-                          nil)
-
-  if hFile == INVALID_HANDLE_VALUE {
+  guard let hFile = try? path.withCanonicalPathRepresentation({ path in
+    CreateFileW(path,
+                decodedFlags.dwDesiredAccess,
+                DWORD(FILE_SHARE_DELETE
+                      | FILE_SHARE_READ
+                      | FILE_SHARE_WRITE),
+                &saAttrs,
+                decodedFlags.dwCreationDisposition,
+                decodedFlags.dwFlagsAndAttributes,
+                nil)
+  }), hFile != INVALID_HANDLE_VALUE else {
     ucrt._set_errno(_mapWindowsErrorToErrno(GetLastError()))
     return -1
   }
@@ -77,17 +77,17 @@ internal func open(
     bInheritHandle: decodedFlags.bInheritHandle
   )
 
-  let hFile = CreateFileW(path,
-                          decodedFlags.dwDesiredAccess,
-                          DWORD(FILE_SHARE_DELETE
-                                  | FILE_SHARE_READ
-                                  | FILE_SHARE_WRITE),
-                          &saAttrs,
-                          decodedFlags.dwCreationDisposition,
-                          decodedFlags.dwFlagsAndAttributes,
-                          nil)
-
-  if hFile == INVALID_HANDLE_VALUE {
+  guard let hFile = try? path.withCanonicalPathRepresentation({ path in
+    CreateFileW(path,
+                decodedFlags.dwDesiredAccess,
+                DWORD(FILE_SHARE_DELETE
+                      | FILE_SHARE_READ
+                      | FILE_SHARE_WRITE),
+                &saAttrs,
+                decodedFlags.dwCreationDisposition,
+                decodedFlags.dwFlagsAndAttributes,
+                nil)
+  }), hFile != INVALID_HANDLE_VALUE else {
     ucrt._set_errno(_mapWindowsErrorToErrno(GetLastError()))
     return -1
   }
@@ -242,7 +242,7 @@ internal func mkdir(
     bInheritHandle: false
   )
 
-  if !CreateDirectoryW(path, &saAttrs) {
+  guard (try? path.withCanonicalPathRepresentation({ path in CreateDirectoryW(path, &saAttrs) })) == true else {
     ucrt._set_errno(_mapWindowsErrorToErrno(GetLastError()))
     return -1
   }
@@ -254,7 +254,7 @@ internal func mkdir(
 internal func rmdir(
   _ path: UnsafePointer<CInterop.PlatformChar>
 ) -> CInt {
-  if !RemoveDirectoryW(path) {
+  guard (try? path.withCanonicalPathRepresentation({ path in RemoveDirectoryW(path) })) == true else {
     ucrt._set_errno(_mapWindowsErrorToErrno(GetLastError()))
     return -1
   }

Tests/SystemTests/FileOperationsTest.swift

@@ -14,6 +14,9 @@ import XCTest
 #else
 @testable import System
 #endif
+#if canImport(Android)
+import Android
+#endif
 
 @available(/*System 0.0.1: macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0*/iOS 8, *)
 final class FileOperationsTest: XCTestCase {

Tests/SystemTests/FileOperationsTestWindows.swift

@@ -174,6 +174,9 @@ final class FileOperationsTestWindows: XCTestCase {
 
   /// Test that the umask works properly
   func testUmask() throws {
+    // See https://learn.microsoft.com/en-us/virtualization/windowscontainers/manage-containers/persistent-storage#permissions
+    try XCTSkipIf(NSUserName() == "ContainerAdministrator", "containers use a different permission model")
+    
     // Default mask should be 0o022
     XCTAssertEqual(FilePermissions.creationMask, [.groupWrite, .otherWrite])
 
@@ -205,6 +208,9 @@ final class FileOperationsTestWindows: XCTestCase {
 
   /// Test that setting permissions on a file works as expected
   func testPermissions() throws {
+    // See https://learn.microsoft.com/en-us/virtualization/windowscontainers/manage-containers/persistent-storage#permissions
+    try XCTSkipIf(NSUserName() == "ContainerAdministrator", "containers use a different permission model")
+
     try FilePermissions.withCreationMask([]) {
       try withTemporaryFilePath(basename: "testPermissions") { path in
         let tests = [

Tests/SystemTests/FileTypesTest.swift

@@ -14,6 +14,9 @@ import SystemPackage
 #else
 import System
 #endif
+#if canImport(Android)
+import Android
+#endif
 
 @available(/*System 0.0.1: macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0*/iOS 8, *)
 final class FileDescriptorTest: XCTestCase {