Skip to content

Commit d9a17bb

Browse files
committed
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.
1 parent 081798c commit d9a17bb

File tree

7 files changed

+173
-54
lines changed

7 files changed

+173
-54
lines changed

Sources/FoundationEssentials/FileManager/FileManager+Directories.swift

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -271,36 +271,49 @@ extension _FileManagerImpl {
271271
SECURITY_ATTRIBUTES(nLength: DWORD(MemoryLayout<SECURITY_ATTRIBUTES>.size),
272272
lpSecurityDescriptor: nil,
273273
bInheritHandle: false)
274-
// `SHCreateDirectoryExW` creates intermediate directories while `CreateDirectoryW` does not.
274+
// `CreateDirectoryW` does not create intermediate directories, so we need to handle that manually.
275+
// Note: `SHCreateDirectoryExW` seems to have issues with long paths.
275276
if createIntermediates {
276-
// `SHCreateDirectoryExW` requires an absolute path while `CreateDirectoryW` works based on the current working
277-
// directory.
278-
try path.withNTPathRepresentation { pwszPath in
279-
let errorCode = SHCreateDirectoryExW(nil, pwszPath, &saAttributes)
280-
guard let errorCode = DWORD(exactly: errorCode) else {
281-
// `SHCreateDirectoryExW` returns `Int` but all error codes are defined in terms of `DWORD`, aka
282-
// `UInt`. We received an unknown error code.
283-
throw CocoaError.errorWithFilePath(.fileWriteUnknown, path)
284-
}
285-
switch errorCode {
286-
case ERROR_SUCCESS:
287-
if let attributes {
288-
try? fileManager.setAttributes(attributes, ofItemAtPath: path)
277+
// Create intermediate directories recursively
278+
func createDirectoryRecursively(at directoryPath: String) throws {
279+
// Check if directory already exists
280+
var isDirectory: Bool = false
281+
if fileExists(atPath: directoryPath, isDirectory: &isDirectory) {
282+
if isDirectory {
283+
return // Directory already exists, nothing to do
284+
} else {
285+
// A file exists at this path, which is an error
286+
throw CocoaError.errorWithFilePath(directoryPath, win32: ERROR_ALREADY_EXISTS, reading: false)
289287
}
290-
case ERROR_ALREADY_EXISTS:
291-
var isDirectory: Bool = false
292-
if fileExists(atPath: path, isDirectory: &isDirectory), isDirectory {
293-
// A directory already exists at this path, which is not an error if we have
294-
// `createIntermediates == true`.
295-
break
288+
}
289+
290+
// Get parent directory
291+
let parentPath = directoryPath.deletingLastPathComponent()
292+
if !parentPath.isEmpty && parentPath != directoryPath {
293+
// Recursively create parent directory first
294+
try createDirectoryRecursively(at: parentPath)
295+
}
296+
297+
// Create this directory
298+
try directoryPath.withNTPathRepresentation { pwszPath in
299+
guard CreateDirectoryW(pwszPath, &saAttributes) else {
300+
let lastError = GetLastError()
301+
// If directory was created by another thread/process between our check and creation, that's OK
302+
if lastError == ERROR_ALREADY_EXISTS {
303+
var isDir: Bool = false
304+
if fileExists(atPath: directoryPath, isDirectory: &isDir), isDir {
305+
return // Directory now exists, success
306+
}
307+
}
308+
throw CocoaError.errorWithFilePath(directoryPath, win32: lastError, reading: false)
296309
}
297-
// A file (not a directory) exists at the given path or the file creation failed and the item
298-
// at this path has been deleted before the call to `fileExists`. Throw the original error.
299-
fallthrough
300-
default:
301-
throw CocoaError.errorWithFilePath(path, win32: errorCode, reading: false)
302310
}
303311
}
312+
313+
try createDirectoryRecursively(at: path)
314+
if let attributes {
315+
try? fileManager.setAttributes(attributes, ofItemAtPath: path)
316+
}
304317
} else {
305318
try path.withNTPathRepresentation { pwszPath in
306319
guard CreateDirectoryW(pwszPath, &saAttributes) else {
@@ -497,9 +510,14 @@ extension _FileManagerImpl {
497510
// This is solely to minimize the number of allocations and number of bytes allocated versus starting with a hardcoded value like MAX_PATH.
498511
// We should NOT early-return if this returns 0, in order to avoid TOCTOU issues.
499512
let dwSize = GetCurrentDirectoryW(0, nil)
500-
return try? FillNullTerminatedWideStringBuffer(initialSize: dwSize >= 0 ? dwSize : DWORD(MAX_PATH), maxSize: DWORD(Int16.max)) {
513+
let cwd = try? FillNullTerminatedWideStringBuffer(initialSize: dwSize >= 0 ? dwSize : DWORD(MAX_PATH), maxSize: DWORD(Int16.max)) {
501514
GetCurrentDirectoryW(DWORD($0.count), $0.baseAddress)
502515
}
516+
517+
// Handle Windows NT object namespace prefix
518+
// The \\?\ prefix is used by Windows NT for device paths and may appear
519+
// in current working directory paths. We strip it to return a standard path.
520+
return cwd?.removingNTPathPrefix()
503521
#else
504522
withUnsafeTemporaryAllocation(of: CChar.self, capacity: FileManager.MAX_PATH_SIZE) { buffer in
505523
guard getcwd(buffer.baseAddress!, FileManager.MAX_PATH_SIZE) != nil else {

Sources/FoundationEssentials/FileManager/FileOperations.swift

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -366,15 +366,14 @@ enum _FileOperations {
366366
var stack = [(path, false)]
367367
while let (directory, checked) = stack.popLast() {
368368
try directory.withNTPathRepresentation {
369-
let ntpath = String(decodingCString: $0, as: UTF16.self)
370-
371-
guard checked || filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
369+
let fullpath = String(decodingCString: $0, as: UTF16.self).removingNTPathPrefix()
372370

371+
guard checked || filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return }
373372
if RemoveDirectoryW($0) { return }
374373
let dwError: DWORD = GetLastError()
375374
guard dwError == ERROR_DIR_NOT_EMPTY else {
376375
let error = CocoaError.removeFileError(dwError, directory)
377-
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: ntpath) ?? false) else {
376+
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: fullpath) ?? false) else {
378377
throw error
379378
}
380379
return
@@ -383,29 +382,29 @@ enum _FileOperations {
383382

384383
for entry in _Win32DirectoryContentsSequence(path: directory, appendSlashForDirectory: false, prefix: [directory]) {
385384
try entry.fileNameWithPrefix.withNTPathRepresentation {
386-
let ntpath = String(decodingCString: $0, as: UTF16.self)
385+
let fullpath = String(decodingCString: $0, as: UTF16.self).removingNTPathPrefix()
387386

388387
if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY,
389388
entry.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != FILE_ATTRIBUTE_REPARSE_POINT {
390-
if filemanager?._shouldRemoveItemAtPath(ntpath) ?? true {
391-
stack.append((ntpath, true))
389+
if filemanager?._shouldRemoveItemAtPath(fullpath) ?? true {
390+
stack.append((fullpath, true))
392391
}
393392
} else {
394393
if entry.dwFileAttributes & FILE_ATTRIBUTE_READONLY == FILE_ATTRIBUTE_READONLY {
395394
guard SetFileAttributesW($0, entry.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY) else {
396-
throw CocoaError.removeFileError(GetLastError(), ntpath)
395+
throw CocoaError.removeFileError(GetLastError(), entry.fileName)
397396
}
398397
}
399398
if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY {
400-
guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
399+
guard filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return }
401400
if !RemoveDirectoryW($0) {
402401
let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
403402
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {
404403
throw error
405404
}
406405
}
407406
} else {
408-
guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
407+
guard filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return }
409408
if !DeleteFileW($0) {
410409
let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
411410
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {

Sources/FoundationEssentials/String/String+Internals.swift

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ extension String {
6666
// 2. Canonicalize the path.
6767
// This will add the \\?\ prefix if needed based on the path's length.
6868
var pwszCanonicalPath: LPWSTR?
69-
let flags: ULONG = PATHCCH_ALLOW_LONG_PATHS
69+
// Alway add the long path prefix since we don't know if this is a directory.
70+
let flags: ULONG = PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH
7071
let result = PathAllocCanonicalize(pwszFullPath.baseAddress, flags, &pwszCanonicalPath)
7172
if let pwszCanonicalPath {
7273
defer { LocalFree(pwszCanonicalPath) }
@@ -79,6 +80,32 @@ extension String {
7980
}
8081
}
8182
}
83+
/// Removes the Windows NT prefix for long file paths if present.
84+
/// The \\?\ prefix is used by Windows NT for device paths and may appear
85+
/// in paths returned by system APIs. This method provides a clean way to
86+
/// normalize such paths to standard format.
87+
///
88+
/// - Returns: A string with the NT object namespace prefix removed, or the original string if no prefix is found.
89+
package func removingNTPathPrefix() -> String {
90+
// Use Windows API PathCchStripPrefix for robust prefix handling
91+
return withCString(encodedAs: UTF16.self) { pwszPath in
92+
// Calculate required buffer size (original path length should be sufficient)
93+
let length = wcslen(pwszPath) + 1 // include null terminator
94+
95+
return withUnsafeTemporaryAllocation(of: WCHAR.self, capacity: Int(length)) { buffer in
96+
// Copy the original path to the buffer
97+
_ = buffer.initialize(from: UnsafeBufferPointer(start: pwszPath, count: Int(length)))
98+
99+
// Call PathCchStripPrefix (modifies buffer in place)
100+
_ = PathCchStripPrefix(buffer.baseAddress, buffer.count)
101+
102+
// Return the result regardless of success/failure
103+
// PathCchStripPrefix modifies the buffer in-place and returns S_OK on success
104+
// If it fails, the original path remains unchanged, which is the desired fallback
105+
return String(decodingCString: buffer.baseAddress!, as: UTF16.self)
106+
}
107+
}
108+
}
82109
}
83110
#endif
84111

Sources/FoundationEssentials/String/String+Path.swift

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -741,15 +741,7 @@ extension String {
741741
guard GetFinalPathNameByHandleW(hFile, $0.baseAddress, dwLength, VOLUME_NAME_DOS) == dwLength - 1 else {
742742
return nil
743743
}
744-
745-
let pathBaseAddress: UnsafePointer<WCHAR>
746-
if Array($0.prefix(4)) == Array(#"\\?\"#.utf16) {
747-
// When using `VOLUME_NAME_DOS`, the returned path uses `\\?\`.
748-
pathBaseAddress = UnsafePointer($0.baseAddress!.advanced(by: 4))
749-
} else {
750-
pathBaseAddress = UnsafePointer($0.baseAddress!)
751-
}
752-
return String(decodingCString: pathBaseAddress, as: UTF16.self)
744+
return String(decodingCString: UnsafePointer($0.baseAddress!), as: UTF16.self).removingNTPathPrefix()
753745
}
754746
}
755747
#else // os(Windows)

Sources/FoundationEssentials/WinSDK+Extensions.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,10 @@ package var PATHCCH_ALLOW_LONG_PATHS: ULONG {
229229
ULONG(WinSDK.PATHCCH_ALLOW_LONG_PATHS.rawValue)
230230
}
231231

232+
package var PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH: ULONG {
233+
ULONG(WinSDK.PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH.rawValue)
234+
}
235+
232236
package var RRF_RT_REG_SZ: DWORD {
233237
DWORD(WinSDK.RRF_RT_REG_SZ)
234238
}

Tests/FoundationEssentialsTests/FileManager/FileManagerTests.swift

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,12 @@ private struct FileManagerTests {
11331133
let fileName = UUID().uuidString
11341134
let cwd = fileManager.currentDirectoryPath
11351135

1136+
#expect(fileManager.changeCurrentDirectoryPath(cwd))
1137+
#expect(cwd == fileManager.currentDirectoryPath)
1138+
1139+
let nearLimitDir = cwd + "/" + String(repeating: "A", count: 255 - cwd.count)
1140+
#expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: nearLimitDir), withIntermediateDirectories: false) }
1141+
11361142
#expect(fileManager.createFile(atPath: dirName + "/" + fileName, contents: nil))
11371143

11381144
let dirURL = URL(filePath: dirName, directoryHint: .checkFileSystem)
@@ -1171,12 +1177,6 @@ private struct FileManagerTests {
11711177

11721178
#expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir1"), withIntermediateDirectories: false) }
11731179

1174-
// SHCreateDirectoryExW's path argument is limited to 248 characters, and the \\?\ prefix doesn't help.
1175-
// https://learn.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shcreatedirectoryexw
1176-
#expect(throws: (any Error).self) {
1177-
try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3"), withIntermediateDirectories: true)
1178-
}
1179-
11801180
// SetCurrentDirectory seems to be limited to MAX_PATH unconditionally, counter to the documentation.
11811181
// https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setcurrentdirectory
11821182
// https://github.com/MicrosoftDocs/feedback/issues/1441
@@ -1195,8 +1195,12 @@ private struct FileManagerTests {
11951195

11961196
#expect((cwd + "/" + dirName + "/" + "lnk").resolvingSymlinksInPath == (cwd + "/" + dirName + "/" + fileName).resolvingSymlinksInPath)
11971197

1198-
#expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2"), withIntermediateDirectories: false) }
1199-
#expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3"), withIntermediateDirectories: false) }
1198+
#expect(throws: Never.self) {
1199+
try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3"), withIntermediateDirectories: true)
1200+
}
1201+
#expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir4"), withIntermediateDirectories: false) }
1202+
#expect(throws: Never.self) { try fileManager.createDirectory(at: URL(fileURLWithPath: dirName + "/" + "subdir4" + "/" + "subdir5"), withIntermediateDirectories: false) }
1203+
12001204
#expect(throws: Never.self) { try Data().write(to: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile")) }
12011205
#expect(throws: Never.self) { try Data().write(to: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile2")) }
12021206
#expect(throws: Never.self) { try fileManager.moveItem(atPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile2", toPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile3") }
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#if os(Windows)
14+
15+
import Testing
16+
17+
#if FOUNDATION_FRAMEWORK
18+
import Foundation
19+
#else
20+
import FoundationEssentials
21+
#endif
22+
23+
@Suite("String NT Path Tests")
24+
struct StringNTPathTests {
25+
26+
@Test("Normal drive path, no prefix")
27+
func noPrefix() {
28+
let path = "C:\\Windows\\System32"
29+
#expect(path.removingNTPathPrefix() == "C:\\Windows\\System32")
30+
}
31+
32+
@Test("Extended-length path prefix (\\\\?\\)")
33+
func extendedPrefix() {
34+
let path = #"\\?\C:\Windows\System32"#
35+
#expect(path.removingNTPathPrefix() == "C:\\Windows\\System32")
36+
}
37+
38+
@Test("UNC path with extended prefix (\\\\?\\UNC\\)")
39+
func uncExtendedPrefix() {
40+
let path = #"\\?\UNC\Server\Share\Folder"#
41+
#expect(path.removingNTPathPrefix() == #"\\Server\Share\Folder"#)
42+
}
43+
44+
@Test("UNC path without extended prefix")
45+
func uncNormal() {
46+
let path = #"\\Server\Share\Folder"#
47+
#expect(path.removingNTPathPrefix() == #"\\Server\Share\Folder"#)
48+
}
49+
50+
@Test("Empty string should stay empty")
51+
func emptyString() {
52+
let path = ""
53+
#expect(path.removingNTPathPrefix() == "")
54+
}
55+
56+
@Test("Path with only prefix should return empty")
57+
func prefixOnly() {
58+
let path = #"\\?\C:\"#
59+
#expect(path.removingNTPathPrefix() == #"C:\"#)
60+
}
61+
62+
@Test("Path longer than MAX_PATH (260 chars)")
63+
func longPathBeyondMaxPath() {
64+
// Create a folder name repeated to exceed 260 chars
65+
let longComponent = String(repeating: "A", count: 280)
66+
let rawPath = #"\\?\C:\Test\"# + longComponent
67+
68+
// After stripping, it should drop the \\?\ prefix but keep the full long component
69+
let expected = "C:\\Test\\" + longComponent
70+
71+
let stripped = rawPath.removingNTPathPrefix()
72+
#expect(stripped == expected)
73+
}
74+
}
75+
#endif

0 commit comments

Comments
 (0)