-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Process] Support using Process::findExecutable()
independently of open_basedir
#47422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Process] Support using Process::findExecutable()
independently of open_basedir
#47422
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. There's a related failure on Windows, see appveyor.
Please add a test case also.
Can you point me where the error occurs? Have bit of a problem interprerting https://ci.appveyor.com/project/fabpot/symfony/builds/44759158 - I only see |
@BlackbitNeueMedien This is because this URL was showing only the end of the logs, as explained in the banner on top. Looking at the full logs, I see an issue: https://ci.appveyor.com/project/fabpot/symfony/builds/44759158?fullLog=true#L1706 shows that your new code leaks to stderr However, I'm still not sure what changes the exit code to a failure one. |
Process::findExecutable()
independently of open_basedir
1e0428c
to
5af7d53
Compare
Thank you @BlackbitDevs. |
This PR was merged into the 6.4 branch. Discussion ---------- [Process] fix tests | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - - `testFindProcessInOpenBasedir` is a duplicate of `testFindWithOpenBaseDir` - `testFindWithOpenBaseDir` currently expects that we search open_basedir instead of PATH when the setting is set, but this doesn't really make sense, and #47422 removed this behavior - `PhpSubprocessTest::testSubprocess` expects a php that defaults to memory_limit=-1, which is not the case currently for the sigchild-enabled binary Commits ------- 4ca4417 [Process] fix tests
…sedir (BlackbitDevs) This PR was merged into the 5.4 branch. Discussion ---------- [Process] Fix finding executables independently of open_basedir | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT This backports #47422 to 5.4, which is a bugfix really. Instead of #58008 and #57954 /cc `@xabbuh` `@fritzmg` Commits ------- 4424763 [Process] Fix finding executables independently of open_basedir
With this PR
Console::findExecutable()
will find executables also if the their path is not allowed inopen_basedir
config similar tosymfony/src/Symfony/Component/Process/PhpExecutableFinder.php
Lines 36 to 41 in ddaedd2
Console::findExecutable()
's responsibility is to find an executable which can be called with a Symfony Process or by PHP's functions likeexec
,system
etc.The goal of PHP's
open_basedir
config is to restrict reading / writing files within PHP processes. Imho this is completely independent of finding an executable.If PHP's intention was to restrict executing applications which are not present in
open_basedir
's paths, it would have been implemented there.