From 3ec5ddc7bc70d14f3575b725876fcd9827cf9ff5 Mon Sep 17 00:00:00 2001 From: Dave Inglis Date: Fri, 22 Aug 2025 11:09:39 -0400 Subject: [PATCH] Fix a corner case where createDirectory would fail on windows The //?/ prefix needs to be added to directories if the length is greater than MAX_PATH - 12, however the PATHCCH_ALLOW_LONG_PATHS to PathAllocCanonicalize would only do that if the path was greater than MAX_PATH. This change uses PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH, which forces it on all the time. Just needed to remove the prefix in a few places as it was making it way out into tests as this is suppose to be transparent to the user. This also fixes createDirectory() "withIntermediateDirectories: true" for long paths on Windows. --- .../FileManager/FileManager+Directories.swift | 71 +++++++++++------- .../FileManager/FileOperations.swift | 19 +++-- .../String/String+Internals.swift | 29 ++++++- .../String/String+Path.swift | 10 +-- .../WinSDK+Extensions.swift | 4 + .../FileManager/FileManagerTests.swift | 20 +++-- .../String/StringNTPathTests.swift | 75 +++++++++++++++++++ 7 files changed, 173 insertions(+), 55 deletions(-) create mode 100644 Tests/FoundationEssentialsTests/String/StringNTPathTests.swift diff --git a/Sources/FoundationEssentials/FileManager/FileManager+Directories.swift b/Sources/FoundationEssentials/FileManager/FileManager+Directories.swift index e1db5a9f4..aa7c6eee5 100644 --- a/Sources/FoundationEssentials/FileManager/FileManager+Directories.swift +++ b/Sources/FoundationEssentials/FileManager/FileManager+Directories.swift @@ -271,36 +271,48 @@ extension _FileManagerImpl { SECURITY_ATTRIBUTES(nLength: DWORD(MemoryLayout.size), lpSecurityDescriptor: nil, bInheritHandle: false) - // `SHCreateDirectoryExW` creates intermediate directories while `CreateDirectoryW` does not. + // `CreateDirectoryW` does not create intermediate directories, so we need to handle that manually. + // Note: `SHCreateDirectoryExW` seems to have issues with long paths. if createIntermediates { - // `SHCreateDirectoryExW` requires an absolute path while `CreateDirectoryW` works based on the current working - // directory. - try path.withNTPathRepresentation { pwszPath in - let errorCode = SHCreateDirectoryExW(nil, pwszPath, &saAttributes) - guard let errorCode = DWORD(exactly: errorCode) else { - // `SHCreateDirectoryExW` returns `Int` but all error codes are defined in terms of `DWORD`, aka - // `UInt`. We received an unknown error code. - throw CocoaError.errorWithFilePath(.fileWriteUnknown, path) - } - switch errorCode { - case ERROR_SUCCESS: - if let attributes { - try? fileManager.setAttributes(attributes, ofItemAtPath: path) - } - case ERROR_ALREADY_EXISTS: - var isDirectory: Bool = false - if fileExists(atPath: path, isDirectory: &isDirectory), isDirectory { - // A directory already exists at this path, which is not an error if we have - // `createIntermediates == true`. - break + // Create intermediate directories recursively + func _createDirectoryRecursively(at directoryPath: String) throws { + try directoryPath.withNTPathRepresentation { pwszPath in + // Create this directory + guard CreateDirectoryW(pwszPath, &saAttributes) else { + let lastError = GetLastError() + if lastError == ERROR_ALREADY_EXISTS { + var isDir: Bool = false + if fileExists(atPath: directoryPath, isDirectory: &isDir), isDir { + return // Directory now exists, success + } + } else if lastError == ERROR_PATH_NOT_FOUND { + let parentPath = directoryPath.deletingLastPathComponent() + if !parentPath.isEmpty && parentPath != directoryPath { + // Recursively create parent directory + try _createDirectoryRecursively(at: parentPath) + // Now try creating this one again. + guard CreateDirectoryW(pwszPath, &saAttributes) else { + let lastError = GetLastError() + if lastError == ERROR_ALREADY_EXISTS { + var isDir: Bool = false + if fileExists(atPath: directoryPath, isDirectory: &isDir), isDir { + return // Directory now exists, success + } + } + throw CocoaError.errorWithFilePath(directoryPath, win32: lastError, reading: false) + } + return + } + } + throw CocoaError.errorWithFilePath(directoryPath, win32: lastError, reading: false) } - // A file (not a directory) exists at the given path or the file creation failed and the item - // at this path has been deleted before the call to `fileExists`. Throw the original error. - fallthrough - default: - throw CocoaError.errorWithFilePath(path, win32: errorCode, reading: false) } } + + try _createDirectoryRecursively(at: path) + if let attributes { + try? fileManager.setAttributes(attributes, ofItemAtPath: path) + } } else { try path.withNTPathRepresentation { pwszPath in guard CreateDirectoryW(pwszPath, &saAttributes) else { @@ -497,9 +509,14 @@ extension _FileManagerImpl { // This is solely to minimize the number of allocations and number of bytes allocated versus starting with a hardcoded value like MAX_PATH. // We should NOT early-return if this returns 0, in order to avoid TOCTOU issues. let dwSize = GetCurrentDirectoryW(0, nil) - return try? FillNullTerminatedWideStringBuffer(initialSize: dwSize >= 0 ? dwSize : DWORD(MAX_PATH), maxSize: DWORD(Int16.max)) { + let cwd = try? FillNullTerminatedWideStringBuffer(initialSize: dwSize >= 0 ? dwSize : DWORD(MAX_PATH), maxSize: DWORD(Int16.max)) { GetCurrentDirectoryW(DWORD($0.count), $0.baseAddress) } + + // Handle Windows NT object namespace prefix + // The \\?\ prefix is used by Windows NT for device paths and may appear + // in current working directory paths. We strip it to return a standard path. + return cwd?.removingNTPathPrefix() #else withUnsafeTemporaryAllocation(of: CChar.self, capacity: FileManager.MAX_PATH_SIZE) { buffer in guard getcwd(buffer.baseAddress!, FileManager.MAX_PATH_SIZE) != nil else { diff --git a/Sources/FoundationEssentials/FileManager/FileOperations.swift b/Sources/FoundationEssentials/FileManager/FileOperations.swift index 1c28880ac..be9320bb0 100644 --- a/Sources/FoundationEssentials/FileManager/FileOperations.swift +++ b/Sources/FoundationEssentials/FileManager/FileOperations.swift @@ -366,15 +366,14 @@ enum _FileOperations { var stack = [(path, false)] while let (directory, checked) = stack.popLast() { try directory.withNTPathRepresentation { - let ntpath = String(decodingCString: $0, as: UTF16.self) - - guard checked || filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return } + let fullpath = String(decodingCString: $0, as: UTF16.self).removingNTPathPrefix() + guard checked || filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return } if RemoveDirectoryW($0) { return } let dwError: DWORD = GetLastError() guard dwError == ERROR_DIR_NOT_EMPTY else { let error = CocoaError.removeFileError(dwError, directory) - guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: ntpath) ?? false) else { + guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: fullpath) ?? false) else { throw error } return @@ -383,21 +382,21 @@ enum _FileOperations { for entry in _Win32DirectoryContentsSequence(path: directory, appendSlashForDirectory: false, prefix: [directory]) { try entry.fileNameWithPrefix.withNTPathRepresentation { - let ntpath = String(decodingCString: $0, as: UTF16.self) + let fullpath = String(decodingCString: $0, as: UTF16.self).removingNTPathPrefix() if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY, entry.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != FILE_ATTRIBUTE_REPARSE_POINT { - if filemanager?._shouldRemoveItemAtPath(ntpath) ?? true { - stack.append((ntpath, true)) + if filemanager?._shouldRemoveItemAtPath(fullpath) ?? true { + stack.append((fullpath, true)) } } else { if entry.dwFileAttributes & FILE_ATTRIBUTE_READONLY == FILE_ATTRIBUTE_READONLY { guard SetFileAttributesW($0, entry.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY) else { - throw CocoaError.removeFileError(GetLastError(), ntpath) + throw CocoaError.removeFileError(GetLastError(), entry.fileName) } } if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY { - guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return } + guard filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return } if !RemoveDirectoryW($0) { let error = CocoaError.removeFileError(GetLastError(), entry.fileName) guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else { @@ -405,7 +404,7 @@ enum _FileOperations { } } } else { - guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return } + guard filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return } if !DeleteFileW($0) { let error = CocoaError.removeFileError(GetLastError(), entry.fileName) guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else { diff --git a/Sources/FoundationEssentials/String/String+Internals.swift b/Sources/FoundationEssentials/String/String+Internals.swift index 16193afda..35de06696 100644 --- a/Sources/FoundationEssentials/String/String+Internals.swift +++ b/Sources/FoundationEssentials/String/String+Internals.swift @@ -66,7 +66,8 @@ extension String { // 2. Canonicalize the path. // This will add the \\?\ prefix if needed based on the path's length. var pwszCanonicalPath: LPWSTR? - let flags: ULONG = PATHCCH_ALLOW_LONG_PATHS + // Alway add the long path prefix since we don't know if this is a directory. + let flags: ULONG = PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH let result = PathAllocCanonicalize(pwszFullPath.baseAddress, flags, &pwszCanonicalPath) if let pwszCanonicalPath { defer { LocalFree(pwszCanonicalPath) } @@ -79,6 +80,32 @@ extension String { } } } + /// Removes the Windows NT prefix for long file paths if present. + /// The \\?\ prefix is used by Windows NT for device paths and may appear + /// in paths returned by system APIs. This method provides a clean way to + /// normalize such paths to standard format. + /// + /// - Returns: A string with the NT object namespace prefix removed, or the original string if no prefix is found. + package func removingNTPathPrefix() -> String { + // Use Windows API PathCchStripPrefix for robust prefix handling + return withCString(encodedAs: UTF16.self) { pwszPath in + // Calculate required buffer size (original path length should be sufficient) + let length = wcslen(pwszPath) + 1 // include null terminator + + return withUnsafeTemporaryAllocation(of: WCHAR.self, capacity: Int(length)) { buffer in + // Copy the original path to the buffer + _ = buffer.initialize(from: UnsafeBufferPointer(start: pwszPath, count: Int(length))) + + // Call PathCchStripPrefix (modifies buffer in place) + _ = PathCchStripPrefix(buffer.baseAddress, buffer.count) + + // Return the result regardless of success/failure + // PathCchStripPrefix modifies the buffer in-place and returns S_OK on success + // If it fails, the original path remains unchanged, which is the desired fallback + return String(decodingCString: buffer.baseAddress!, as: UTF16.self) + } + } + } } #endif diff --git a/Sources/FoundationEssentials/String/String+Path.swift b/Sources/FoundationEssentials/String/String+Path.swift index bc149baac..449e04537 100644 --- a/Sources/FoundationEssentials/String/String+Path.swift +++ b/Sources/FoundationEssentials/String/String+Path.swift @@ -741,15 +741,7 @@ extension String { guard GetFinalPathNameByHandleW(hFile, $0.baseAddress, dwLength, VOLUME_NAME_DOS) == dwLength - 1 else { return nil } - - let pathBaseAddress: UnsafePointer - if Array($0.prefix(4)) == Array(#"\\?\"#.utf16) { - // When using `VOLUME_NAME_DOS`, the returned path uses `\\?\`. - pathBaseAddress = UnsafePointer($0.baseAddress!.advanced(by: 4)) - } else { - pathBaseAddress = UnsafePointer($0.baseAddress!) - } - return String(decodingCString: pathBaseAddress, as: UTF16.self) + return String(decodingCString: UnsafePointer($0.baseAddress!), as: UTF16.self).removingNTPathPrefix() } } #else // os(Windows) diff --git a/Sources/FoundationEssentials/WinSDK+Extensions.swift b/Sources/FoundationEssentials/WinSDK+Extensions.swift index 6322c4c86..c95bebc18 100644 --- a/Sources/FoundationEssentials/WinSDK+Extensions.swift +++ b/Sources/FoundationEssentials/WinSDK+Extensions.swift @@ -242,6 +242,10 @@ package var PATHCCH_ALLOW_LONG_PATHS: ULONG { ULONG(WinSDK.PATHCCH_ALLOW_LONG_PATHS.rawValue) } +package var PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH: ULONG { + ULONG(WinSDK.PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH.rawValue) +} + package var RRF_RT_REG_SZ: DWORD { DWORD(WinSDK.RRF_RT_REG_SZ) } diff --git a/Tests/FoundationEssentialsTests/FileManager/FileManagerTests.swift b/Tests/FoundationEssentialsTests/FileManager/FileManagerTests.swift index 44893c17e..2f5e4afea 100644 --- a/Tests/FoundationEssentialsTests/FileManager/FileManagerTests.swift +++ b/Tests/FoundationEssentialsTests/FileManager/FileManagerTests.swift @@ -1133,6 +1133,12 @@ private struct FileManagerTests { let fileName = UUID().uuidString let cwd = fileManager.currentDirectoryPath + #expect(fileManager.changeCurrentDirectoryPath(cwd)) + #expect(cwd == fileManager.currentDirectoryPath) + + let nearLimitDir = cwd + "/" + String(repeating: "A", count: 255 - cwd.count) + #expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: nearLimitDir), withIntermediateDirectories: false) } + #expect(fileManager.createFile(atPath: dirName + "/" + fileName, contents: nil)) let dirURL = URL(filePath: dirName, directoryHint: .checkFileSystem) @@ -1171,12 +1177,6 @@ private struct FileManagerTests { #expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir1"), withIntermediateDirectories: false) } - // SHCreateDirectoryExW's path argument is limited to 248 characters, and the \\?\ prefix doesn't help. - // https://learn.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shcreatedirectoryexw - #expect(throws: (any Error).self) { - try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3"), withIntermediateDirectories: true) - } - // SetCurrentDirectory seems to be limited to MAX_PATH unconditionally, counter to the documentation. // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setcurrentdirectory // https://github.com/MicrosoftDocs/feedback/issues/1441 @@ -1195,8 +1195,12 @@ private struct FileManagerTests { #expect((cwd + "/" + dirName + "/" + "lnk").resolvingSymlinksInPath == (cwd + "/" + dirName + "/" + fileName).resolvingSymlinksInPath) - #expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2"), withIntermediateDirectories: false) } - #expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3"), withIntermediateDirectories: false) } + #expect(throws: Never.self) { + try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3"), withIntermediateDirectories: true) + } + #expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir4"), withIntermediateDirectories: false) } + #expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir4" + "/" + "subdir5"), withIntermediateDirectories: false) } + #expect(throws: Never.self) { try Data().write(to: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile")) } #expect(throws: Never.self) { try Data().write(to: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile2")) } #expect(throws: Never.self) { try fileManager.moveItem(atPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile2", toPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile3") } diff --git a/Tests/FoundationEssentialsTests/String/StringNTPathTests.swift b/Tests/FoundationEssentialsTests/String/StringNTPathTests.swift new file mode 100644 index 000000000..60041cf96 --- /dev/null +++ b/Tests/FoundationEssentialsTests/String/StringNTPathTests.swift @@ -0,0 +1,75 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2023 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 +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +#if os(Windows) + +import Testing + +#if FOUNDATION_FRAMEWORK +import Foundation +#else +import FoundationEssentials +#endif + +@Suite("String NT Path Tests") +struct StringNTPathTests { + + @Test("Normal drive path, no prefix") + func noPrefix() { + let path = "C:\\Windows\\System32" + #expect(path.removingNTPathPrefix() == "C:\\Windows\\System32") + } + + @Test("Extended-length path prefix (\\\\?\\)") + func extendedPrefix() { + let path = #"\\?\C:\Windows\System32"# + #expect(path.removingNTPathPrefix() == "C:\\Windows\\System32") + } + + @Test("UNC path with extended prefix (\\\\?\\UNC\\)") + func uncExtendedPrefix() { + let path = #"\\?\UNC\Server\Share\Folder"# + #expect(path.removingNTPathPrefix() == #"\\Server\Share\Folder"#) + } + + @Test("UNC path without extended prefix") + func uncNormal() { + let path = #"\\Server\Share\Folder"# + #expect(path.removingNTPathPrefix() == #"\\Server\Share\Folder"#) + } + + @Test("Empty string should stay empty") + func emptyString() { + let path = "" + #expect(path.removingNTPathPrefix() == "") + } + + @Test("Path with only prefix should return empty") + func prefixOnly() { + let path = #"\\?\C:\"# + #expect(path.removingNTPathPrefix() == #"C:\"#) + } + + @Test("Path longer than MAX_PATH (260 chars)") + func longPathBeyondMaxPath() { + // Create a folder name repeated to exceed 260 chars + let longComponent = String(repeating: "A", count: 280) + let rawPath = #"\\?\C:\Test\"# + longComponent + + // After stripping, it should drop the \\?\ prefix but keep the full long component + let expected = "C:\\Test\\" + longComponent + + let stripped = rawPath.removingNTPathPrefix() + #expect(stripped == expected) + } +} +#endif \ No newline at end of file