Skip to content

Commit 41e3ff4

Browse files
authored
Fix some cases where AbsolutePath::relative(to: AbsolutePath) would assert (swiftlang#534)
- there where a couple case where relative(to:) would assert on windows when a path was long (>206), when tailing slashes where not removed in some cases and when paths where on different drives. - strip long path prefix in appending methods.
1 parent fe94371 commit 41e3ff4

5 files changed

Lines changed: 188 additions & 17 deletions

File tree

Sources/TSCBasic/FileSystem.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ private struct LocalFileSystem: FileSystem {
537537
let fsr: UnsafePointer<Int8> = cwdStr.fileSystemRepresentation
538538
defer { fsr.deallocate() }
539539

540-
return try? AbsolutePath(String(cString: fsr))
540+
return try? AbsolutePath(validating: String(cString: fsr))
541541
#endif
542542
}
543543

Sources/TSCBasic/Path.swift

Lines changed: 73 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public struct AbsolutePath: Hashable, Sendable {
8888
}
8989
defer { LocalFree(pwszResult) }
9090

91-
self.init(String(decodingCString: pwszResult, as: UTF16.self))
91+
try self.init(validating: String(decodingCString: pwszResult, as: UTF16.self))
9292
#else
9393
try self.init(basePath, RelativePath(validating: str))
9494
#endif
@@ -236,7 +236,7 @@ public struct RelativePath: Hashable, Sendable {
236236
fileprivate let _impl: PathImpl
237237

238238
/// Private initializer when the backing storage is known.
239-
private init(_ impl: PathImpl) {
239+
fileprivate init(_ impl: PathImpl) {
240240
_impl = impl
241241
}
242242

@@ -515,13 +515,28 @@ private struct WindowsPath: Path, Sendable {
515515
}
516516

517517
init(string: String) {
518+
var normalizedString = string
519+
520+
// Uppercase drive letter if lowercase
518521
if string.first?.isASCII ?? false, string.first?.isLetter ?? false, string.first?.isLowercase ?? false,
519-
string.count > 1, string[string.index(string.startIndex, offsetBy: 1)] == ":"
520-
{
521-
self.string = "\(string.first!.uppercased())\(string.dropFirst(1))"
522-
} else {
523-
self.string = string
522+
string.count > 1, string[string.index(string.startIndex, offsetBy: 1)] == ":" {
523+
normalizedString = "\(string.first!.uppercased())\(string.dropFirst(1))"
524+
}
525+
526+
// Remove trailing backslashes, but preserve them for root directories like "C:\"
527+
var result = normalizedString
528+
while result.hasSuffix("\\") {
529+
// Check if this is a root directory (e.g., "C:\" or just "\")
530+
// A root directory is either just "\" or "X:\" where X is a drive letter
531+
let isRootDir = result.count == 1 || // Just "\"
532+
(result.count == 3 && result.dropFirst().first == ":") // "X:\"
533+
if isRootDir {
534+
break // Preserve trailing slash for root directories
535+
}
536+
result = String(result.dropLast())
524537
}
538+
539+
self.string = result
525540
}
526541

527542
private static func repr(_ path: String) -> String {
@@ -544,7 +559,7 @@ private struct WindowsPath: Path, Sendable {
544559
self.init(string: ".")
545560
} else {
546561
let realpath: String = Self.repr(path)
547-
// Treat a relative path as an invalid relative path...
562+
// Treat an absolute path as an invalid relative path
548563
if Self.isAbsolutePath(realpath) || realpath.first == "\\" {
549564
throw PathValidationError.invalidRelativePath(path)
550565
}
@@ -568,6 +583,7 @@ private struct WindowsPath: Path, Sendable {
568583
_ = string.withCString(encodedAs: UTF16.self) { root in
569584
name.withCString(encodedAs: UTF16.self) { path in
570585
PathAllocCombine(root, path, ULONG(PATHCCH_ALLOW_LONG_PATHS.rawValue), &result)
586+
_ = PathCchStripPrefix(result, wcslen(result))
571587
}
572588
}
573589
defer { LocalFree(result) }
@@ -579,6 +595,7 @@ private struct WindowsPath: Path, Sendable {
579595
_ = string.withCString(encodedAs: UTF16.self) { root in
580596
relativePath.string.withCString(encodedAs: UTF16.self) { path in
581597
PathAllocCombine(root, path, ULONG(PATHCCH_ALLOW_LONG_PATHS.rawValue), &result)
598+
_ = PathCchStripPrefix(result, wcslen(result))
582599
}
583600
}
584601
defer { LocalFree(result) }
@@ -924,6 +941,18 @@ extension AbsolutePath {
924941
let pathComps = self.components
925942
let baseComps = base.components
926943

944+
#if os(Windows)
945+
// On Windows, check if paths are on different drives.
946+
// If they are, there's no valid relative path between them.
947+
// In this case, we return all components of the target path (including drive)
948+
// and skip the reconstruction assertion.
949+
let differentDrives: Bool = {
950+
guard !pathComps.isEmpty && !baseComps.isEmpty else { return false }
951+
// Drive letters are the first component (e.g., "C:")
952+
return pathComps[0].uppercased() != baseComps[0].uppercased()
953+
}()
954+
#endif
955+
927956
// It's common for the base to be an ancestor, so try that first.
928957
if pathComps.starts(with: baseComps) {
929958
// Special case, which is a plain path without `..` components. It
@@ -950,23 +979,54 @@ extension AbsolutePath {
950979
newPathComps = newPathComps.dropFirst()
951980
newBaseComps = newBaseComps.dropFirst()
952981
}
982+
#if os(Windows)
983+
// On Windows, if we have different drives, we cannot create a valid
984+
// relative path. Return all path components joined as a "relative" path.
985+
// This won't reconstruct correctly, but it's the best we can do.
986+
if differentDrives {
987+
// For cross-drive paths, we need to return a drive-relative path.
988+
// Strip the drive letter and preserve the leading backslash to maintain
989+
// drive-relative semantics: \directory\file.txt (not directory\file.txt).
990+
// We use the private init to bypass validation since paths starting
991+
// with \ are normally rejected as absolute paths.
992+
let compsWithoutDrive = Array(pathComps.dropFirst())
993+
let pathWithoutDrive = ([""] + compsWithoutDrive).joined(separator: "\\")
994+
result = RelativePath(PathImpl(string: pathWithoutDrive))
995+
} else {
996+
// Now construct a path consisting of as many `..`s as are in the
997+
// `newBaseComps` followed by what remains in `newPathComps`.
998+
var relComps = Array(repeating: "..", count: newBaseComps.count)
999+
relComps.append(contentsOf: newPathComps)
1000+
let pathString = relComps.joined(separator: "\\")
1001+
do {
1002+
result = try RelativePath(validating: pathString)
1003+
} catch {
1004+
preconditionFailure("invalid relative path computed from \(pathString)")
1005+
}
1006+
}
1007+
#else
9531008
// Now construct a path consisting of as many `..`s as are in the
9541009
// `newBaseComps` followed by what remains in `newPathComps`.
9551010
var relComps = Array(repeating: "..", count: newBaseComps.count)
9561011
relComps.append(contentsOf: newPathComps)
957-
#if os(Windows)
958-
let pathString = relComps.joined(separator: "\\")
959-
#else
9601012
let pathString = relComps.joined(separator: "/")
961-
#endif
9621013
do {
9631014
result = try RelativePath(validating: pathString)
9641015
} catch {
9651016
preconditionFailure("invalid relative path computed from \(pathString)")
9661017
}
1018+
#endif
9671019
}
9681020

969-
assert(AbsolutePath(base, result) == self)
1021+
#if os(Windows)
1022+
// Skip the assertion check for cross-drive paths on Windows,
1023+
// as there's no valid relative path that can reconstruct across drives.
1024+
if !differentDrives {
1025+
assert(AbsolutePath(base, result) == self, "\(AbsolutePath(base, result)) != \(self)")
1026+
}
1027+
#else
1028+
assert(AbsolutePath(base, result) == self, "\(AbsolutePath(base, result)) != \(self)")
1029+
#endif
9701030
return result
9711031
}
9721032

Sources/TSCBasic/PathShims.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public func resolveSymlinks(_ path: AbsolutePath) throws -> AbsolutePath {
4444
} else {
4545
pathBaseAddress = UnsafePointer($0.baseAddress!)
4646
}
47-
return try AbsolutePath(String(decodingCString: pathBaseAddress, as: UTF16.self))
47+
return try AbsolutePath(validating: String(decodingCString: pathBaseAddress, as: UTF16.self))
4848
}
4949
#else
5050
let pathStr = path.pathString

Sources/TSCUtility/FSWatch.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,9 @@ public final class RDCWatcher {
250250
}
251251

252252
if !GetOverlappedResult(watch.hDirectory, &watch.overlapped, &dwBytesReturned, false) {
253+
guard let path = try? AbsolutePath(validating: watch.path) else { continue }
253254
queue.async {
254-
delegate?.pathsDidReceiveEvent([AbsolutePath(watch.path)])
255+
delegate?.pathsDidReceiveEvent([path])
255256
}
256257
return
257258
}
@@ -272,7 +273,8 @@ public final class RDCWatcher {
272273
String(utf16CodeUnitsNoCopy: &pNotify.pointee.FileName,
273274
count: Int(pNotify.pointee.FileNameLength) / MemoryLayout<WCHAR>.stride,
274275
freeWhenDone: false)
275-
paths.append(AbsolutePath(file))
276+
guard let path = try? AbsolutePath(validating: file) else { continue }
277+
paths.append(path)
276278

277279
pNotify = (UnsafeMutableRawPointer(pNotify) + Int(pNotify.pointee.NextEntryOffset))
278280
.assumingMemoryBound(to: FILE_NOTIFY_INFORMATION.self)
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
This source file is part of the Swift.org open source project
3+
4+
Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
5+
Licensed under Apache License v2.0 with Runtime Library Exception
6+
7+
See http://swift.org/LICENSE.txt for license information
8+
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
*/
10+
11+
import Foundation
12+
import TSCBasic
13+
import TSCTestSupport
14+
import XCTest
15+
16+
class PathWindowsRelativeTests: XCTestCase {
17+
18+
#if os(Windows)
19+
func testRelativePathAcrossDifferentDrives() {
20+
// On Windows, you cannot express a path from one drive to another
21+
// using relative path components (.. and .). This test verifies that
22+
// the relative(to:) method handles this case without assertion failure.
23+
24+
let pathOnCDrive = AbsolutePath(#"C:\Users\test"#)
25+
let baseOnDDrive = AbsolutePath(#"D:\"#)
26+
27+
// This should not trigger an assertion failure.
28+
// The method will return a relative path that cannot properly reconstruct
29+
// the original path (since cross-drive relative paths are impossible),
30+
// but it should handle the case gracefully.
31+
let relative = pathOnCDrive.relative(to: baseOnDDrive)
32+
33+
// The relative path should be non-empty
34+
XCTAssertFalse(relative.pathString.isEmpty)
35+
36+
// Note: AbsolutePath(baseOnDDrive, relative) will NOT equal pathOnCDrive
37+
// because there's no valid relative path between different drives on Windows.
38+
// This is expected behavior for cross-drive paths.
39+
}
40+
41+
func testRelativePathOnSameDrive() {
42+
// Verify that relative paths work correctly when on the same drive
43+
let path = AbsolutePath(#"C:\Users\test\Documents"#)
44+
let base = AbsolutePath(#"C:\Users"#)
45+
46+
let relative = path.relative(to: base)
47+
48+
// Should be able to reconstruct the original path
49+
XCTAssertEqual(AbsolutePath(base, relative), path)
50+
XCTAssertEqual(relative.pathString, #"test\Documents"#)
51+
}
52+
53+
func testRelativePathWithParentTraversal() {
54+
// Test going up and down on the same drive
55+
let path = AbsolutePath(#"C:\Projects\MyApp"#)
56+
let base = AbsolutePath(#"C:\Users\test"#)
57+
58+
let relative = path.relative(to: base)
59+
60+
// Should be able to reconstruct the original path
61+
XCTAssertEqual(AbsolutePath(base, relative), path)
62+
// From C:\Users\test to C:\Projects\MyApp:
63+
// - Go up 2 levels (test -> Users -> C:)
64+
// - Then down to Projects\MyApp
65+
XCTAssertEqual(relative.pathString, #"..\..\Projects\MyApp"#)
66+
}
67+
68+
func testCrossDriveVariants() {
69+
// Test various cross-drive scenarios
70+
let scenarios = [
71+
(AbsolutePath(#"C:\Users\test"#), AbsolutePath(#"D:\"#)),
72+
(AbsolutePath(#"D:\Data\files"#), AbsolutePath(#"C:\Windows"#)),
73+
(AbsolutePath(#"E:\Backup"#), AbsolutePath(#"C:\Users"#)),
74+
]
75+
76+
for (path, base) in scenarios {
77+
// Should not crash or trigger assertion
78+
let _ = path.relative(to: base)
79+
}
80+
}
81+
82+
func testCrossDrivePreservesLeadingBackslash() {
83+
// When computing a relative path across different drives,
84+
// the leading backslash must be preserved to maintain drive-relative semantics.
85+
// C:\directory\file.txt -> \directory\file.txt (not directory\file.txt)
86+
// This distinction is important:
87+
// - \directory\file.txt is a drive-relative absolute path
88+
// - directory\file.txt is relative to the current working directory on that drive
89+
90+
let pathOnCDrive = AbsolutePath(#"C:\Users\test\Documents\file.txt"#)
91+
let baseOnDDrive = AbsolutePath(#"D:\Projects"#)
92+
93+
let relative = pathOnCDrive.relative(to: baseOnDDrive)
94+
95+
// The relative path should start with a backslash to indicate drive-relative
96+
XCTAssertTrue(relative.pathString.hasPrefix("\\"),
97+
"Cross-drive relative path should start with \\ to preserve drive-relative semantics, got: \(relative.pathString)")
98+
99+
// Should contain the path components without the drive letter
100+
XCTAssertTrue(relative.pathString.contains("Users"),
101+
"Path should contain directory components")
102+
XCTAssertTrue(relative.pathString.contains("test"),
103+
"Path should contain directory components")
104+
105+
// More specifically, it should be something like \Users\test\Documents\file.txt
106+
XCTAssertEqual(relative.pathString, #"\Users\test\Documents\file.txt"#)
107+
}
108+
#endif
109+
}

0 commit comments

Comments
 (0)