Symfony: [RFC][Dotenv] Add a convenient way to retrieve env vars

Created on 5 Jan 2018 · 53Comments · Source: symfony/symfony

| Q | A
| ---------------- | -----
| Bug report? | no
| Feature request? | yes
| BC Break report? | no
| RFC? | yes
| Symfony version | 3.x, 4.x

We can access env vars in several ways, and the Dotenv class is already capable of handling env vars in all possible ways:

https://github.com/symfony/symfony/blob/f95ac4f8090f12283ef4efcccb2e05d2fef87bea/src/Symfony/Component/Dotenv/Dotenv.php#L72-L77

There are also some cases where the current Flex recipes are using $_SERVER and it seems it breaks and should use $_ENV.
A nice example is the symfony/recipes#331 issue: the APP_ENV env var is created before the entrypoint is executed, and PHP doesn't set it in $_SERVER but sets it in $_ENV.

It's possible that it's a PHP issue, but I think we can handle this right in DotEnv, and possibly the same way it handles checks in the populate() method.

Proposal

I propose to add Dotenv::get($name) and Dotenv::exists($name) methods that would retrieve env vars based on their existence in these cases and this order:

First in $_ENV
Then in $_SERVER
And fallback to getenv() even if it's not thread-safe

This way, we could use this in our front-controllers, either console or web, and we would never expect any edge case from this.

This would mean that Dotenv would be required in dev & prod, and not only in dev, but then the APP_ENV check could be reduced to this:

if (!Dotenv::exists('APP_ENV')) {
    (new Dotenv())->load(__DIR__.'/../.env');
}

WDYT?

Dotenv Feature

Source

Pierstoval

👍6

Most helpful comment

Don't be, this is just code. Symfony is not exclusive to any non-Symfony packages, quite the contrary. If someone wants to use or create one that provides these helpers, that's perfectly fine!
And in the context of Symfony full stack, the DI configuration already allows accessing env vars so this wouldn't be used nor recommended.
But I'm going to step down from this issue, I'm exposing myself too much to emotional comments by opposing. My arguments are written, I'll let others chime in now. I'm fine with the final decision whatever it is. What matters is having a discussion.

nicolas-grekas on 25 Sep 2018

👍8 ❤1

All 53 comments

I am not sure this would really solve such issues. Because this would mean that you would always have to install the Dotenv component. Otherwise, your front controller's code would lead to errors in environments where you set the env vars through your vhost settings or anywhere else outside .env files.

xabbuh on 5 Jan 2018

👍3

same as @xabbuh
just use "%env(FOO)%" and free your services from knowing anything about where their configuration comes from.
bootstraping files are very different and need to be standalone.

nicolas-grekas on 5 Jan 2018

👍3

I updated my proposal because you're right about dev&prod and env vars elsewhere: that's why a Dotenv::exists would also have to be mandatory

Pierstoval on 5 Jan 2018

@nicolas-grekas It's not only about the DI component and the services, it's also about setting up the project in the front controllers.

Today, environment is based on APP_ENV, which is absolutely mandatory if you want a working app. And this var cannot be solved by the DI, so we need something to make it available clearly and avoid PHP flaws that set the env vars in different places and make getenv not safe.

I don't think having this is convenient in our front controllers:

if (!isset($_SERVER['APP_ENV'], $_ENV['APP_ENV']) && false === getenv('APP_ENV')) {
    // Load .env
}

$env = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? getenv('APP_ENV');

Pierstoval on 5 Jan 2018

👍3

Ah yes, situation regarding $_SERVER/$_ENV/getenv()/putenv() is a mess. There were already ton of bugs based on confusion between these and more will come.

https://github.com/symfony/symfony/pull/25559
https://github.com/symfony/symfony/pull/25523
https://github.com/symfony/symfony/pull/25162
https://github.com/symfony/symfony/pull/24686
https://github.com/symfony/symfony/pull/24113
https://github.com/symfony/symfony/pull/23949
https://github.com/symfony/symfony/pull/23899

That said, @nicolas-grekas suggeestion to avoid using it directly and just retrieve it via DI is good one. Your code example isn't so bad either. Come on, it's just one place, isn't it? If you have a project with multiple front controllers, just create simple function for it.

However, simple composer package for just doing that would be handy. It could be used in all symfony components as an abstraction layer for reading/writing env vars to avoid problems such as the ones I linked.

ostrolucky on 6 Jan 2018

We could get inspiration from components like oscarotero/env or vlucas/phpdotenv. This is "just another env library" like it is right now.

Maybe another Env component can be created. Then, Dotenv would require it, but all other Symfony components would possibly require it too if they load env vars.

Like @ostrolucky linked, there are a lot of issues regarding environment vars. And there also are issues for external components (like the front controllers and console, or maybe CLI applications using the Console component, like Composer or lots of others).

Pierstoval on 7 Jan 2018

👍1

This would add another dependency to your app for something that is better done via parameters.
:-1: on my side.
At last, inject parameter_bag if you want.

nicolas-grekas on 14 Mar 2018

The problem is that the parameters part is only for the DI (even though DI is a _huge_ part of the framework). Improving the Dotenv component might make sure we don't have issues like the ones recently spotted in the framework's recipes.

There's also some dichotomy depending on whether you're using php in command-line, with apache + mod_php, with nginx + php-fpm...

Considering the high diversity of projects (docker, dedicated, PaaS), and the fact that PHP doesn't really have a stable environment vars management, I still think we should consider giving Symfony devs a proper way of retrieving env vars instead of relying on hacks like in the front controller or in the console file 🤔

Pierstoval on 14 Mar 2018

The front controller is very special as it is almost "naked", we don't have nor want any kind of dependencies there. The Dotenv component is not a service, it's only for bootstrapping, and we don't want to make it a service. Then, in your own code in your apps, you have DI at hand.

nicolas-grekas on 14 Mar 2018

So we don't take in account any project not under Flex & FrameworkBundle?

What about the console? Does the console really need the DI if it wants to use env vars?

Pierstoval on 14 Mar 2018

use $_SERVER['FOO'], or services if you have the container somehow

nicolas-grekas on 14 Mar 2018

👎1

@nicolas-grekas We're encountering issues related to $_SERVER / $_ENV. We're running Docker container based on php:7.2-apache image with default configuration and passing env variables through environment option in docker-compose.yml.

Our entry script does not see APP_ENV within $_SERVER, but it's populated into $_ENV.

Wirone on 11 Jun 2018

👍1

I disagree on this. Dotenv is a dev tool for people that doesn't know how to use the env. vars, If you use it everywhere becomes a production tool and I don't think we need this tbh.
Maybe this proposal should go to PHP to request update/deprecate _SERVER and/or _ENV or do something about getenv/setenv to make it easier to use/standard.

Saphyel on 11 Jun 2018

👎1

It's the same with us. We are using Docker and set enviroment variables to configure application on different environment(devs/prod). And there is inconsistency in Symfony libraries in some cases you are using getenv but in some it is removed because of thread safety issues. But this is simple if you want to be thread safety use $_SERVER to configure your application, why remove getenv from Dotenv.php:

// don't check existence with getenv() because of thread safety issues
            if (!isset($loadedVars[$name]) && (isset($_ENV[$name]) || (isset($_SERVER[$name]) && $notHttpName))) {

but it is still used in EnvVarProcessor:

} elseif (isset($_ENV[$name])) {
            $env = $_ENV[$name];
        } elseif (isset($_SERVER[$name]) && 0 !== strpos($name, 'HTTP_')) {
            $env = $_SERVER[$name];
        } elseif (false === ($env = getenv($name)) || null === $env) { // null is a possible value because of thread safety issue

and there is only $_SERVER in index.php:

$env = $_SERVER['APP_ENV'] ?? 'dev';
$debug = $_SERVER['APP_DEBUG'] ?? ('prod' !== $env);

fugi on 27 Jul 2018

As you can see, the EnvVarProcessor usage is only a fallback if $_ENV and $_SERVER are not set, but in most cases, they _should_ be set, even though it's not sure depending on your software architecture.

I still think that having a proper way to retrieve & check existence for env vars would be useful for projects not using the DI component (especially third-party ones, other frameworks or CMS built with some Symfony components, etc.)

Pierstoval on 27 Jul 2018

Yeah, I totally agree. Like you wrote, it should be:

First in $_ENV
Then in $_SERVER
And fallback to getenv() even if it's not thread-safe

Why omitting fallback to getenv? In Docker, by default, variables are passed by environment variables. If you want to pass it to PHP script you need to write additional ENTRYPOINT script and change Apache vhost config to set $_SERVER.

fugi on 30 Jul 2018

I found another use-case where env can cause problems.

When using Symfony/Panther, it will start a PHP server by using Symfony/Process. By default, it sends current's process environment vars to the process, but the PHP built-in server sets $_ENV but not $_SERVER, which makes it load default .env file.

Related issue: symfony/panther#68

The only solution is to make index.php fall back on $_ENV, so either we continue add (or propose to add) a certain amount of dirty code in our front controller just to fit to the problems created by PHP's native inconsistencies, or we add much prettier Env::has() and Env::get() or any similar feature to the framework... And I'd be happy to do it if the core team agrees.

Pierstoval on 12 Sep 2018

I agree with @Pierstoval , index.php cannot be based on $_SERVER only, we have to consider $_ENV at least. I also described a problem in https://github.com/symfony/recipes/pull/461#issuecomment-421254110 - if you set up some env vars and run built-in Symfony web server - index.php does not have env vars in $_SERVER, but has in $_ENV. So if we made it possible to consider both $_SERVER and $_ENV in index.php it would be possible to have Symfony web server registered in bundles.php for dev mode only but load the website in any mode you want: prod, test, etc. - that would be cool and easy! I won't need to create a additional front controllers then like index_prod.php, index_test.php, etc. and hardcoding APP_ENV.

And now the question is how to make it possible to consider env vars in both $_SERVER and $_ENV in index.php? I agree with @nicolas-grekas that index.php file should be thin and with minimum deps, but this way we'll have long conditions like:

if (!isset($_ENV['APP_ENV']) || !isset($_SERVER['APP_ENV'])) {
    // load .env file
}

$env = $_SERVER['APP_ENV'] ?? ($_ENV['APP_ENV'] ?? 'dev');

And that's only if we do not want to consider getenv(). So probably having Env::has() and Env::get() would help here and make code more readable at least. I'm not sure if %env(APP_ENV)% reads both $_SERVER and $_ENV to provide a fallback, but if it does - we can keep in sync %env()% and Env::get() then to be consistent.

bocharsky-bw on 15 Sep 2018

👍1

I'm not sure if %env(APP_ENV)% reads both $_SERVER and $_ENV to provide a fallback, but if it does - we can keep in sync %env()% and Env::get() then to be consistent.

Yes, the DI component takes this in account, but only the DI component.

Pierstoval on 15 Sep 2018

👍1

Great, so what about to take out that logic into a separate class e.g. Env as you suggested. Then we can reuse this Env in both places: for %env(APP_ENV)% and in index.php at least, i.e. share the same logic. Devs can still use low level $_SERVER/$_ENV in some cases if needed, but using Env::get() would have more sense for me, as we can see we should take into account both $_SERVER/$_ENV. So I like this idea.

bocharsky-bw on 15 Sep 2018

👍4

Would be good if Env::get() could cast results, using the same logic as EnvVarProcessor uses.

So with signature like:

class Env {
    public function get(string $name, string $type = 'string', $defaultValue = null) {
    }
}

we could achieve many useful things, like retrieving boolean configuration flags with true as default value - simple getenv requires "complex" conditions for checking if variable is set, casting it to bool with filter_var and so on, which is useless noise in the code since it could be reusable.

// Examples
Env::get('FORCE_SSL', 'bool', true);
Env::get('ALLOWED_REMOTE_IPS', 'json', []); // when you want to resctrict usage directly in front controller

As default, Env::get() would act like simple string-based read-as-is common way for accessing environment variables.

Wirone on 24 Sep 2018

@Wirone your proposal is interesting, but I would even go further by adding the complete EnvVarProcessor feature so we could do something like Env::get('json:base64:file:CREDENTIALS_FILE'), but that's maybe for another time. First, I'd like this RFC to be approved before moving to further ideas, which is not the case now :)

Pierstoval on 24 Sep 2018

@Pierstoval yeah, that approach also would be really helpful. Also it would be more consistent with DI. I'm :+1: for it (but of course with support for default value).

Wirone on 24 Sep 2018

Env::get('json:base64:file:CREDENTIALS_FILE')

that's not proper OOP: env processors are services. But making this work with static methods means they should access some global state. Instead, you should decouple your code and not care where the injected value comes from - env or not. That's what we use DI for.

Still :-1: on my side.