Fix a corner case where createDirectory would fail on windows #1479
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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? | ||
| // 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 | ||
|
There was a problem hiding this comment. Do we not also need the PATHCCH_ALLOW_LONG_PATHS flag anymore? @daveinglis Also, the same code is copied to:
There are also variations in swift-tools-support-core and swift-testing. Can you open PRs to make the change there as well? There was a problem hiding this comment. The PATHCCH_ALLOW_LONG_PATHS is not needed, PATHCCH_ENSURE_IS_EXTENDED_LENGTH_PATH will always prepend the There was a problem hiding this comment. So I saw the swift-build version, but it's never used in the context of creating a directory so should work fine as is. I can look at the other packages but if the usage is never used with creating directories do we still need it changed? There was a problem hiding this comment.
You never know when it might be used in new contexts, and because of cargo-culting I think we want to ensure that the most-correct implementation is kept in sync across all places it's copied to. Ideally we'd have one function that everything could call into, but not a great place for this sort of SPI... There was a problem hiding this comment. ok, I will start putting up some PR for the other version so that all match. |
||
| 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 | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.deletingLastPathComponent()will replace all\with/in the path, but I think it's OK since the next call to.withNTPathRepresentationwill callGetFullPathNameWand normalize them back to\. I was curious if this might cause issues if someone passes a path that already has a\\?\prefix to.createDirectory(atPath:), which would then have parent paths of the form//?/..., but I ran some simple tests and it still seems to work OK. We could add a.replacing(._slash, with: ._backslash)to the end to be safe.