Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[DependencyInjection] Stop considering empty env vars as populated#48705

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

Merged
fabpot merged 1 commit intosymfony:6.3fromnicolas-grekas:di-empty-env
Dec 28, 2022

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedDec 19, 2022•
edited
Loading

QA
Branch?6.3
Bug fix?no
New feature?no (DX improvement)
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

When a.env file declares an empty var (egFOO=), env-var loaders are skipped.
This is a foot gun that has hit me many times when using secrets and that I consider a DX bug.
This PR fixes the issue by ensuring that env vars defined as empty strings don't prevent loaders from being called.

@carsonbotcarsonbot added this to the5.4 milestoneDec 19, 2022
@nicolas-grekasnicolas-grekas changed the title[FrameworkBundle] fix: fix typo about help[DependencyInjection] Fix considering empty env vars as populatedDec 19, 2022
@fabpotfabpot modified the milestones:5.4,6.3Dec 27, 2022
@nicolas-grekasnicolas-grekas added DXDX = Developer eXperience (anything that improves the experience of using Symfony) and removed Bug labelsDec 27, 2022
@nicolas-grekasnicolas-grekas changed the base branch from5.4 to6.3December 28, 2022 11:32
@nicolas-grekasnicolas-grekas changed the title[DependencyInjection] Fix considering empty env vars as populated[DependencyInjection] Stop considering empty env vars as populatedDec 28, 2022
@nicolas-grekas
Copy link
MemberAuthor

PR rebased on 6.3

@fabpot
Copy link
Member

Thank you@nicolas-grekas.

@PhilETaylor
Copy link
Contributor

PhilETaylor commentedApr 20, 2023•
edited
Loading

Just a question on this.

Prior to 6.3 it was common to have things like

###> sentry/sentry-symfony ####SENTRY_DSN="https://<redacted>@o96822.ingest.sentry.io/< redacted>"SENTRY_DSN=""###< sentry/sentry-symfony ###

in the.env during development, to quickly toggle between something being enabled or disabled. This is then references in config/sentry.php (just as an example)

returnstaticfunction (ContainerConfigurator$containerConfigurator):void {$containerConfigurator->extension('sentry',        ['dsn' =>env('SENTRY_DSN'),        ]    );};

or in the.env.test.* files to have empty, but defined env vars so that the app can load, but those api's being untested did not need a valid string.

twitter_consumer_key=twitter_consumer_secret=twitter_access_token=twitter_access_token_secret=

Is it the intention of this PR to introduce a b/c break where this would work in 6.2, but after upgrading to Symfony 6.3, this would now throw anthrow new EnvNotFoundException(sprintf('Environment variable not found: "%s".', $name)); exception even though the env var is still there, its just an empty string?

If it was your intention for this to not be backward compatible, then cool - it worked :)

Just trying to work out if it's on purpose or a bug introduced.

@nicolas-grekas
Copy link
MemberAuthor

Did you experience this exception or are you just wondering? If you're wondering, can you give it a try?

@PhilETaylor
Copy link
Contributor

I've experienced it on multiple projects upgrading to 6.3.x-dev sorry

Will dig deeper tomorrow and find a minimal reproducer for you

@PhilETaylor
Copy link
Contributor

As promised, a scaled down reproducer is detailed here for you#50094

nicolas-grekas added a commit that referenced this pull requestMay 12, 2023
…:list (nicolas-grekas)This PR was merged into the 6.3 branch.Discussion----------[FrameworkBundle] Ignore vars from dotenv files in secrets:list| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Sidekick of#48705Commits-------8561045 [FrameworkBundle] Ignore vars from dotenv files in secrets:list
nicolas-grekas added a commit that referenced this pull requestJun 9, 2023
…vars (Okhoshi)This PR was merged into the 6.3 branch.Discussion----------[DependencyInjection] Fix support for `false` boolean env vars| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -After upgrading `symfony/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 to `false` (the boolean value, like in the snippet below) doesn't work anymore, and Symfony reports that the variable cannot be found.```php$_SERVER['FOO'] = false;```It seems to be a side effect of the changes made in#48705.Commits-------5101d18 [DependencyInjection] Fix support for `false` boolean env vars
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@ycerutoycerutoyceruto approved these changes

@chalasrchalasrchalasr approved these changes

@lyrixxlyrixxAwaiting requested review from lyrixx

@wouterjwouterjAwaiting requested review from wouterj

@dunglasdunglasAwaiting requested review from dunglas

@OskarStarkOskarStarkAwaiting requested review from OskarStark

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees

No one assigned

Labels

DependencyInjectionDXDX = Developer eXperience (anything that improves the experience of using Symfony)Status: Needs Review

Projects

None yet

Milestone

6.3

Development

Successfully merging this pull request may close these issues.

6 participants

@nicolas-grekas@fabpot@PhilETaylor@yceruto@chalasr@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp