diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml new file mode 100644 index 00000000..d543be29 --- /dev/null +++ b/.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 diff --git a/Sources/System/FilePath/FilePathTempWindows.swift b/Sources/System/FilePath/FilePathTempWindows.swift index d6e45f4f..259aaafe 100644 --- a/Sources/System/FilePath/FilePathTempWindows.swift +++ b/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()) } } diff --git a/Sources/System/FilePath/FilePathWindows.swift b/Sources/System/FilePath/FilePathWindows.swift index b725dd17..2226816e 100644 --- a/Sources/System/FilePath/FilePathWindows.swift +++ b/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(_ 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.. 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 diff --git a/Sources/System/Internals/Mocking.swift b/Sources/System/Internals/Mocking.swift index 2945651c..ffdaaa92 100644 --- a/Sources/System/Internals/Mocking.swift +++ b/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 diff --git a/Sources/System/Internals/WindowsSyscallAdapters.swift b/Sources/System/Internals/WindowsSyscallAdapters.swift index 706881ec..6437d16a 100644 --- a/Sources/System/Internals/WindowsSyscallAdapters.swift +++ b/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 ) -> CInt { - if !RemoveDirectoryW(path) { + guard (try? path.withCanonicalPathRepresentation({ path in RemoveDirectoryW(path) })) == true else { ucrt._set_errno(_mapWindowsErrorToErrno(GetLastError())) return -1 } diff --git a/Tests/SystemTests/FileOperationsTest.swift b/Tests/SystemTests/FileOperationsTest.swift index ed05dcf4..18adde37 100644 --- a/Tests/SystemTests/FileOperationsTest.swift +++ b/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 { diff --git a/Tests/SystemTests/FileOperationsTestWindows.swift b/Tests/SystemTests/FileOperationsTestWindows.swift index 82030516..7b87b354 100644 --- a/Tests/SystemTests/FileOperationsTestWindows.swift +++ b/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 = [ diff --git a/Tests/SystemTests/FileTypesTest.swift b/Tests/SystemTests/FileTypesTest.swift index 58ceac1b..5258709a 100644 --- a/Tests/SystemTests/FileTypesTest.swift +++ b/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 {