-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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.
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
[Process] Skip environment variables with false value in Process #25869
Conversation
@@ -331,7 +331,9 @@ public function start(callable $callback = null/*, array $env = array()*/) | |||
} else { | |||
$envPairs = array(); | |||
foreach ($env as $k => $v) { | |||
$envPairs[] = $k.'='.$v; | |||
if (is_string($v)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be more specific IMHO. Eg numbers should not be filtered. false !== ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, this is the test I did to fix the problem, but after, I saw the test in the getDefaultEnv()
method. I also prefer to use false !==
because it is more specific. I change that immediately.
@nicolas-grekas It's updated. |
Thank you @francoispluchino. |
…rocess (francoispluchino) This PR was merged into the 3.3 branch. Discussion ---------- [Process] Skip environment variables with false value in Process | Q | A | ------------- | --- | Branch? | master, 4.0, 3.3, 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ With the commit 03adce2, all env variables are injecting in the process, and the env variable with the `false` value are converted in empty string. For example, it's problematic with the bundle SymfonyWebServerBundle and the last version of the [symfony/framework-bundle recipe](https://github.com/symfony/recipes/blob/master/symfony/framework-bundle/3.3/public/index.php#L11), because the WebServer override the value of `APP_ENV` with `false` to override previously loaded variables (c5a1218), and with the commit 03adce2, the `APP_ENV` variable is injected with a empty string value, instead of not being injected, as was the case before. This PR not use the same logic as the [getDefaultEnv()](https://github.com/symfony/symfony/blob/4.0/src/Symfony/Component/Process/Process.php#L1553) method. Commits ------- 2daf4f9 [Process] Skip environment variables with false value in Process
@nicolas-grekas It's always a pleasure to help. |
With the commit 03adce2, all env variables are injecting in the process, and the env variable with the
false
value are converted in empty string.For example, it's problematic with the bundle SymfonyWebServerBundle and the last version of the symfony/framework-bundle recipe, because the WebServer override the value of
APP_ENV
withfalse
to override previously loaded variables (c5a1218), and with the commit 03adce2, theAPP_ENV
variable is injected with a empty string value, instead of not being injected, as was the case before.This PR not use the same logic as the getDefaultEnv() method.