feature #21470 [Process] Deprecate not inheriting env vars + compat related settings (nicolas-grekas)
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Process] Deprecate not inheriting env vars + compat related settings
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | yes
| Tests pass? | no
| Fixed tickets | -
| License | MIT
| Doc PR | -
Turning compat on/off is not a feature in itself.
About env vars: if one has unwanted env vars, one will still be able to remove them explicitly for the command. From my experience, not having eg PATH or HTTP_PROXY, etc. is more problematic. I'd prefer people to care about setting/unsetting the environment vars **they know about**, rather than allowing them to start with no ENV and discover later that they missed setting some var.
Commits
-------
df14451a73
[Process] Deprecate not inheriting env vars + compat related settings
This commit is contained in:
commit
46daa359ea
|
@ -96,7 +96,7 @@ script:
|
|||
- if [[ ! $deps && ! $PHP = hhvm* ]]; then echo "$COMPONENTS" | parallel --gnu '$PHPUNIT --exclude-group tty,benchmark,intl-data {}'"$REPORT"; fi
|
||||
- if [[ ! $deps && ! $PHP = hhvm* ]]; then echo -e "\\nRunning tests requiring tty"; $PHPUNIT --group tty; fi
|
||||
- if [[ ! $deps && $PHP = hhvm* ]]; then $PHPUNIT --exclude-group benchmark,intl-data; fi
|
||||
- if [[ ! $deps && $PHP = ${MIN_PHP%.*} ]]; then echo -e "1\\n0" | xargs -I{} sh -c 'echo "\\nPHP --enable-sigchild enhanced={}" && ENHANCE_SIGCHLD={} php-$MIN_PHP/sapi/cli/php .phpunit/phpunit-4.8/phpunit --colors=always src/Symfony/Component/Process/'; fi
|
||||
- if [[ ! $deps && $PHP = ${MIN_PHP%.*} ]]; then echo -e "1\\n0" | xargs -I{} sh -c 'echo "\\nPHP --enable-sigchild enhanced={}" && SYMFONY_DEPRECATIONS_HELPER=weak ENHANCE_SIGCHLD={} php-$MIN_PHP/sapi/cli/php .phpunit/phpunit-4.8/phpunit --colors=always src/Symfony/Component/Process/'; fi
|
||||
- if [[ $deps = high ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --ansi; $PHPUNIT --exclude-group tty,benchmark,intl-data'$LEGACY"$REPORT"; fi
|
||||
- if [[ $deps = low ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --ansi --prefer-lowest --prefer-stable; $PHPUNIT --exclude-group tty,benchmark,intl-data'"$REPORT"; fi
|
||||
# Test the PhpUnit bridge using the original phpunit script
|
||||
|
|
|
@ -63,6 +63,15 @@ HttpKernel
|
|||
* The `Psr6CacheClearer::addPool()` method has been deprecated. Pass an array of pools indexed
|
||||
by name to the constructor instead.
|
||||
|
||||
Process
|
||||
-------
|
||||
|
||||
* Not inheriting environment variables is deprecated.
|
||||
|
||||
* Configuring `proc_open()` options is deprecated.
|
||||
|
||||
* Configuring Windows and sigchild compatibility is deprecated - they will be always enabled in 4.0.
|
||||
|
||||
Security
|
||||
--------
|
||||
|
||||
|
|
|
@ -225,6 +225,15 @@ HttpKernel
|
|||
* The `Psr6CacheClearer::addPool()` method has been removed. Pass an array of pools indexed
|
||||
by name to the constructor instead.
|
||||
|
||||
Process
|
||||
-------
|
||||
|
||||
* Environment variables are always inherited in sub-processes.
|
||||
|
||||
* Configuring `proc_open()` options has been removed.
|
||||
|
||||
* Configuring Windows and sigchild compatibility is not possible anymore - they are always enabled.
|
||||
|
||||
Security
|
||||
--------
|
||||
|
||||
|
|
|
@ -1,6 +1,14 @@
|
|||
CHANGELOG
|
||||
=========
|
||||
|
||||
3.3.0
|
||||
-----
|
||||
|
||||
* deprecated not inheriting environment variables
|
||||
* deprecated configuring `proc_open()` options
|
||||
* deprecated configuring enhanced Windows compatibility
|
||||
* deprecated configuring enhanced sigchild compatibility
|
||||
|
||||
2.5.0
|
||||
-----
|
||||
|
||||
|
|
|
@ -33,7 +33,7 @@ class PhpProcess extends Process
|
|||
* @param int $timeout The timeout in seconds
|
||||
* @param array $options An array of options for proc_open
|
||||
*/
|
||||
public function __construct($script, $cwd = null, array $env = null, $timeout = 60, array $options = array())
|
||||
public function __construct($script, $cwd = null, array $env = null, $timeout = 60, array $options = null)
|
||||
{
|
||||
$executableFinder = new PhpExecutableFinder();
|
||||
if (false === $php = $executableFinder->find()) {
|
||||
|
@ -52,6 +52,9 @@ class PhpProcess extends Process
|
|||
// command with exec
|
||||
$php = 'exec '.$php;
|
||||
}
|
||||
if (null !== $options) {
|
||||
@trigger_error(sprintf('The $options parameter of the %s constructor is deprecated since version 3.3 and will be removed in 4.0.', __CLASS__), E_USER_DEPRECATED);
|
||||
}
|
||||
|
||||
parent::__construct($php, $cwd, $env, $script, $timeout, $options);
|
||||
}
|
||||
|
|
|
@ -58,7 +58,7 @@ class Process implements \IteratorAggregate
|
|||
private $lastOutputTime;
|
||||
private $timeout;
|
||||
private $idleTimeout;
|
||||
private $options;
|
||||
private $options = array('suppress_errors' => true);
|
||||
private $exitcode;
|
||||
private $fallbackStatus = array();
|
||||
private $processInformation;
|
||||
|
@ -145,7 +145,7 @@ class Process implements \IteratorAggregate
|
|||
*
|
||||
* @throws RuntimeException When proc_open is not installed
|
||||
*/
|
||||
public function __construct($commandline, $cwd = null, array $env = null, $input = null, $timeout = 60, array $options = array())
|
||||
public function __construct($commandline, $cwd = null, array $env = null, $input = null, $timeout = 60, array $options = null)
|
||||
{
|
||||
if (!function_exists('proc_open')) {
|
||||
throw new RuntimeException('The Process class relies on proc_open, which is not available on your PHP installation.');
|
||||
|
@ -171,7 +171,10 @@ class Process implements \IteratorAggregate
|
|||
$this->pty = false;
|
||||
$this->enhanceWindowsCompatibility = true;
|
||||
$this->enhanceSigchildCompatibility = '\\' !== DIRECTORY_SEPARATOR && $this->isSigchildEnabled();
|
||||
$this->options = array_replace(array('suppress_errors' => true, 'binary_pipes' => true), $options);
|
||||
if (null !== $options) {
|
||||
@trigger_error(sprintf('The $options parameter of the %s constructor is deprecated since version 3.3 and will be removed in 4.0.', __CLASS__), E_USER_DEPRECATED);
|
||||
$this->options = array_replace($this->options, $options);
|
||||
}
|
||||
}
|
||||
|
||||
public function __destruct()
|
||||
|
@ -268,26 +271,23 @@ 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 = $this->env;
|
||||
$envBackup = array();
|
||||
if (null !== $env && $this->inheritEnv) {
|
||||
foreach ($env as $k => $v) {
|
||||
$envBackup[$k] = getenv($v);
|
||||
putenv(false === $v || null === $v ? $k : "$k=$v");
|
||||
}
|
||||
$env = null;
|
||||
} else {
|
||||
$env = $this->env;
|
||||
} elseif (null !== $env) {
|
||||
@trigger_error(sprintf('Not inheriting environment variables is deprecated since Symfony 3.3 and will always happen in 4.0. Set "Process::inheritEnvironmentVariables()" to true instead.', __METHOD__), E_USER_DEPRECATED);
|
||||
}
|
||||
if ('\\' === DIRECTORY_SEPARATOR && $this->enhanceWindowsCompatibility) {
|
||||
$commandline = 'cmd /V:ON /E:ON /D /C "('.$envline.$commandline.')';
|
||||
$commandline = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $commandline).')';
|
||||
foreach ($this->processPipes->getFiles() as $offset => $filename) {
|
||||
$commandline .= ' '.$offset.'>'.ProcessUtils::escapeArgument($filename);
|
||||
$commandline .= ' '.$offset.'>"'.$filename.'"';
|
||||
}
|
||||
$commandline .= '"';
|
||||
|
||||
if (!isset($this->options['bypass_shell'])) {
|
||||
$this->options['bypass_shell'] = true;
|
||||
|
@ -297,18 +297,20 @@ class Process implements \IteratorAggregate
|
|||
$descriptors[3] = array('pipe', 'w');
|
||||
|
||||
// See https://unix.stackexchange.com/questions/71205/background-process-pipe-input
|
||||
$commandline = $envline.'{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;';
|
||||
$commandline = '{ ('.$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, $env, $this->options);
|
||||
|
||||
foreach ($envBackup as $k => $v) {
|
||||
putenv(false === $v ? $k : "$k=$v");
|
||||
}
|
||||
|
||||
if (!is_resource($this->process)) {
|
||||
throw new RuntimeException('Unable to launch a new process.');
|
||||
}
|
||||
|
@ -1089,6 +1091,8 @@ class Process implements \IteratorAggregate
|
|||
*
|
||||
* An environment variable value should be a string.
|
||||
* If it is an array, the variable is ignored.
|
||||
* If it is false or null, it will be removed when
|
||||
* env vars are otherwise inherited.
|
||||
*
|
||||
* That happens in PHP when 'argv' is registered into
|
||||
* the $_ENV array for instance.
|
||||
|
@ -1099,15 +1103,7 @@ class Process implements \IteratorAggregate
|
|||
*/
|
||||
public function setEnv(array $env)
|
||||
{
|
||||
// Process can not handle env values that are arrays
|
||||
$env = array_filter($env, function ($value) {
|
||||
return !is_array($value);
|
||||
});
|
||||
|
||||
$this->env = array();
|
||||
foreach ($env as $key => $value) {
|
||||
$this->env[$key] = (string) $value;
|
||||
}
|
||||
$this->env = $env;
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
@ -1148,9 +1144,13 @@ class Process implements \IteratorAggregate
|
|||
* Gets the options for proc_open.
|
||||
*
|
||||
* @return array The current options
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0.
|
||||
*/
|
||||
public function getOptions()
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
return $this->options;
|
||||
}
|
||||
|
||||
|
@ -1160,9 +1160,13 @@ class Process implements \IteratorAggregate
|
|||
* @param array $options The new options
|
||||
*
|
||||
* @return self The current Process instance
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0.
|
||||
*/
|
||||
public function setOptions(array $options)
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
$this->options = $options;
|
||||
|
||||
return $this;
|
||||
|
@ -1174,9 +1178,13 @@ class Process implements \IteratorAggregate
|
|||
* This is true by default.
|
||||
*
|
||||
* @return bool
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0. Enhanced Windows compatibility will always be enabled.
|
||||
*/
|
||||
public function getEnhanceWindowsCompatibility()
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Enhanced Windows compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
return $this->enhanceWindowsCompatibility;
|
||||
}
|
||||
|
||||
|
@ -1186,9 +1194,13 @@ class Process implements \IteratorAggregate
|
|||
* @param bool $enhance
|
||||
*
|
||||
* @return self The current Process instance
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0. Enhanced Windows compatibility will always be enabled.
|
||||
*/
|
||||
public function setEnhanceWindowsCompatibility($enhance)
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Enhanced Windows compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
$this->enhanceWindowsCompatibility = (bool) $enhance;
|
||||
|
||||
return $this;
|
||||
|
@ -1198,9 +1210,13 @@ class Process implements \IteratorAggregate
|
|||
* Returns whether sigchild compatibility mode is activated or not.
|
||||
*
|
||||
* @return bool
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0. Sigchild compatibility will always be enabled.
|
||||
*/
|
||||
public function getEnhanceSigchildCompatibility()
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Sigchild compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
return $this->enhanceSigchildCompatibility;
|
||||
}
|
||||
|
||||
|
@ -1214,9 +1230,13 @@ class Process implements \IteratorAggregate
|
|||
* @param bool $enhance
|
||||
*
|
||||
* @return self The current Process instance
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0.
|
||||
*/
|
||||
public function setEnhanceSigchildCompatibility($enhance)
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Sigchild compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
$this->enhanceSigchildCompatibility = (bool) $enhance;
|
||||
|
||||
return $this;
|
||||
|
@ -1231,6 +1251,10 @@ class Process implements \IteratorAggregate
|
|||
*/
|
||||
public function inheritEnvironmentVariables($inheritEnv = true)
|
||||
{
|
||||
if (!$inheritEnv) {
|
||||
@trigger_error(sprintf('Not inheriting environment variables is deprecated since Symfony 3.3 and will always happen in 4.0. Set "Process::inheritEnvironmentVariables()" to true instead.', __METHOD__), E_USER_DEPRECATED);
|
||||
}
|
||||
|
||||
$this->inheritEnv = (bool) $inheritEnv;
|
||||
|
||||
return $this;
|
||||
|
@ -1240,9 +1264,13 @@ class Process implements \IteratorAggregate
|
|||
* Returns whether environment variables will be inherited or not.
|
||||
*
|
||||
* @return bool
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0. Environment variables will always be inherited.
|
||||
*/
|
||||
public function areEnvironmentVariablesInherited()
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Environment variables will always be inherited.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
return $this->inheritEnv;
|
||||
}
|
||||
|
||||
|
|
|
@ -26,7 +26,7 @@ class ProcessBuilder
|
|||
private $env = array();
|
||||
private $input;
|
||||
private $timeout = 60;
|
||||
private $options = array();
|
||||
private $options;
|
||||
private $inheritEnv = true;
|
||||
private $prefix = array();
|
||||
private $outputDisabled = false;
|
||||
|
@ -120,9 +120,13 @@ class ProcessBuilder
|
|||
* @param bool $inheritEnv
|
||||
*
|
||||
* @return $this
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0.
|
||||
*/
|
||||
public function inheritEnvironmentVariables($inheritEnv = true)
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
$this->inheritEnv = $inheritEnv;
|
||||
|
||||
return $this;
|
||||
|
@ -217,9 +221,13 @@ class ProcessBuilder
|
|||
* @param string $value The option value
|
||||
*
|
||||
* @return $this
|
||||
*
|
||||
* @deprecated since version 3.3, to be removed in 4.0.
|
||||
*/
|
||||
public function setOption($name, $value)
|
||||
{
|
||||
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);
|
||||
|
||||
$this->options[$name] = $value;
|
||||
|
||||
return $this;
|
||||
|
@ -262,12 +270,10 @@ class ProcessBuilder
|
|||
throw new LogicException('You must add() command arguments before calling getProcess().');
|
||||
}
|
||||
|
||||
$options = $this->options;
|
||||
|
||||
$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);
|
||||
$process = new Process($script, $this->cwd, $this->env, $this->input, $this->timeout, $this->options);
|
||||
|
||||
if ($this->inheritEnv) {
|
||||
$process->inheritEnvironmentVariables();
|
||||
|
|
|
@ -15,6 +15,9 @@ use Symfony\Component\Process\ProcessBuilder;
|
|||
|
||||
class ProcessBuilderTest extends \PHPUnit_Framework_TestCase
|
||||
{
|
||||
/**
|
||||
* @group legacy
|
||||
*/
|
||||
public function testInheritEnvironmentVars()
|
||||
{
|
||||
$proc = ProcessBuilder::create()
|
||||
|
@ -22,6 +25,13 @@ class ProcessBuilderTest extends \PHPUnit_Framework_TestCase
|
|||
->getProcess();
|
||||
|
||||
$this->assertTrue($proc->areEnvironmentVariablesInherited());
|
||||
|
||||
$proc = ProcessBuilder::create()
|
||||
->add('foo')
|
||||
->inheritEnvironmentVariables(false)
|
||||
->getProcess();
|
||||
|
||||
$this->assertFalse($proc->areEnvironmentVariablesInherited());
|
||||
}
|
||||
|
||||
public function testAddEnvironmentVariables()
|
||||
|
@ -35,12 +45,10 @@ class ProcessBuilderTest extends \PHPUnit_Framework_TestCase
|
|||
->add('command')
|
||||
->setEnv('foo', 'bar2')
|
||||
->addEnvironmentVariables($env)
|
||||
->inheritEnvironmentVariables(false)
|
||||
->getProcess()
|
||||
;
|
||||
|
||||
$this->assertSame($env, $proc->getEnv());
|
||||
$this->assertFalse($proc->areEnvironmentVariablesInherited());
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -1383,15 +1383,12 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
|
|||
$this->assertSame('456', $p2->getOutput());
|
||||
}
|
||||
|
||||
public function testInheritEnvEnabled()
|
||||
public function testEnvIsInherited()
|
||||
{
|
||||
$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');
|
||||
|
@ -1400,12 +1397,16 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
|
|||
$this->assertEquals($expected, $env);
|
||||
}
|
||||
|
||||
/**
|
||||
* @group legacy
|
||||
*/
|
||||
public function testInheritEnvDisabled()
|
||||
{
|
||||
$process = $this->getProcess(self::$phpBin.' -r '.escapeshellarg('echo serialize($_SERVER);'), null, array('BAR' => 'BAZ'));
|
||||
|
||||
putenv('FOO=BAR');
|
||||
|
||||
$this->assertSame($process, $process->inheritEnvironmentVariables(false));
|
||||
$this->assertFalse($process->areEnvironmentVariablesInherited());
|
||||
|
||||
$process->run();
|
||||
|
@ -1427,9 +1428,10 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
|
|||
*
|
||||
* @return Process
|
||||
*/
|
||||
private function getProcess($commandline, $cwd = null, array $env = null, $input = null, $timeout = 60, array $options = array())
|
||||
private function getProcess($commandline, $cwd = null, array $env = null, $input = null, $timeout = 60)
|
||||
{
|
||||
$process = new Process($commandline, $cwd, $env, $input, $timeout, $options);
|
||||
$process = new Process($commandline, $cwd, $env, $input, $timeout);
|
||||
$process->inheritEnvironmentVariables();
|
||||
|
||||
if (false !== $enhance = getenv('ENHANCE_SIGCHLD')) {
|
||||
try {
|
||||
|
|
Reference in New Issue