8000 [DI] ENV parameters at runtime with PHP 7 strict types not working properly · Issue #20434 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] ENV parameters at runtime with PHP 7 strict types not working properly #20434

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.

Sign up for GitHub

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

Jump to bottom
Closed
sandrokeil opened this issue Nov 7, 2016 · 10 comments
Closed

[DI] ENV parameters at runtime with PHP 7 strict types not working properly #20434

sandrokeil opened this issue Nov 7, 2016 · 10 comments
Labels
DependencyInjection Feature

Comments

@sandrokeil
Copy link
sandrokeil commented Nov 7, 2016 •
edited
Loading

I'm just playing around with the Symfony 3.2 Beta and Runtime Environment Variables.

There are two problems:

  1. It's not data type aware
  2. It's not working if environment parameter is a service name
  3. No null value support

1. It's not data type aware

All environment variables are strings e.g. if you use nginx + php-fpm. So we need a type cast on reading the environment variable. For example %env('MY_INTEGER', 'integer')% or %env('MY_BOOLEAN', 'bool')% etc. Otherwise it makes no sense, because you get errors like Invalid type for path "swiftmailer.mailers.default.disable_delivery". Expected boolean, but got string.

How can we solve this?

2. It's not working if environment parameter is a service name

If I put the mailer_transport parameter to an environment variable I get the error You have requested a non-existent service "%env(MAILER_TRANSPORT)%". This env variable has the value smtp. I guess @fabpot mentioned this here.

Would be great if we can resolve this too. Any ideas?

3. No null value support

It should be possible to allow default null values or to allow not defined environment variables which have then as default value null. The mailer_transport parameter can be null.

With $container->setParameter('env(MY_DEFAULT_NULL)', null); I get the error The default value of an env() parameter must be scalar, but "NULL" given. And if this environment variable is not defined, I get the error Environment variable not found: "MY_DEFAULT_NULL".

/cc @nicolas-grekas
@nicolas-grekas
Copy link
Member

Flagging as feature request, because the current behavior has its validity scope, and this description is about extending this validity scope.

Note that I'm not sure there is anything to do in Symfony core: the "fix" might very well be in DI extensions, when they do too much things at compile time, vs runtime. See #19843 for an example of enhanced DI extension.

@sandrokeil
Copy link
Author

Alright, I added another problem Nonullvalue support in the description.

@stof
Copy link
Member
stof commented Nov 7, 2016

Using parameters in service references is not supported today.

This means that using an env placeholder in a config setting used to set a service alias (the Swiftmailer transport config in your case) won't be supported either.
Btw, in the case of the transport, I don't think it makes sense to refactor the whole configuration to support changing the transport being used at runtime without clearing the cache.

Regarding the possibility to set null as default value for an env placeholder, this is totally valid indeed IMO.

@javiereguiluz javiereguiluz mentioned this issue Nov 7, 2016
Closed
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Nov 7, 2016 •
edited
Loading

No null value support

already supported as default value:

parameters:
    env(FOO): ~

@fabpot
Copy link
Member
fabpot commented Nov 7, 2016

That's a problem. There is a discrepancy between what you can do with a real env value (only strings) and what you can do here with a parameter.

@fabpot
Copy link
Member
fabpot commented Nov 7, 2016

So, at least for 3.2, let's throw an exception with the env var value is defined as a parameter and the value is not null or a string.

This can be then revisited in 3.3 if we start supporting conversions from strings to other types.

@sandrokeil
Copy link
Author

Thanks for clarification.

No null value support
already supported as default value:

I use a PHP configuration file and this doesn't work. I got an ~ instead of a null value for this parameter. @nicolas-grekas

$container->setParameter('MY_NOT_DEFINED', '%env(MY_NOT_DEFINED)%');
$container->setParameter('env(MY_NOT_DEFINED)', '~');

@nicolas-grekas
Copy link
Member

Did you try $container->setParameter('env(MY_NOT_DEFINED)', null);? The previous notation is yaml only.

@sandrokeil
Copy link
Author

@nicolas-grekas Yes, see 3. in the description.

With $container->setParameter('env(MY_DEFAULT_NULL)', null); I get the error The default value of an env() parameter must be scalar, but "NULL" given. And if this environment variable is not defined, I get the error Environment variable not found: "MY_DEFAULT_NULL".

@nicolas-grekas
Copy link
Member

Fixed

@andrerom andrerom mentioned this issue Feb 14, 2017
Closed
@smcjones smcjones mentioned this issue Mar 24, 2017
Closed
@nicolas-grekas nicolas-grekas mentioned this issue Aug 16, 2017
Merged
fabpot added a commit that referenced this issue Sep 7, 2017
@fabpot
feature #23901 [DI] Allow processing env vars (nicolas-grekas)
1bfa6a9 
This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Allow processing env vars

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | see description
| License       | MIT
| Doc PR        | -

This PR is an updated version of #20276 ~~(it embeds #23899 for now.)~~

It superscedes/closes:
- [DI] Add support for secrets #23621 ping @dunglas
- Runtime container parameter not found event filter #23669 ping @marfillaster
- [DependencyInjection] [DX] Support for JSON string environment variables #23823 ping @Pierstoval
- add support for composite environment variables #17689 ping @greg0ire
- [DI] ENV parameters at runtime with PHP 7 strict types not working properly #20434 ping @sandrokeil
- Issue when using a SQLite database and the DATABASE_URL env var #23527 ping @javiereguiluz

#22151 is another story, so not fixed here.

The way it works is via `%env(foo:BAR)%` prefixes, where "foo" can be bound to any services you'd like.
By default, the following prefixes are supported:
- `bool`, `int`, `float`, `string`, `base64`
- `const` (for referencing PHP constants)
- `json` (supporting only json **arrays** for type predictability)
- `file` (eg for access to secrets stored in files.)
- `resolve` (for processing parameters inside env vars.)

New prefixes can be added by implementing the new `EnvProviderInterface`, and tagging with `container.env_provider` (see `Rot13EnvProvider` in tests.)

Prefixes can be combined to chain processing, eg.
`%env(json:base64:file:FOO)%` will be roughly equivalent to
`json_decode(base64_decode(file_get_content(getenv('FOO'))))`.

Commits
-------

1f92e45 [DI] Allow processing env vars
@bpolaszek bpolaszek mentioned this issue Jan 3, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection Feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @javiereguiluz @nicolas-grekas @stof @sandrokeil
0