On Windows, don't rely on the OS to find executables #12180
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolas-grekas
force-pushed
the
windows-executable-finder
branch
7 times, most recently
from
da7e9af to
728106c
Compare
nicolas-grekas
commented
Oct 29, 2024
There was a problem hiding this comment.
Grepping for ProcessExecutor::escape is the best way to see what remains of string command-lines after this PR:
ZipDownloader: could be turned into array commandsUtil\Svn: this one is rooted for string command-lines (execute(string $command, ...))EventDispatcher: this class runs scripts only if allowed to so its safePerforce:executeCommandis document as@param non-empty-string $commandSvnDownloader: because it usesUtil\Svn
These classes are nonetheless hardened thanks to the added logic in ProcessExecutor.
nicolas-grekas
force-pushed
the
windows-executable-finder
branch
from
728106c to
e1caf42
Compare
|
Symfony "generic" PR at symfony/symfony#58710 |
… does not reset URL between commands
Seldaek
force-pushed
the
windows-executable-finder
branch
from
cfa5be4 to
7578df9
Compare
Seldaek
force-pushed
the
windows-executable-finder
branch
from
944c76a to
e641d60
Compare
nicolas-grekas
changed the title
Seldaek
force-pushed
the
windows-executable-finder
branch
from
69ceac3 to
a7fee29
Compare
Seldaek
force-pushed
the
windows-executable-finder
branch
2 times, most recently
from
995b37b to
88c5f2d
Compare
nicolas-grekas
added a commit
to symfony/symfony
that referenced
this pull request
Nov 4, 2024
…tables (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
symfony-splitter
pushed a commit
to symfony/process
that referenced
this pull request
Nov 4, 2024
…tables (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 ------- b35a7d42931 [Process] On Windows, don't rely on the OS to find executables
Seldaek
force-pushed
the
windows-executable-finder
branch
from
88c5f2d to
6058e49
Compare
composer.lockPackage changes
Dev Package changes
Settings · Docs · Powered by Private Packagist |
|
Thanks! |
|
Thanks too! |
|
The composer.lock diff comment has been updated to reflect new changes in this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Windows always looks up in the current directory when searching for a binary.
This is risky practice, so this PR tries to prevent this by using ExecutableFinder instead (which looks up using the PATH only).
While at it, I replaced all string command lines by array commands. This removed the need for explicit argument escaping.
Some remain, but most are removed.