NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[DependencyInjection] Fix support for`false` boolean env vars#50530

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:6.3fromokhoshi:di-false-env

Jun 9, 2023

Merged

[DependencyInjection] Fix support for`false` boolean env vars#50530

nicolas-grekas merged 1 commit intosymfony:6.3fromokhoshi:di-false-env

Jun 9, 2023

Conversation

Copy link

Contributor

okhoshi commentedJun 1, 2023

Q	A
Branch?	6.3
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	-
License	MIT
Doc PR	-

After upgradingsymfony/dependency-injection package to version 6.3, some of our env vars couldn't be found anymore.

For some scripts, we are providing an adhoc default value to env vars by setting$_SERVER directly. However, since version 6.3 of the DependencyInjection component, setting an env var tofalse (the boolean value, like in the snippet below) doesn't work anymore, and Symfony reports that the variable cannot be found.

$_SERVER['FOO'] =false;

It seems to be a side effect of the changes made in#48705.

[DependencyInjection] Fix support forfalse boolean env vars

5101d18

carsonbot added Status: Needs Review Bug DependencyInjection labels

Jun 1, 2023

carsonbot added this to the6.3 milestone

Jun 1, 2023

Copy link

Member

stof commentedJun 1, 2023•
edited
Loading

environment variables are always strings though. What you are covering in tests is not something compatible with actual environment variables (or with the DotEnv component).

Copy link

ContributorAuthor

okhoshi commentedJun 1, 2023

Because you can set any scalar value from PHP code, and they won't be converted to strings nor anything will prevent you to set them in the first place. FWIW, we've never had any troubles so far, be it with Symfony or DotEnv component.

I'm covering with tests because it was a supported behaviour that has been broken with 6.3 release. If you think it shouldn't have been supported from the start, and thus that this is not a bug, I'm also fine with that, feel free to let me know and close this PR.
But it's not really consistent then since any other scalar value will work.