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] Fix autocasting null env values to empty string#50837

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

Conversation

@fancyweb
Copy link
Contributor

@fancywebfancyweb commentedJun 30, 2023
edited
Loading

QA
Branch?5.4
Bug fix?yes
New feature?-
Deprecations?-
Tickets#50815 (comment)
LicenseMIT
Doc PR-

This PR fixes autocastingnull env values to''.
We continue to autocast all other values to string when there's no prefix.

It doesn't revert#50517, it just fixes an unintended side effect.

It doesn'tfix#50815 request, it confirms the behavior: the solution for this issue would be to remove thestring: prefix.

There's still no solution to have the "keep null as null" and "cast if not null" behavior.

@carsonbotcarsonbot added this to the5.4 milestoneJun 30, 2023
@fancywebfancywebforce-pushed thedi/fix-autocast-null-env-to-empty-string branch from3b996e9 to7fd7de2CompareJune 30, 2023 16:28
@fancywebfancywebforce-pushed thedi/fix-autocast-null-env-to-empty-string branch 2 times, most recently from764d623 to07f00e1CompareJune 30, 2023 16:42
}

$returnNull =false;
if ('' ===$prefix) {
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

'' becomes a magic value that acts as thestring prefix except for null where we return null

@fancywebfancywebforce-pushed thedi/fix-autocast-null-env-to-empty-string branch 2 times, most recently fromd42f633 to024526aCompareJune 30, 2023 17:37
@nicolas-grekasnicolas-grekasforce-pushed thedi/fix-autocast-null-env-to-empty-string branch from024526a to2c0815dCompareJuly 3, 2023 12:55
@nicolas-grekas
Copy link
Member

Thank you@fancyweb.

@nicolas-grekasnicolas-grekas merged commit1e1d91e intosymfony:5.4Jul 3, 2023
@fancywebfancyweb deleted the di/fix-autocast-null-env-to-empty-string branchJuly 3, 2023 13:00
@melkamar
Copy link

melkamar commentedJul 12, 2023
edited
Loading

I may have shot myself in the foot with#50415 😁 I have now upgraded to a version including this fix and am having issues with Swiftmailer configuration:

swiftmailer:encryption:'%env(string:default::MAILER_ENCRYPTION)%'

With this andMAILER_ENCRYPTION unset, the parameter now becomes an empty string ``. However, the Swiftmailer library does not like that as it only handlesnull in a special way, not an empty string, and throws

  [InvalidArgumentException]  The  encryption is not supported

When I change the definition of the parameter to

swiftmailer:encryption:'%env(default::MAILER_ENCRYPTION)%'

which, as I understand this PR, should do what I expect it to (passnull asnull), the container build crashes on type mismatch:

  [Symfony\Component\Config\Definition\Exception\InvalidTypeException]  Invalid type for path "swiftmailer.mailers.default.encryption". Expected one of "bool", "int", "float", "string", but got one of "bool", "int", "float", "string", "array".

This is because the SwiftMailerconfiguration expects a scalar node but the result of the env-var-processing may be an array (though not in my particular case, I think).

This seems to be the case that you wrote about in the PR description

There's still no solution to have the "keep null as null" and "cast if not null" behavior.

It feels like there is a disconnect between what's possible with the parameter/yaml based configuration and what's possible to do with environment variables. One solution to the problem above would be to have a "nullable" set of prefixes, but I don't know if that's making things even more complicated.


Edit: this seems to do the trick in our code, at least:

swiftmailer:encryption:'%env(nullablestring:default::MAILER_ENCRYPTION)%'
<?phpuseSymfony\Component\DependencyInjection\EnvVarProcessorInterface;useSymfony\Component\DependencyInjection\Exception\RuntimeException;finalclass NullableStringEnvProcessorimplements EnvVarProcessorInterface{/**     * {@inheritDoc}     */publicfunctiongetEnv(string$prefix,string$name,\Closure$getEnv)    {if ($prefix ==='nullablestring') {$val =$getEnv($name);if ($val ===null) {returnnull;            }return (string)$val;        }thrownewRuntimeException(\sprintf('Unsupported env var prefix "%s" for env name "%s".',$prefix,$name));    }/**     * {@inheritDoc}     */publicstaticfunctiongetProvidedTypes()    {return ['nullablestring' =>'string',        ];    }}

@fancyweb
Copy link
ContributorAuthor

Could you open a new issue with a reproducer and a full stack trace plz?

This was referencedJul 29, 2023
}
$processor =$processors->has($prefix) ?$processors->get($prefix) :newEnvVarProcessor($this);

if ($processors->has($prefix)) {
Copy link
Contributor

@bendaviesbendaviesJul 31, 2023
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

this doesn't work as$prefix is always set tostring on 390.

fancyweb reacted with eyes emoji
@bendavies
Copy link
Contributor

confirmed that this fix doesn't work unfortunately, empty stings instead of nulls are still passed to services.
problem noted above.

@fancyweb
Copy link
ContributorAuthor

I tested it and indeed it doesn't work in the full framework context. I'll fix it again, for the last time I hope 😓

nicolas-grekas added a commit that referenced this pull requestSep 20, 2023
… empty string with `container.env_var_processors_locator` (fancyweb)This PR was merged into the 5.4 branch.Discussion----------[DependencyInjection] Fix autocasting `null` env values to empty string with `container.env_var_processors_locator`| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |#50837 (comment)| License       | MIT| Doc PR        | noThe previous fix doesn't work when `$processors` comes from `container.env_var_processors_locator`.Commits-------766bc6e [DependencyInjection] Fix autocasting null env values to empty string with container.env_var_processors_locator
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@bendaviesbendaviesbendavies left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

5 participants

@fancyweb@nicolas-grekas@melkamar@bendavies@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp