From 524508cdcf492867cc46a9db28d1f63b6ed871f3 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 14 Jun 2016 15:16:11 +0200 Subject: [PATCH] [Process] Allow merging env vars instead of replacing them --- src/Symfony/Component/Process/Process.php | 46 +++++++++++++++++-- .../Component/Process/ProcessBuilder.php | 9 ++-- .../Process/Tests/ProcessBuilderTest.php | 25 +--------- .../Component/Process/Tests/ProcessTest.php | 34 ++++++++++++++ 4 files changed, 82 insertions(+), 32 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 53c4d8781d..3fd1fd1246 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -73,6 +73,7 @@ class Process implements \IteratorAggregate private $incrementalErrorOutputOffset = 0; private $tty; private $pty; + private $inheritEnv = false; private $useFileHandles = false; /** @var PipesInterface */ @@ -267,9 +268,22 @@ class Process implements \IteratorAggregate $descriptors = $this->getDescriptors(); $commandline = $this->commandline; + $envline = ''; + if (null !== $this->env && $this->inheritEnv) { + if ('\\' === DIRECTORY_SEPARATOR && !empty($this->options['bypass_shell']) && !$this->enhanceWindowsCompatibility) { + throw new LogicException('The "bypass_shell" option must be false to inherit environment variables while enhanced Windows compatibility is off'); + } + $env = '\\' === DIRECTORY_SEPARATOR ? '(SET %s)&&' : 'export %s;'; + foreach ($this->env as $k => $v) { + $envline .= sprintf($env, ProcessUtils::escapeArgument("$k=$v")); + } + $env = null; + } else { + $env = $this->env; + } if ('\\' === DIRECTORY_SEPARATOR && $this->enhanceWindowsCompatibility) { - $commandline = 'cmd /V:ON /E:ON /D /C "('.$commandline.')'; + $commandline = 'cmd /V:ON /E:ON /D /C "('.$envline.$commandline.')'; foreach ($this->processPipes->getFiles() as $offset => $filename) { $commandline .= ' '.$offset.'>'.ProcessUtils::escapeArgument($filename); } @@ -283,15 +297,17 @@ class Process implements \IteratorAggregate $descriptors[3] = array('pipe', 'w'); // See https://unix.stackexchange.com/questions/71205/background-process-pipe-input - $commandline = '{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;'; + $commandline = $envline.'{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;'; $commandline .= 'pid=$!; echo $pid >&3; wait $pid; code=$?; echo $code >&3; exit $code'; // Workaround for the bug, when PTS functionality is enabled. // @see : https://bugs.php.net/69442 $ptsWorkaround = fopen(__FILE__, 'r'); + } elseif ('' !== $envline) { + $commandline = $envline.$commandline; } - $this->process = proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $this->env, $this->options); + $this->process = proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $env, $this->options); if (!is_resource($this->process)) { throw new RuntimeException('Unable to launch a new process.'); @@ -1197,6 +1213,30 @@ class Process implements \IteratorAggregate return $this; } + /** + * Sets whether environment variables will be inherited or not. + * + * @param bool $inheritEnv + * + * @return self The current Process instance + */ + public function inheritEnvironmentVariables($inheritEnv = true) + { + $this->inheritEnv = (bool) $inheritEnv; + + return $this; + } + + /** + * Returns whether environment variables will be inherited or not. + * + * @return bool + */ + public function areEnvironmentVariablesInherited() + { + return $this->inheritEnv; + } + /** * Performs a check between the timeout definition and the time the process started. * diff --git a/src/Symfony/Component/Process/ProcessBuilder.php b/src/Symfony/Component/Process/ProcessBuilder.php index 194594afc2..0a43416d4d 100644 --- a/src/Symfony/Component/Process/ProcessBuilder.php +++ b/src/Symfony/Component/Process/ProcessBuilder.php @@ -267,14 +267,11 @@ class ProcessBuilder $arguments = array_merge($this->prefix, $this->arguments); $script = implode(' ', array_map(array(__NAMESPACE__.'\\ProcessUtils', 'escapeArgument'), $arguments)); + $process = new Process($script, $this->cwd, $this->env, $this->input, $this->timeout, $options); + if ($this->inheritEnv) { - $env = array_replace($_ENV, $_SERVER, $this->env); - } else { - $env = $this->env; + $process->inheritEnvironmentVariables(); } - - $process = new Process($script, $this->cwd, $env, $this->input, $this->timeout, $options); - if ($this->outputDisabled) { $process->disableOutput(); } diff --git a/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php b/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php index 3e29af710f..e229c1bea6 100644 --- a/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php +++ b/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php @@ -17,17 +17,11 @@ class ProcessBuilderTest extends \PHPUnit_Framework_TestCase { public function testInheritEnvironmentVars() { - $_ENV['MY_VAR_1'] = 'foo'; - $proc = ProcessBuilder::create() ->add('foo') ->getProcess(); - unset($_ENV['MY_VAR_1']); - - $env = $proc->getEnv(); - $this->assertArrayHasKey('MY_VAR_1', $env); - $this->assertEquals('foo', $env['MY_VAR_1']); + $this->assertTrue($proc->areEnvironmentVariablesInherited()); } public function testAddEnvironmentVariables() @@ -46,22 +40,7 @@ class ProcessBuilderTest extends \PHPUnit_Framework_TestCase ; $this->assertSame($env, $proc->getEnv()); - } - - public function testProcessShouldInheritAndOverrideEnvironmentVars() - { - $_ENV['MY_VAR_1'] = 'foo'; - - $proc = ProcessBuilder::create() - ->setEnv('MY_VAR_1', 'bar') - ->add('foo') - ->getProcess(); - - unset($_ENV['MY_VAR_1']); - - $env = $proc->getEnv(); - $this->assertArrayHasKey('MY_VAR_1', $env); - $this->assertEquals('bar', $env['MY_VAR_1']); + $this->assertFalse($proc->areEnvironmentVariablesInherited()); } /** diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php index 0ebd307ee2..77b1b3b5bc 100644 --- a/src/Symfony/Component/Process/Tests/ProcessTest.php +++ b/src/Symfony/Component/Process/Tests/ProcessTest.php @@ -1357,6 +1357,40 @@ class ProcessTest extends \PHPUnit_Framework_TestCase $this->assertSame('456', $p2->getOutput()); } + public function testInheritEnvEnabled() + { + $process = $this->getProcess(self::$phpBin.' -r '.escapeshellarg('echo serialize($_SERVER);'), null, array('BAR' => 'BAZ')); + + putenv('FOO=BAR'); + + $this->assertSame($process, $process->inheritEnvironmentVariables(1)); + $this->assertTrue($process->areEnvironmentVariablesInherited()); + + $process->run(); + + $expected = array('BAR' => 'BAZ', 'FOO' => 'BAR'); + $env = array_intersect_key(unserialize($process->getOutput()), $expected); + + $this->assertSame($expected, $env); + } + + public function testInheritEnvDisabled() + { + $process = $this->getProcess(self::$phpBin.' -r '.escapeshellarg('echo serialize($_SERVER);'), null, array('BAR' => 'BAZ')); + + putenv('FOO=BAR'); + + $this->assertFalse($process->areEnvironmentVariablesInherited()); + + $process->run(); + + $expected = array('BAR' => 'BAZ', 'FOO' => 'BAR'); + $env = array_intersect_key(unserialize($process->getOutput()), $expected); + unset($expected['FOO']); + + $this->assertSame($expected, $env); + } + /** * @param string $commandline * @param null|string $cwd