Skip to content

Commit 039c330

Browse files
jrflatjrflat
authored
Revert "Fix a corner case where createDirectory would fail on windows (#1479)" (#1512)
This reverts commit aa97b79.
1 parent 27effcb commit 039c330

File tree

7 files changed

+55
-173
lines changed

7 files changed

+55
-173
lines changed

‎Sources/FoundationEssentials/FileManager/FileManager+Directories.swift‎

Lines changed: 27 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -271,48 +271,36 @@ extension _FileManagerImpl {
271271
SECURITY_ATTRIBUTES(nLength: DWORD(MemoryLayout<SECURITY_ATTRIBUTES>.size),
272272
lpSecurityDescriptor: nil,
273273
bInheritHandle: false)
274-
// `CreateDirectoryW` does not create intermediate directories, so we need to handle that manually.
275-
// Note: `SHCreateDirectoryExW` seems to have issues with long paths.
274+
// `SHCreateDirectoryExW` creates intermediate directories while `CreateDirectoryW` does not.
276275
if createIntermediates {
277-
// Create intermediate directories recursively
278-
func _createDirectoryRecursively(at directoryPath: String) throws {
279-
try directoryPath.withNTPathRepresentation { pwszPath in
280-
// Create this directory
281-
guard CreateDirectoryW(pwszPath, &saAttributes) else {
282-
let lastError = GetLastError()
283-
if lastError == ERROR_ALREADY_EXISTS {
284-
var isDir: Bool = false
285-
if fileExists(atPath: directoryPath, isDirectory: &isDir), isDir {
286-
return // Directory now exists, success
287-
}
288-
} else if lastError == ERROR_PATH_NOT_FOUND {
289-
let parentPath = directoryPath.deletingLastPathComponent()
290-
if !parentPath.isEmpty && parentPath != directoryPath {
291-
// Recursively create parent directory
292-
try _createDirectoryRecursively(at: parentPath)
293-
// Now try creating this one again.
294-
guard CreateDirectoryW(pwszPath, &saAttributes) else {
295-
let lastError = GetLastError()
296-
if lastError == ERROR_ALREADY_EXISTS {
297-
var isDir: Bool = false
298-
if fileExists(atPath: directoryPath, isDirectory: &isDir), isDir {
299-
return // Directory now exists, success
300-
}
301-
}
302-
throw CocoaError.errorWithFilePath(directoryPath, win32: lastError, reading: false)
303-
}
304-
return
305-
}
306-
}
307-
throw CocoaError.errorWithFilePath(directoryPath, win32: lastError, reading: false)
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)
289+
}
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
308296
}
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)
309302
}
310303
}
311-
312-
try _createDirectoryRecursively(at: path)
313-
if let attributes {
314-
try? fileManager.setAttributes(attributes, ofItemAtPath: path)
315-
}
316304
} else {
317305
try path.withNTPathRepresentation { pwszPath in
318306
guard CreateDirectoryW(pwszPath, &saAttributes) else {
@@ -509,14 +497,9 @@ extension _FileManagerImpl {
509497
// This is solely to minimize the number of allocations and number of bytes allocated versus starting with a hardcoded value like MAX_PATH.
510498
// We should NOT early-return if this returns 0, in order to avoid TOCTOU issues.
511499
let dwSize = GetCurrentDirectoryW(0, nil)
512-
let cwd = try? FillNullTerminatedWideStringBuffer(initialSize: dwSize >= 0 ? dwSize : DWORD(MAX_PATH), maxSize: DWORD(Int16.max)) {
500+
return try? FillNullTerminatedWideStringBuffer(initialSize: dwSize >= 0 ? dwSize : DWORD(MAX_PATH), maxSize: DWORD(Int16.max)) {
513501
GetCurrentDirectoryW(DWORD($0.count), $0.baseAddress)
514502
}
515-
516-
// Handle Windows NT object namespace prefix
517-
// The \\?\ prefix is used by Windows NT for device paths and may appear
518-
// in current working directory paths. We strip it to return a standard path.
519-
return cwd?.removingNTPathPrefix()
520503
#else
521504
withUnsafeTemporaryAllocation(of: CChar.self, capacity: FileManager.MAX_PATH_SIZE) { buffer in
522505
guard getcwd(buffer.baseAddress!, FileManager.MAX_PATH_SIZE) != nil else {

‎Sources/FoundationEssentials/FileManager/FileOperations.swift‎

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

371-
guard checked || filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return }
372373
if RemoveDirectoryW($0) { return }
373374
let dwError: DWORD = GetLastError()
374375
guard dwError == ERROR_DIR_NOT_EMPTY else {
375376
let error = CocoaError.removeFileError(dwError, directory)
376-
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: fullpath) ?? false) else {
377+
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: ntpath) ?? false) else {
377378
throw error
378379
}
379380
return
@@ -382,29 +383,29 @@ enum _FileOperations {
382383

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

387388
if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY,
388389
entry.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != FILE_ATTRIBUTE_REPARSE_POINT {
389-
if filemanager?._shouldRemoveItemAtPath(fullpath) ?? true {
390-
stack.append((fullpath, true))
390+
if filemanager?._shouldRemoveItemAtPath(ntpath) ?? true {
391+
stack.append((ntpath, true))
391392
}
392393
} else {
393394
if entry.dwFileAttributes & FILE_ATTRIBUTE_READONLY == FILE_ATTRIBUTE_READONLY {
394395
guard SetFileAttributesW($0, entry.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY) else {
395-
throw CocoaError.removeFileError(GetLastError(), entry.fileName)
396+
throw CocoaError.removeFileError(GetLastError(), ntpath)
396397
}
397398
}
398399
if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY {
399-
guard filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return }
400+
guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
400401
if !RemoveDirectoryW($0) {
401402
let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
402403
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {
403404
throw error
404405
}
405406
}
406407
} else {
407-
guard filemanager?._shouldRemoveItemAtPath(fullpath) ?? true else { return }
408+
guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
408409
if !DeleteFileW($0) {
409410
let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
410411
guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {

‎Sources/FoundationEssentials/String/String+Internals.swift‎

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ 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-
// 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
69+
let flags: ULONG = PATHCCH_ALLOW_LONG_PATHS
7170
let result = PathAllocCanonicalize(pwszFullPath.baseAddress, flags, &pwszCanonicalPath)
7271
if let pwszCanonicalPath {
7372
defer { LocalFree(pwszCanonicalPath) }
@@ -80,32 +79,6 @@ extension String {
8079
}
8180
}
8281
}
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-
}
10982
}
11083
#endif
11184

‎Sources/FoundationEssentials/String/String+Path.swift‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,15 @@ extension String {
741741
guard GetFinalPathNameByHandleW(hFile, $0.baseAddress, dwLength, VOLUME_NAME_DOS) == dwLength - 1 else {
742742
return nil
743743
}
744-
return String(decodingCString: UnsafePointer($0.baseAddress!), as: UTF16.self).removingNTPathPrefix()
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)
745753
}
746754
}
747755
#else // os(Windows)

‎Sources/FoundationEssentials/WinSDK+Extensions.swift‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,6 @@ package var PATHCCH_ALLOW_LONG_PATHS: ULONG {
242242
ULONG(WinSDK.PATHCCH_ALLOW_LONG_PATHS.rawValue)
243243
}
244244

245-
package var PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH: ULONG {
246-
ULONG(WinSDK.PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH.rawValue)
247-
}
248-
249245
package var RRF_RT_REG_SZ: DWORD {
250246
DWORD(WinSDK.RRF_RT_REG_SZ)
251247
}

‎Tests/FoundationEssentialsTests/FileManager/FileManagerTests.swift‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,12 +1133,6 @@ 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-
11421136
#expect(fileManager.createFile(atPath: dirName + "/" + fileName, contents: nil))
11431137

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

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

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,12 +1195,8 @@ private struct FileManagerTests {
11951195

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

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-
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) }
12041200
#expect(throws: Never.self) { try Data().write(to: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile")) }
12051201
#expect(throws: Never.self) { try Data().write(to: URL(fileURLWithPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile2")) }
12061202
#expect(throws: Never.self) { try fileManager.moveItem(atPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile2", toPath: dirName + "/" + "subdir2" + "/" + "subdir3" + "/" + "somefile3") }

‎Tests/FoundationEssentialsTests/String/StringNTPathTests.swift‎

Lines changed: 0 additions & 75 deletions
This file was deleted.

0 commit comments

Comments
 (0)