From 01094d136b7ec7c1c9bfdc8d2e56b70be5ed262a Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 25 Feb 2025 15:26:43 -0500 Subject: [PATCH] Clean up symlink following and recursively follow symlink chains. --- .../SwiftFormat/Utilities/FileIterator.swift | 72 ++++++++++++------- .../Utilities/FileIteratorTests.swift | 40 +++++++++++ 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/Sources/SwiftFormat/Utilities/FileIterator.swift b/Sources/SwiftFormat/Utilities/FileIterator.swift index b0a8d2f06..f6d05776c 100644 --- a/Sources/SwiftFormat/Utilities/FileIterator.swift +++ b/Sources/SwiftFormat/Utilities/FileIterator.swift @@ -71,25 +71,18 @@ public struct FileIterator: Sequence, IteratorProtocol { if dirIterator != nil { output = nextInDirectory() } else { - guard var next = urlIterator.next() else { + guard let next = urlIterator.next() else { // If we've reached the end of all the URLs we wanted to iterate over, exit now. return nil } - - guard let fileType = fileType(at: next) else { + guard let (next, fileType) = fileAndType(at: next, followSymlinks: followSymlinks) else { continue } switch fileType { case .typeSymbolicLink: - guard - followSymlinks, - let destination = try? FileManager.default.destinationOfSymbolicLink(atPath: next.path) - else { - break - } - next = URL(fileURLWithPath: destination, relativeTo: next) - fallthrough + // If we got here, we encountered a symlink but didn't follow it. Skip it. + continue case .typeDirectory: dirIterator = FileManager.default.enumerator( @@ -132,27 +125,20 @@ public struct FileIterator: Sequence, IteratorProtocol { } #endif - guard item.lastPathComponent.hasSuffix(fileSuffix), let fileType = fileType(at: item) else { + guard item.lastPathComponent.hasSuffix(fileSuffix), + let (item, fileType) = fileAndType(at: item, followSymlinks: followSymlinks) + else { continue } - var path = item.path switch fileType { - case .typeSymbolicLink where followSymlinks: - guard - let destination = try? FileManager.default.destinationOfSymbolicLink(atPath: path) - else { - break - } - path = URL(fileURLWithPath: destination, relativeTo: item).path - fallthrough - case .typeRegular: // We attempt to relativize the URLs based on the current working directory, not the // directory being iterated over, so that they can be displayed better in diagnostics. Thus, // if the user passes paths that are relative to the current working directory, they will // be displayed as relative paths. Otherwise, they will still be displayed as absolute // paths. + let path = item.path let relativePath: String if !workingDirectory.isRoot, path.hasPrefix(workingDirectory.path) { relativePath = String(path.dropFirst(workingDirectory.path.count).drop(while: { $0 == "/" || $0 == #"\"# })) @@ -173,9 +159,41 @@ public struct FileIterator: Sequence, IteratorProtocol { } } -/// Returns the type of the file at the given URL. -private func fileType(at url: URL) -> FileAttributeType? { - // We cannot use `URL.resourceValues(forKeys:)` here because it appears to behave incorrectly on - // Linux. - return try? FileManager.default.attributesOfItem(atPath: url.path)[.type] as? FileAttributeType +/// Returns the actual URL and type of the file at the given URL, following symlinks if requested. +/// +/// - Parameters: +/// - url: The URL to get the file and type of. +/// - followSymlinks: Whether to follow symlinks. +/// - Returns: The actual URL and type of the file at the given URL, or `nil` if the file does not +/// exist or is not a supported file type. If `followSymlinks` is `true`, the returned URL may be +/// different from the given URL; otherwise, it will be the same. +private func fileAndType(at url: URL, followSymlinks: Bool) -> (URL, FileAttributeType)? { + func typeOfFile(at url: URL) -> FileAttributeType? { + // We cannot use `URL.resourceValues(forKeys:)` here because it appears to behave incorrectly on + // Linux. + return try? FileManager.default.attributesOfItem(atPath: url.path)[.type] as? FileAttributeType + } + + guard var fileType = typeOfFile(at: url) else { + return nil + } + + // We would use `standardizedFileURL.path` here as we do in the iterator above to ensure that + // path components like `.` and `..` are resolved, but the standardized URLs returned by + // Foundation pre-Swift-6.0 resolve symlinks. This causes the file type of a URL and its + // standardized path to not match. + var visited: Set = [url.absoluteString] + var url = url + while followSymlinks && fileType == .typeSymbolicLink, + let destination = try? FileManager.default.destinationOfSymbolicLink(atPath: url.path) + { + url = URL(fileURLWithPath: destination, relativeTo: url) + // If this URL is in the visited set, we must have a symlink cycle. Ignore it gracefully. + guard !visited.contains(url.absoluteString), let newType = typeOfFile(at: url) else { + return nil + } + visited.insert(url.absoluteString) + fileType = newType + } + return (url, fileType) } diff --git a/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift b/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift index 6c15e6f41..ad4be0c76 100644 --- a/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift +++ b/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift @@ -44,6 +44,16 @@ final class FileIteratorTests: XCTestCase { try touch("project/.build/generated.swift") try symlink("project/link.swift", to: "project/.hidden.swift") try symlink("project/rellink.swift", relativeTo: ".hidden.swift") + + #if !(os(Windows) && compiler(<5.10)) + // Test both a self-cycle and a cycle between multiple symlinks. + try symlink("project/cycliclink.swift", relativeTo: "cycliclink.swift") + try symlink("project/linktolink.swift", relativeTo: "link.swift") + + // Test symlinks that use nonstandardized paths. + try symlink("project/2stepcyclebegin.swift", relativeTo: "../project/2stepcycleend.swift") + try symlink("project/2stepcycleend.swift", relativeTo: "./2stepcyclebegin.swift") + #endif } override func tearDownWithError() throws { @@ -72,6 +82,36 @@ final class FileIteratorTests: XCTestCase { XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") }) } + func testFollowSymlinksToSymlinks() throws { + #if os(Windows) && compiler(<5.10) + try XCTSkipIf(true, "Foundation does not follow symlinks on Windows") + #endif + let seen = allFilesSeen( + iteratingOver: [tmpURL("project/linktolink.swift")], + followSymlinks: true + ) + // Hidden but found through the visible symlink chain. + XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") }) + } + + func testSymlinkCyclesAreIgnored() throws { + #if os(Windows) && compiler(<5.10) + try XCTSkipIf(true, "Foundation does not follow symlinks on Windows") + #endif + let seen = allFilesSeen( + iteratingOver: [ + tmpURL("project/cycliclink.swift"), + tmpURL("project/2stepcyclebegin.swift"), + tmpURL("project/link.swift"), + ], + followSymlinks: true + ) + // Hidden but found through the visible symlink chain. + XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") }) + // And the cycles were ignored. + XCTAssertEqual(seen.count, 1) + } + func testTraversesHiddenFilesIfExplicitlySpecified() throws { #if os(Windows) && compiler(<5.10) try XCTSkipIf(true, "Foundation does not follow symlinks on Windows")