8000 [Process] add a method to the ProcessBuilder to automatically prepend the executed script with "exec" by xabbuh · Pull Request #11335 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Process] add a method to the ProcessBuilder to automatically prepend the executed script with "exec" #11335

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[Process] add a method to the ProcessBuilder to automatically prepend the executed script with "exec" #11335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions src/Symfony/Component/Process/ProcessBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Process\Exception\InvalidArgumentException;
use Symfony\Component\Process\Exception\LogicException;
use Symfony\Component\Process\Exception\RuntimeException;

/**
* Process builder.
Expand All @@ -30,6 +31,12 @@ class ProcessBuilder
private $inheritEnv = true;
private $prefix = array();
private $outputDisabled = false;
private $shellWrapperEnabled = false;

/**
* @var bool
*/
private static $sigchild;

/**
* Constructor
Expand Down Expand Up @@ -251,6 +258,40 @@ public function enableOutput()
return $this;
}

/**
* Do not enable the shell wrapper (i.e. do not prepend the executed command with "exec").
*
* @return ProcessBuilder
*/
public function disableShellWrapper()
{
$this->shellWrapperEnabled = false;

return $this;
}

/**
* Enable the shell wrapper (i.e. prepend the executed command with "exec").
*
* @return ProcessBuilder
*
* @throws RuntimeException if the OS is Windows or if PHP was built with --enable-sigchild
*/
public function enableShellWrapper()
{
if (defined('PHP_WINDOWS_VERSION_BUILD')) {
throw new RuntimeException('The shell wrapper is not supported on Windows platforms.');
}

if ($this->isSigchildEnabled()) {
throw new RuntimeException('This PHP has been compiled with --enable-sigchild. The shell wrapper cannot be enabled.');
}

$this->shellWrapperEnabled = true;

return $this;
}

/**
* Creates a Process instance and returns it.
*
Expand All @@ -269,6 +310,10 @@ public function getProcess()
$arguments = array_merge($this->prefix, $this->arguments);
$script = implode(' ', array_map(array(__NAMESPACE__.'\\ProcessUtils', 'escapeArgument'), $arguments));

if ($this->shellWrapperEnabled) {
$script = 'exec '.$script;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prepending with exec will break several cases, which are not prevented by your current patch:

  • Windows (which does not have exec)
  • complex commands (using &&, || or multiple commands separated by ;)
  • enhanced sigchild compatibility mode (because of the previous case)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Does Windows have a similar command?
  • How would you use exec in the context of multiple commands? Is that actually possible at all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot use exec for multiple commands.
But actually, I'm not sure the ProcessBuilder allows building such commands though. It needs to be checked.
the case of sigchild is an issue though.

I don't know about such a command for windows. @romainneutron do you have an idea ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I see, the ProcessBuilder cannot create multiple commands. Running something like this:

$builder = new ProcessBuilder(array(
    'php',
    './test1.php',
    ';',
    'php',
    './test2.php',
));

passes ;, php and ./test2.php as arguments to the test1.php script.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romainneutron Do you have any thoughts about this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProcessBuilder cannot create chained commands (commands separated by ; or &&), it only allows single commands.

I don't know any equivalent on windows, but I don't think it's relevant on this platform: We would use exec to solve the issue mentioned in #5759 (The sh wrapper paragraph)

I think we could allow preprending on linux system that are not in sigchild environment. I've to admit this is a hacky way to solve this PHP issue, however it works very well and would allow using the ProcessBuilder when the use of exec is needed: There is no way to use this functionality at the moment: as the ProcessBuilder escapes all arguments and as exec is not a command (it must not be escaped), the only way is to build the command by hand, which is not something I would recommend.

I'd be in favor of such addition, with all the care and love suggested by @stof (windows and sigchild environments detection to avoid errors)


if ($this->inheritEnv) {
// include $_ENV for BC purposes
$env = array_replace($_ENV, $_SERVER, $this->env);
Expand All @@ -284,4 +329,25 @@ public function getProcess()

return $process;
}

/**
* Returns whether PHP has been compiled with the '--enable-sigchild' option or not.
*
* @return bool
*/
private function isSigchildEnabled()
{
if (null !== self::$sigchild) {
return self::$sigchild;
}

if (!function_exists('phpinfo')) {
return self::$sigchild = false;
}

ob_start();
phpinfo(INFO_GENERAL);

return self::$sigchild = false !== strpos(ob_get_clean(), '--enable-sigchild');
}
}
83 changes: 83 additions & 0 deletions src/Symfony/Component/Process/Tests/ProcessBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@

namespace Symfony\Component\Process\Tests;

use Symfony\Component\Process\Exception\RuntimeException;
use Symfony\Component\Process\ProcessBuilder;

class ProcessBuilderTest extends \PHPUnit_Framework_TestCase
{
private static $sigchild;

public function testInheritEnvironmentVars()
{
$_ENV['MY_VAR_1'] = 'foo';
Expand Down Expand Up @@ -222,4 +225,84 @@ public function testInvalidInput()
$builder = ProcessBuilder::create();
$builder->setInput(array());
}

public function testShouldNotPrependCommandLineWithExecByDefault()
{
$process = ProcessBuilder::create()
->add('foo')
->getProcess();

$this->assertNotRegExp('/^exec /', $process->getCommandLine());
}

public function testShouldPrependCommandLineWithExec()
{
if ($this->isSigchildEnabled()) {
$this->markTestSkipped('->enableShellWrapper() is not supported in sigchild environment');
}

$process = ProcessBuilder::create()
->add('foo')
->enableShellWrapper()
->getProcess();

$this->assertRegExp('/^exec /', $process->getCommandLine());
}

/**
* @expectedException RuntimeException
* @expectedExceptionMessage The shell wrapper is not supported on Windows platforms.
*/
public function testEnableShellWrapperOnWindowsThrowsException()
{
if (!defined('PHP_WINDOWS_VERSION_BUILD')) {
$this->markTestSkipped('->enableShellWrapper() is not supported on Windows');
}

ProcessBuilder::create()->enableShellWrapper();
}

/**
* @expectedException RuntimeException
* @expectedExceptionMessage This PHP has been compiled with --enable-sigchild. The shell wrapper cannot be enabled.
*/
public function testEnableShellWrapperInSigchildEnvironmentThrowsException()
{
if (!$this->isSigchildEnabled()) {
$this->markTestSkipped('->enableShellWrapper() is not supported in sigchild environment');
}

ProcessBuilder::create()->enableShellWrapper();
}

public function testShouldNotPrependCommandLineWithExec()
{
$process = ProcessBuilder::create()
->add('foo')
->disableShellWrapper()
->getProcess();

$this->assertNotRegExp('/^exec /', $process->getCommandLine());
}

/**
* Returns whether PHP has been compiled with the '--enable-sigchild' option or not.
*
* @return bool
*/
private function isSigchildEnabled()
{
if (null !== self::$sigchild) {
return self::$sigchild;
}

if (!function_exists('phpinfo')) {
return self::$sigchild = false;
}

ob_start();
phpinfo(INFO_GENERAL);

return self::$sigchild = false !== strpos(ob_get_clean(), '--enable-sigchild');
}
}
0