8000 minor #58710 [Process] On Windows, don't rely on the OS to find execu… · symfony/symfony@a86878f · GitHub
[go: up one dir, main page]

Skip to content

Commit a86878f

Browse files
minor #58710 [Process] On Windows, don't rely on the OS to find executables (nicolas-grekas)
This PR was merged into the 7.2 branch. Discussion ---------- [Process] On Windows, don't rely on the OS to find executables | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT Porting part of composer/composer#12180 here: On Windows, when searching for an executable, the OS always looks at the current directory before using the PATH variable. This makes it easier than desired to hijack executables. Unix-like OSes don't have this issue. This PR proposes to rely on ExecutableFinder instead. Commits ------- b35a7d4 [Process] On Windows, don't rely on the OS to find executables
2 parents 663bf2a + b35a7d4 commit a86878f

File tree

3 files changed

+23
-5
lines changed

3 files changed

+23
-5
lines changed

src/Symfony/Component/Process/Process.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ class Process imple 8000 ments \IteratorAggregate
8585
private ?int $cachedExitCode = null;
8686

8787
private static ?bool $sigchild = null;
88+
private static array $executables = [];
8889

8990
/**
9091
* Exit codes translation table.
@@ -1543,6 +1544,12 @@ private function buildShellCommandline(string|array $commandline): string
15431544
return $commandline;
15441545
}
15451546

1547+
if ('\\' === \DIRECTORY_SEPARATOR && isset($commandline[0][0]) && \strlen($commandline[0]) === strcspn($commandline[0], ':/\\')) {
1548+
// On Windows, we don't rely on the OS to find the executable if possible to avoid lookups
1549+
// in the current directory which could be untrusted. Instead we use the ExecutableFinder.
1550+
$commandline[0] = (self::$executables[$commandline[0]] ??= (new ExecutableFinder())->find($commandline[0])) ?? $commandline[0];
1551+
}
1552+
15461553
return implode(' ', array_map($this->escapeArgument(...), $commandline));
15471554
}
15481555

src/Symfony/Component/Process/Tests/ProcessFailedExceptionTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput(
8080

8181
$exception = new ProcessFailedException($process);
8282

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

127127
$exception = new ProcessFailedException($process);
128128

129-
$this->assertEquals(
130-
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
131-
str_replace("'php'", 'php', $exception->getMessage())
129+
$this->assertStringMatchesFormat(
130+
"The command \"%s\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
131+
$exception->getMessage()
132132
);
133133
}
134134
}

src/Symfony/Component/Process/Tests/ProcessTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,6 +1692,17 @@ public function testNotIgnoringSignal()
16921692
$this->assertSame(\SIGTERM, $process->getTermSignal());
16931693
}
16941694

1695+
public function testPathResolutionOnWindows()
1696+
{
1697+
if ('\\' !== \DIRECTORY_SEPARATOR) {
1698+
$this->markTestSkipped('This test is for Windows platform only');
1699+
}
1700+
1701+
$process = $this->getProcess(['where']);
1702+
1703+
$this->assertSame('C:\\Windows\\system32\\where.EXE', $process->getCommandLine());
1704+
}
1705+
16951706
private function getProcess(string|array $commandline, ?string $cwd = null, ?array $env = null, mixed $input = null, ?int $timeout = 60): Process
16961707
{
16971708
if (\is_string($commandline)) {

0 commit comments

Comments
 (0)
0