[DependencyInjection] Fix autocasting null env values to empty string #50837
Conversation
3b996e9 to
7fd7de2
Compare
764d623 to
07f00e1
Compare
| @@ -126,6 +126,12 @@ public function getEnv(string $prefix, string $name, \Closure $getEnv) | |||
| } | |||
| } | |||
|
|
|||
| $returnNull = false; | |||
| if ('' === $prefix) { | |||
There was a problem hiding this comment.
'' becomes a magic value that acts as the string prefix except for null where we return null
d42f633 to
024526a
Compare
024526a to
2c0815d
Compare
|
Thank you @fancyweb. |
|
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 and 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 (pass This is because the SwiftMailer configuration 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
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)%'<?php
use Symfony\Component\DependencyInjection\EnvVarProcessorInterface;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
final class NullableStringEnvProcessor implements EnvVarProcessorInterface
{
/**
* {@inheritDoc}
*/
public function getEnv(string $prefix, string $name, \Closure $getEnv)
{
if ($prefix === 'nullablestring') {
$val = $getEnv($name);
if ($val === null) {
return null;
}
return (string) $val;
}
throw new RuntimeException(\sprintf('Unsupported env var prefix "%s" for env name "%s".', $prefix, $name));
}
/**
* {@inheritDoc}
*/
public static function getProvidedTypes()
{
return [
'nullablestring' => 'string',
];
}
} |
|
Could you open a new issue with a reproducer and a full stack trace plz? |
| @@ -390,7 +390,15 @@ protected function getEnv(string $name) | |||
| $prefix = 'string'; | |||
| $localName = $name; | |||
| } | |||
| $processor = $processors->has($prefix) ? $processors->get($prefix) : new EnvVarProcessor($this); | |||
|
|
|||
| if ($processors->has($prefix)) { | |||
There was a problem hiding this comment.
this doesn't work as $prefix is always set to string on 390.
|
confirmed that this fix doesn't work unfortunately, empty stings instead of nulls are still passed to services. |
|
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 😓 |
… 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 | no The 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
This PR fixes autocasting
nullenv 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't fix #50815 request, it confirms the behavior: the solution for this issue would be to remove the
string:prefix.There's still no solution to have the "keep null as null" and "cast if not null" behavior.