Skip to content

Commit

Permalink
[Process] On Windows, don't rely on the OS to find executables
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Oct 30, 2024
1 parent 2ad775b commit 18f50a7
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
7 changes: 7 additions & 0 deletions Process.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class Process implements \IteratorAggregate
private ?int $cachedExitCode = null;

private static ?bool $sigchild = null;
private static array $executables = [];

/**
* Exit codes translation table.
Expand Down Expand Up @@ -1543,6 +1544,12 @@ private function buildShellCommandline(string|array $commandline): string
return $commandline;
}

if ('\\' === \DIRECTORY_SEPARATOR && isset($commandline[0][0]) && \strlen($commandline[0]) === strcspn($commandline[0], ':/\\')) {
// On Windows, we don't rely on the OS to find the executable if possible to avoid lookups
// in the current directory which could be untrusted. Instead we use the ExecutableFinder.
$commandline[0] = (self::$executables[$commandline[0]] ??= (new ExecutableFinder())->find($commandline[0])) ?? $commandline[0];
}

return implode(' ', array_map($this->escapeArgument(...), $commandline));
}

Expand Down
10 changes: 5 additions & 5 deletions Tests/ProcessFailedExceptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput(

$exception = new ProcessFailedException($process);

$this->assertEquals(
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
$this->assertStringMatchesFormat(
"The command \"%s\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
str_replace("'php'", 'php', $exception->getMessage())
);
}
Expand Down Expand Up @@ -126,9 +126,9 @@ public function testDisabledOutputInFailedExceptionDoesNotPopulateOutput()

$exception = new ProcessFailedException($process);

$this->assertEquals(
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
str_replace("'php'", 'php', $exception->getMessage())
$this->assertStringMatchesFormat(
"The command \"%s\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
$exception->getMessage()
);
}
}
11 changes: 11 additions & 0 deletions Tests/ProcessTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,17 @@ public function testNotIgnoringSignal()
$this->assertSame(\SIGTERM, $process->getTermSignal());
}

public function testPathResolutionOnWindows()
{
if ('\\' !== \DIRECTORY_SEPARATOR) {
$this->markTestSkipped('This test is for Windows platform only');
}

$process = $this->getProcess(['where']);

$this->assertSame('C:\\Windows\\system32\\where.EXE', $process->getCommandLine());
}

private function getProcess(string|array $commandline, ?string $cwd = null, ?array $env = null, mixed $input = null, ?int $timeout = 60): Process
{
if (\is_string($commandline)) {
Expand Down

0 comments on commit 18f50a7

Please sign in to comment.