8000 merged branch naitsirch/process-inherit-env-fix (PR #8718) · daifma/symfony@e617ddb · GitHub
[go: up one dir, main page]
Skip to content

Commit e617ddb

Browse files
fabpotfabpot
committed
merged branch naitsirch/process-inherit-env-fix (PR symfony#8718)
This PR was submitted for the 2.3 branch but it was merged into the master branch instead (closes symfony#8718). Discussion ---------- [Process] always manually inherit $_SERVER by kriswallsmith symfony#8067 | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ The `$_ENV` superglobal is not populated if `E` is not present in the `variables_order` directive. Since populating this variable is not recommended (for performance reasons), we should not rely on it. This change updates the builder so `$env=null` is never passed to `proc_open()`. Instead we always merge the `$_SERVER` superglobal into any environment variables that were manually set on the builder (unless inherit has been disabled). This is a copy of PR symfony#8067 by @kriswallsmith. I have created a new PR because there were some failing tests and kriswallsmith seems to have no time to rebase. Commits ------- 75be4d9 [Process] always manually inherit $_SERVER
2 parents a0a1c31 + 2baa2c6 commit e617ddb

File tree

2 files changed

+18
-59
lines changed

2 files changed

+18
-59
lines changed

‎src/Symfony/Component/Process/ProcessBuilder.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ public function getProcess()
164164
$script = implode(' ', array_map(array(__NAMESPACE__.'\\ProcessUtils', 'escapeArgument'), $arguments));
165165

166166
if ($this->inheritEnv) {
167-
$env = $this->env ? $this->env + $_ENV : null;
167+
// include $_ENV for BC purposes
168+
$env = array_replace($_ENV, $_SERVER, $this->env);
168169
} else {
169170
$env = $this->env;
170171
}

‎src/Symfony/Component/Process/Tests/ProcessBuilderTest.php

Lines changed: 16 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -17,75 +17,33 @@ class ProcessBuilderTest extends \PHPUnit_Framework_TestCase
1717
{
1818
public function testInheritEnvironmentVars()
1919
{
20-
$snapshot = $_ENV;
21-
$_ENV = $expected = array('foo' => 'bar');
20+
$_ENV['MY_VAR_1'] = 'foo';
2221

23-
$pb = new ProcessBuilder();
24-
$pb->add('foo')->inheritEnvironmentVariables();
25-
$proc = $pb->getProcess();
22+
$proc = ProcessBuilder::create()
23+
->add('foo')
24+
->getProcess();
2625

27-
$this->assertNull($proc->getEnv(), '->inheritEnvironmentVariables() copies $_ENV');
26+
unset($_ENV['MY_VAR_1']);
2827

29-
$_ENV = $snapshot;
28+
$env = $proc->getEnv();
29+
$this->assertArrayHasKey('MY_VAR_1', $env);
30+
$this->assertEquals('foo', $env['MY_VAR_1']);
3031
}
3132

3233
public function testProcessShouldInheritAndOverrideEnvironmentVars()
3334
{
34-
$snapshot = $_ENV;
35-
$_ENV = array('foo' => 'bar', 'bar' => 'baz');
36-
$expected = array('foo' => 'foo', 'bar' => 'baz');
37-
38-
$pb = new ProcessBuilder();
39-
$pb->add('foo')->inheritEnvironmentVariables()
40-
->setEnv('foo', 'foo');
41-
$proc = $pb->getProcess();
42-
43-
$this->assertEquals($expected, $proc->getEnv(), '->inheritEnvironmentVariables() copies $_ENV');
44-
45-
$_ENV = $snapshot;
46-
}
47-
48-
public function testProcessBuilderShouldNotPassEnvArrays()
49-
{
50-
$snapshot = $_ENV;
51-
$_ENV = array('a' => array('b', 'c'), 'd' => 'e', 'f' => 'g');
52-
$expected = array('d' => 'e', 'f' => 'g');
53-
54-
$pb = new ProcessBuilder();
55-
$pb->add('a')->inheritEnvironmentVariables()
56-
->setEnv('d', 'e');
57-
$proc = $pb->getProcess();
58-
59-
$this->assertEquals($expected, $proc->getEnv(), '->inheritEnvironmentVariables() removes array values from $_ENV');
60-
61-
$_ENV = $snapshot;
62-
}
63-
64-
public function testInheritEnvironmentVarsByDefault()
65-
{
66-
$pb = new ProcessBuilder();
67-
$proc = $pb->add('foo')->getProcess();
68-
69-
$this->assertNull($proc->getEnv());
70-
}
71-
72-
public function testNotReplaceExplicitlySetVars()
73-
{
74-
$snapshot = $_ENV;
75-
$_ENV = array('foo' => 'bar');
76-
$expected = array('foo' => 'baz');
35+
$_ENV['MY_VAR_1'] = 'foo';
7736

78-
$pb = new ProcessBuilder();
79-
$pb
80-
->setEnv('foo', 'baz')
81-
->inheritEnvironmentVariables()
37+
$proc = ProcessBuilder::create()
38+
->setEnv('MY_VAR_1', 'bar')
8239
->add('foo')
83-
;
84-
$proc = $pb->getProcess();
40+
->getProcess();
8541

86-
$this->assertEquals($expected, $proc->getEnv(), '->inheritEnvironmentVariables() copies $_ENV');
42+
unset($_ENV['MY_VAR_1']);
8743

88-
$_ENV = $snapshot;
44+
$env = $proc->getEnv();
45+
$this->assertArrayHasKey('MY_VAR_1', $env);
46+
$this->assertEquals('bar', $env['MY_VAR_1']);
8947
}
9048

9149
/**

0 commit comments

Comments
 (0)
0