[Process] Decouple Process::findExecutable() from open_basedir #54151
Conversation
|
This PR has a failing test because we only run SIGCHILD tests on PHP 7.2. I wonder if this is specific to 7.2, or if we would see the same failures if we ran it on PHP 8+, too...? |
|
#47422 was merged as a feature into 6.4 which was still under development at that time. If you want us to reclassify this change as a bugfix, I think you need to elaborate on that. Otherwise, this PR will be closed soon because we don't backport features to LTS branches. |
|
Got it. I believe this is a bug. $finder = new \Symfony\Component\Process\ExecutableFinder();
$finder->find('docker'); // /usr/bin/docker
exec('which docker') // /user/bin/docker
ini_set('open_basedir', '/tmp/foo');
$finder->find('docker'); // null
exec('which docker') // /user/bin/docker |
|
Actually, this behavior is present on Process ^5, ^6, and ^7. PR #47422 doesn't solve it as I expected. Test file: <?php
require __DIR__ . '/vendor/autoload.php';
$finder = new \Symfony\Component\Process\ExecutableFinder();
var_dump($finder->find('docker'));
var_dump(exec('which docker'));
ini_set('open_basedir', '/tmp/foo');
var_dump($finder->find('docker'));
var_dump(exec('which docker')); |
|
It seems that <?php
require __DIR__ . '/vendor/autoload.php';
$finder = new \Symfony\Component\Process\ExecutableFinder();
$finder->find('docker'); // /usr/bin/docker
exec('which docker'); // /user/bin/docker
is_executable('/usr/bin/docker'); // true
exec('/usr/bin/docker --version'); // Docker version 25.0.2, build 29cf629222
ini_set('open_basedir', '/tmp/foo');
$finder->find('docker'); // null
exec('which docker'); // /user/bin/docker
is_executable('/usr/bin/docker'); // false
exec('/usr/bin/docker --version'); // Docker version 25.0.2, build 29cf629222
|
|
Actually, it's not that <?php
require __DIR__ . '/vendor/autoload.php';
$finder = new \Symfony\Component\Process\ExecutableFinder();
exec('mkdir -p /tmp/bar && rm -rf /tmp/bar/*.test');
touch('/tmp/bar/no-basedir-touch.test'); // Created.
exec('touch /tmp/bar/no-basedir-exec.test'); // Created.
ini_set('open_basedir', '/tmp/foo');
touch('/tmp/bar/touch-basedir.test'); // Not created.
exec('touch /tmp/bar/touch-basedir-exec.test'); // Created.Result: So the bug, after all, is using |
|
Works for me as a bugfix. |
|
But indeed the tests need to be taken care of! |
This PR doesn't fix it, this is still present in Symfony ^7 |
|
Initially, I thought that the upstream PR would fix it, but it doesn't. We can either consider it a "feature" not a "bug", or we can fix it. I just wonder if there are any security concerns on fixing this. |
|
I'll close this PR and open a bug report |
This PR ports the PR #47422 to Symfony Process 5.4