Skip to content

Commit ab2072d

Browse files
authored
Keep order of paths fixed (#211)
Thanks for the fix!
1 parent f3cf34d commit ab2072d

File tree

3 files changed

+129
-31
lines changed

3 files changed

+129
-31
lines changed

Sources/Subprocess/Platforms/Subprocess+Unix.swift

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -271,15 +271,13 @@ extension Arguments {
271271

272272
// MARK: - Executable Searching
273273
extension Executable {
274-
internal static var defaultSearchPaths: Set<String> {
275-
return Set([
276-
"/usr/bin",
277-
"/bin",
278-
"/usr/sbin",
279-
"/sbin",
280-
"/usr/local/bin",
281-
])
282-
}
274+
internal static let defaultSearchPaths = [
275+
"/usr/bin",
276+
"/bin",
277+
"/usr/sbin",
278+
"/sbin",
279+
"/usr/local/bin",
280+
]
283281

284282
internal func resolveExecutablePath(withPathValue pathValue: String?) throws -> String {
285283
switch self.storage {
@@ -288,21 +286,10 @@ extension Executable {
288286
if Configuration.pathAccessible(executableName, mode: X_OK) {
289287
return executableName
290288
}
291-
// Get $PATH from environment
292-
let searchPaths: Set<String>
293-
if let pathValue = pathValue {
294-
let localSearchPaths = pathValue.split(separator: ":").map { String($0) }
295-
searchPaths = Set(localSearchPaths).union(Self.defaultSearchPaths)
296-
} else {
297-
searchPaths = Self.defaultSearchPaths
298-
}
299-
300-
for path in searchPaths {
301-
let fullPath = "\(path)/\(executableName)"
302-
let fileExists = Configuration.pathAccessible(fullPath, mode: X_OK)
303-
if fileExists {
304-
return fullPath
305-
}
289+
let firstAccessibleExecutable = possibleExecutablePaths(withPathValue: pathValue)
290+
.first { Configuration.pathAccessible($0, mode: X_OK) }
291+
if let firstAccessibleExecutable {
292+
return firstAccessibleExecutable
306293
}
307294
throw SubprocessError(
308295
code: .init(.executableNotFound(executableName)),
@@ -323,13 +310,12 @@ extension Executable {
323310
// executableName could be a full path
324311
results.insert(executableName)
325312
// Get $PATH from environment
326-
let searchPaths: Set<String>
327-
if let pathValue = pathValue {
328-
let localSearchPaths = pathValue.split(separator: ":").map { String($0) }
329-
searchPaths = Set(localSearchPaths).union(Self.defaultSearchPaths)
330-
} else {
331-
searchPaths = Self.defaultSearchPaths
332-
}
313+
let searchPaths =
314+
if let pathValue = pathValue {
315+
pathValue.split(separator: ":").map { String($0) } + Self.defaultSearchPaths
316+
} else {
317+
Self.defaultSearchPaths
318+
}
333319
for path in searchPaths {
334320
results.insert(
335321
FilePath(path).appending(executableName).string

Tests/SubprocessTests/IntegrationTests.swift

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,66 @@ extension SubprocessIntegrationTests {
136136
_ = try await Subprocess.run(.path(fakePath), output: .discarded)
137137
}
138138
}
139+
140+
#if !os(Windows)
141+
/// Integration test verifying that subprocess execution respects PATH ordering.
142+
@Test func testPathOrderingIsRespected() async throws {
143+
// Create temporary directories to simulate a PATH with multiple executables.
144+
let tempDir = FileManager.default.temporaryDirectory
145+
let firstDir = tempDir.appendingPathComponent("path-test-first-\(UUID().uuidString)")
146+
let secondDir = tempDir.appendingPathComponent("path-test-second-\(UUID().uuidString)")
147+
148+
try FileManager.default.createDirectory(at: firstDir, withIntermediateDirectories: true)
149+
try FileManager.default.createDirectory(at: secondDir, withIntermediateDirectories: true)
150+
151+
defer {
152+
try? FileManager.default.removeItem(at: firstDir)
153+
try? FileManager.default.removeItem(at: secondDir)
154+
}
155+
156+
// Create two different "test-executable" scripts that output different values.
157+
let executableName = "test-executable-\(UUID().uuidString)"
158+
159+
// First
160+
let firstExecutable = firstDir.appendingPathComponent(executableName)
161+
try """
162+
#!/bin/sh
163+
echo "FIRST"
164+
""".write(to: firstExecutable, atomically: true, encoding: .utf8)
165+
try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: firstExecutable.path())
166+
167+
// Second
168+
let secondExecutable = secondDir.appendingPathComponent(executableName)
169+
try """
170+
#!/bin/sh
171+
echo "SECOND"
172+
""".write(to: secondExecutable, atomically: true, encoding: .utf8)
173+
try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: secondExecutable.path())
174+
175+
// Run repeatedly to increase chance of catching any ordering issues.
176+
for _ in 0..<10 {
177+
let first = try await Subprocess.run(
178+
.name(executableName),
179+
environment: .inherit.updating([
180+
"PATH": "\(firstDir.path()):\(secondDir.path())"
181+
]),
182+
output: .string(limit: 8)
183+
)
184+
#expect(first.terminationStatus.isSuccess)
185+
#expect(first.standardOutput?.trimmingNewLineAndQuotes() == "FIRST")
186+
187+
let second = try await Subprocess.run(
188+
.name(executableName),
189+
environment: .inherit.updating([
190+
"PATH": "\(secondDir.path()):\(firstDir.path())"
191+
]),
192+
output: .string(limit: 8)
193+
)
194+
#expect(second.terminationStatus.isSuccess)
195+
#expect(second.standardOutput?.trimmingNewLineAndQuotes() == "SECOND")
196+
}
197+
}
198+
#endif
139199
}
140200

141201
// MARK: - Argument Tests

Tests/SubprocessTests/UnixTests.swift

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,58 @@ extension SubprocessUnixTests {
201201
}
202202
}
203203

204+
// MARK: - PATH Resolution Tests
205+
extension SubprocessUnixTests {
206+
@Test func testExecutablePathsPreserveOrder() throws {
207+
let executable = Executable.name("test-bin")
208+
let pathValue = "/first/path:/second/path:/third/path"
209+
210+
let paths = executable.possibleExecutablePaths(withPathValue: pathValue)
211+
let pathsArray = Array(paths)
212+
213+
#expect(
214+
pathsArray == [
215+
"test-bin",
216+
"/first/path/test-bin",
217+
"/second/path/test-bin",
218+
"/third/path/test-bin",
219+
220+
// Default search paths
221+
"/usr/bin/test-bin",
222+
"/bin/test-bin",
223+
"/usr/sbin/test-bin",
224+
"/sbin/test-bin",
225+
"/usr/local/bin/test-bin",
226+
])
227+
}
228+
229+
@Test func testNoDuplicatedExecutablePaths() throws {
230+
let executable = Executable.name("test-bin")
231+
let duplicatePath = "/first/path:/first/path:/second/path"
232+
let duplicatePaths = executable.possibleExecutablePaths(withPathValue: duplicatePath)
233+
234+
#expect(Array(duplicatePaths).count == Set(duplicatePaths).count)
235+
}
236+
237+
@Test func testPossibleExecutablePathsWithNilPATH() throws {
238+
let executable = Executable.name("test-bin")
239+
let paths = executable.possibleExecutablePaths(withPathValue: nil)
240+
let pathsArray = Array(paths)
241+
242+
#expect(
243+
pathsArray == [
244+
"test-bin",
245+
246+
// Default search paths
247+
"/usr/bin/test-bin",
248+
"/bin/test-bin",
249+
"/usr/sbin/test-bin",
250+
"/sbin/test-bin",
251+
"/usr/local/bin/test-bin",
252+
])
253+
}
254+
}
255+
204256
// MARK: - Misc
205257
extension SubprocessUnixTests {
206258
@Test func testExitSignal() async throws {

0 commit comments

Comments
 (0)