diff --git a/Sources/TSCBasic/FileSystem.swift b/Sources/TSCBasic/FileSystem.swift index 8f3e4f7e..edf31257 100644 --- a/Sources/TSCBasic/FileSystem.swift +++ b/Sources/TSCBasic/FileSystem.swift @@ -537,7 +537,7 @@ private struct LocalFileSystem: FileSystem { let fsr: UnsafePointer = cwdStr.fileSystemRepresentation defer { fsr.deallocate() } - return try? AbsolutePath(String(cString: fsr)) + return try? AbsolutePath(validating: String(cString: fsr)) #endif } diff --git a/Sources/TSCBasic/Path.swift b/Sources/TSCBasic/Path.swift index 208f9819..02845785 100644 --- a/Sources/TSCBasic/Path.swift +++ b/Sources/TSCBasic/Path.swift @@ -88,7 +88,7 @@ public struct AbsolutePath: Hashable, Sendable { } defer { LocalFree(pwszResult) } - self.init(String(decodingCString: pwszResult, as: UTF16.self)) + try self.init(validating: String(decodingCString: pwszResult, as: UTF16.self)) #else try self.init(basePath, RelativePath(validating: str)) #endif @@ -236,7 +236,7 @@ public struct RelativePath: Hashable, Sendable { fileprivate let _impl: PathImpl /// Private initializer when the backing storage is known. - private init(_ impl: PathImpl) { + fileprivate init(_ impl: PathImpl) { _impl = impl } @@ -515,13 +515,28 @@ private struct WindowsPath: Path, Sendable { } init(string: String) { + var normalizedString = string + + // Uppercase drive letter if lowercase if string.first?.isASCII ?? false, string.first?.isLetter ?? false, string.first?.isLowercase ?? false, - string.count > 1, string[string.index(string.startIndex, offsetBy: 1)] == ":" - { - self.string = "\(string.first!.uppercased())\(string.dropFirst(1))" - } else { - self.string = string + string.count > 1, string[string.index(string.startIndex, offsetBy: 1)] == ":" { + normalizedString = "\(string.first!.uppercased())\(string.dropFirst(1))" + } + + // Remove trailing backslashes, but preserve them for root directories like "C:\" + var result = normalizedString + while result.hasSuffix("\\") { + // Check if this is a root directory (e.g., "C:\" or just "\") + // A root directory is either just "\" or "X:\" where X is a drive letter + let isRootDir = result.count == 1 || // Just "\" + (result.count == 3 && result.dropFirst().first == ":") // "X:\" + if isRootDir { + break // Preserve trailing slash for root directories + } + result = String(result.dropLast()) } + + self.string = result } private static func repr(_ path: String) -> String { @@ -544,7 +559,7 @@ private struct WindowsPath: Path, Sendable { self.init(string: ".") } else { let realpath: String = Self.repr(path) - // Treat a relative path as an invalid relative path... + // Treat an absolute path as an invalid relative path if Self.isAbsolutePath(realpath) || realpath.first == "\\" { throw PathValidationError.invalidRelativePath(path) } @@ -568,6 +583,7 @@ private struct WindowsPath: Path, Sendable { _ = string.withCString(encodedAs: UTF16.self) { root in name.withCString(encodedAs: UTF16.self) { path in PathAllocCombine(root, path, ULONG(PATHCCH_ALLOW_LONG_PATHS.rawValue), &result) + _ = PathCchStripPrefix(result, wcslen(result)) } } defer { LocalFree(result) } @@ -579,6 +595,7 @@ private struct WindowsPath: Path, Sendable { _ = string.withCString(encodedAs: UTF16.self) { root in relativePath.string.withCString(encodedAs: UTF16.self) { path in PathAllocCombine(root, path, ULONG(PATHCCH_ALLOW_LONG_PATHS.rawValue), &result) + _ = PathCchStripPrefix(result, wcslen(result)) } } defer { LocalFree(result) } @@ -924,6 +941,18 @@ extension AbsolutePath { let pathComps = self.components let baseComps = base.components +#if os(Windows) + // On Windows, check if paths are on different drives. + // If they are, there's no valid relative path between them. + // In this case, we return all components of the target path (including drive) + // and skip the reconstruction assertion. + let differentDrives: Bool = { + guard !pathComps.isEmpty && !baseComps.isEmpty else { return false } + // Drive letters are the first component (e.g., "C:") + return pathComps[0].uppercased() != baseComps[0].uppercased() + }() +#endif + // It's common for the base to be an ancestor, so try that first. if pathComps.starts(with: baseComps) { // Special case, which is a plain path without `..` components. It @@ -950,23 +979,54 @@ extension AbsolutePath { newPathComps = newPathComps.dropFirst() newBaseComps = newBaseComps.dropFirst() } +#if os(Windows) + // On Windows, if we have different drives, we cannot create a valid + // relative path. Return all path components joined as a "relative" path. + // This won't reconstruct correctly, but it's the best we can do. + if differentDrives { + // For cross-drive paths, we need to return a drive-relative path. + // Strip the drive letter and preserve the leading backslash to maintain + // drive-relative semantics: \directory\file.txt (not directory\file.txt). + // We use the private init to bypass validation since paths starting + // with \ are normally rejected as absolute paths. + let compsWithoutDrive = Array(pathComps.dropFirst()) + let pathWithoutDrive = ([""] + compsWithoutDrive).joined(separator: "\\") + result = RelativePath(PathImpl(string: pathWithoutDrive)) + } else { + // Now construct a path consisting of as many `..`s as are in the + // `newBaseComps` followed by what remains in `newPathComps`. + var relComps = Array(repeating: "..", count: newBaseComps.count) + relComps.append(contentsOf: newPathComps) + let pathString = relComps.joined(separator: "\\") + do { + result = try RelativePath(validating: pathString) + } catch { + preconditionFailure("invalid relative path computed from \(pathString)") + } + } +#else // Now construct a path consisting of as many `..`s as are in the // `newBaseComps` followed by what remains in `newPathComps`. var relComps = Array(repeating: "..", count: newBaseComps.count) relComps.append(contentsOf: newPathComps) -#if os(Windows) - let pathString = relComps.joined(separator: "\\") -#else let pathString = relComps.joined(separator: "/") -#endif do { result = try RelativePath(validating: pathString) } catch { preconditionFailure("invalid relative path computed from \(pathString)") } +#endif } - assert(AbsolutePath(base, result) == self) +#if os(Windows) + // Skip the assertion check for cross-drive paths on Windows, + // as there's no valid relative path that can reconstruct across drives. + if !differentDrives { + assert(AbsolutePath(base, result) == self, "\(AbsolutePath(base, result)) != \(self)") + } +#else + assert(AbsolutePath(base, result) == self, "\(AbsolutePath(base, result)) != \(self)") +#endif return result } diff --git a/Sources/TSCBasic/PathShims.swift b/Sources/TSCBasic/PathShims.swift index 8851751a..d08026af 100644 --- a/Sources/TSCBasic/PathShims.swift +++ b/Sources/TSCBasic/PathShims.swift @@ -44,7 +44,7 @@ public func resolveSymlinks(_ path: AbsolutePath) throws -> AbsolutePath { } else { pathBaseAddress = UnsafePointer($0.baseAddress!) } - return try AbsolutePath(String(decodingCString: pathBaseAddress, as: UTF16.self)) + return try AbsolutePath(validating: String(decodingCString: pathBaseAddress, as: UTF16.self)) } #else let pathStr = path.pathString diff --git a/Sources/TSCUtility/FSWatch.swift b/Sources/TSCUtility/FSWatch.swift index 1987e4e0..e410f717 100644 --- a/Sources/TSCUtility/FSWatch.swift +++ b/Sources/TSCUtility/FSWatch.swift @@ -250,8 +250,9 @@ public final class RDCWatcher { } if !GetOverlappedResult(watch.hDirectory, &watch.overlapped, &dwBytesReturned, false) { + guard let path = try? AbsolutePath(validating: watch.path) else { continue } queue.async { - delegate?.pathsDidReceiveEvent([AbsolutePath(watch.path)]) + delegate?.pathsDidReceiveEvent([path]) } return } @@ -272,7 +273,8 @@ public final class RDCWatcher { String(utf16CodeUnitsNoCopy: &pNotify.pointee.FileName, count: Int(pNotify.pointee.FileNameLength) / MemoryLayout.stride, freeWhenDone: false) - paths.append(AbsolutePath(file)) + guard let path = try? AbsolutePath(validating: file) else { continue } + paths.append(path) pNotify = (UnsafeMutableRawPointer(pNotify) + Int(pNotify.pointee.NextEntryOffset)) .assumingMemoryBound(to: FILE_NOTIFY_INFORMATION.self) diff --git a/Tests/TSCBasicTests/PathWindowsRelativeTests.swift b/Tests/TSCBasicTests/PathWindowsRelativeTests.swift new file mode 100644 index 00000000..e53504b9 --- /dev/null +++ b/Tests/TSCBasicTests/PathWindowsRelativeTests.swift @@ -0,0 +1,109 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See http://swift.org/LICENSE.txt for license information + See http://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +import Foundation +import TSCBasic +import TSCTestSupport +import XCTest + +class PathWindowsRelativeTests: XCTestCase { + + #if os(Windows) + func testRelativePathAcrossDifferentDrives() { + // On Windows, you cannot express a path from one drive to another + // using relative path components (.. and .). This test verifies that + // the relative(to:) method handles this case without assertion failure. + + let pathOnCDrive = AbsolutePath(#"C:\Users\test"#) + let baseOnDDrive = AbsolutePath(#"D:\"#) + + // This should not trigger an assertion failure. + // The method will return a relative path that cannot properly reconstruct + // the original path (since cross-drive relative paths are impossible), + // but it should handle the case gracefully. + let relative = pathOnCDrive.relative(to: baseOnDDrive) + + // The relative path should be non-empty + XCTAssertFalse(relative.pathString.isEmpty) + + // Note: AbsolutePath(baseOnDDrive, relative) will NOT equal pathOnCDrive + // because there's no valid relative path between different drives on Windows. + // This is expected behavior for cross-drive paths. + } + + func testRelativePathOnSameDrive() { + // Verify that relative paths work correctly when on the same drive + let path = AbsolutePath(#"C:\Users\test\Documents"#) + let base = AbsolutePath(#"C:\Users"#) + + let relative = path.relative(to: base) + + // Should be able to reconstruct the original path + XCTAssertEqual(AbsolutePath(base, relative), path) + XCTAssertEqual(relative.pathString, #"test\Documents"#) + } + + func testRelativePathWithParentTraversal() { + // Test going up and down on the same drive + let path = AbsolutePath(#"C:\Projects\MyApp"#) + let base = AbsolutePath(#"C:\Users\test"#) + + let relative = path.relative(to: base) + + // Should be able to reconstruct the original path + XCTAssertEqual(AbsolutePath(base, relative), path) + // From C:\Users\test to C:\Projects\MyApp: + // - Go up 2 levels (test -> Users -> C:) + // - Then down to Projects\MyApp + XCTAssertEqual(relative.pathString, #"..\..\Projects\MyApp"#) + } + + func testCrossDriveVariants() { + // Test various cross-drive scenarios + let scenarios = [ + (AbsolutePath(#"C:\Users\test"#), AbsolutePath(#"D:\"#)), + (AbsolutePath(#"D:\Data\files"#), AbsolutePath(#"C:\Windows"#)), + (AbsolutePath(#"E:\Backup"#), AbsolutePath(#"C:\Users"#)), + ] + + for (path, base) in scenarios { + // Should not crash or trigger assertion + let _ = path.relative(to: base) + } + } + + func testCrossDrivePreservesLeadingBackslash() { + // When computing a relative path across different drives, + // the leading backslash must be preserved to maintain drive-relative semantics. + // C:\directory\file.txt -> \directory\file.txt (not directory\file.txt) + // This distinction is important: + // - \directory\file.txt is a drive-relative absolute path + // - directory\file.txt is relative to the current working directory on that drive + + let pathOnCDrive = AbsolutePath(#"C:\Users\test\Documents\file.txt"#) + let baseOnDDrive = AbsolutePath(#"D:\Projects"#) + + let relative = pathOnCDrive.relative(to: baseOnDDrive) + + // The relative path should start with a backslash to indicate drive-relative + XCTAssertTrue(relative.pathString.hasPrefix("\\"), + "Cross-drive relative path should start with \\ to preserve drive-relative semantics, got: \(relative.pathString)") + + // Should contain the path components without the drive letter + XCTAssertTrue(relative.pathString.contains("Users"), + "Path should contain directory components") + XCTAssertTrue(relative.pathString.contains("test"), + "Path should contain directory components") + + // More specifically, it should be something like \Users\test\Documents\file.txt + XCTAssertEqual(relative.pathString, #"\Users\test\Documents\file.txt"#) + } + #endif +}