Overriding Apache env variables #39

Closed
levacic opened this Issue Oct 17, 2014 · 5 comments

Projects

None yet

4 participants

@levacic
levacic commented Oct 17, 2014

So some time ago I had a problem where a certain environment variable was configured in Apache's <VirtualHost> block, and phpdotenv wasn't overwriting that value with what was configured in the .env file.

I use getenv() calls in my config files instead of $_ENV to simplify handling non-configured values - otherwise I'd have to do something like array_key_exists($_ENV, 'key') or something, which is too verbose with no benefit.

The problem with that is, when you have, for example SetEnv APP_ENV production in Apache's <VirtualHost> block, then the code

putenv('APP_ENV=staging');
echo getenv('APP_ENV');

will output production.

To successfully override this, PHP's apache_setenv function must be called instead, like this:

if (function_exists('apache_setenv'))
{
    apache_setenv('APP_ENV', 'staging');
}

The function_exists() check must be present so as not to cause errors when the user is not running Apache.

Now of course, this depends on a higher-level decision of whether an Apache-configured environment variable should take precedence over the one specified in a project's .env file - and to me it's kind of obvious that it shouldn't, since an .env file is a more granular configuration method, and as such, it should take precedence, but of course I may be wrong about that.

Do you think this could/should be included in phpdotenv?


Also, as an unrelated question (and sorry for hijacking this issue, I just want to avoid spamming issues), is there any reason why you chose to use this specific syntax for .env files, other than being similar to Ruby's dotenv syntax? Just plain requiring PHP files (as e.g. Laravel < 5 does) seems to be a much more straightforward approach, which avoids the various issues related to actually parsing the .env files, including comments etc. and additionally simplifies generating them, as you can simply var_export() an array and be done with it - with the current syntax, it doesn't seem to be as trivial, as one must be careful to correctly escape stuff since the syntax isn't really documented completely (e.g. there's nothing in the docs to explain what to do if I want the literal {$BASE_DIR}/tmp as a variable value, without resolving {$BASE_DIR} to something else). I'm interested in whether you feel there is any advantage to the current approach over simply doing $variables = require '/path/to/project/.env.php'; - thanks in advance for your thoughts on this, and sorry for the longish-double-issue.

Thanks for your work, cheers!

@vlucas
Owner
vlucas commented Oct 17, 2014

The primary advantage with the current approach is that it makes the variables more easily exportable to shell variables.

export APP_ENV='staging'
export DATABASE_URL='sqlite::memory::'

Allows you to do this in the local shell:

eval $(cat .env)

Of course if you use comments and other things, then it might not be so straightforward.

@vlucas
Owner
vlucas commented Oct 17, 2014

That said, I definitely feel weird about a bunch of special parsing rules, and I do think this project would benefit from the option of a PHP file include as well.

@levacic
levacic commented Oct 17, 2014

Ah, didn't even know I could do that - it isn't mentioned anywhere in the docs, and I just dug through the source code right now to verify it. I personally don't see a use case for that - but to be complete, if your file was:

<?php

return [
    'APP_ENV' => 'staging',
    'DATABASE_URL' => 'sqlite::memory::'
];

you could do something like:

eval $(php -r 'foreach (require ".env" as $key => $value) { echo "export {$key}={$value}\n"; }')

Now, I wouldn't ever do this, and it is a bit ugly, and far more complicated than eval $(cat .env), but it's just a proof of concept that a one-liner is possible here as well, if you need it for some deployment process or something - additionally, it doesn't even matter if you have comments or whatever, since PHP already knows how to parse PHP files, so we don't need to write our own parser as we do for the current syntax.

So anyway, you'd be willing to accept a PR that allows PHP files to be used as well? This wouldn't be too complicated actually - we could refactor these lines into a few methods with some detection logic for whether we have a PHP file or not (for example by checking if it starts with <?php, since a valid .env file, according to the current syntax, can't start with that - or just checking if the passed configuration filename ends with .php), and using the appropriate "loading" method afterwards - and this would be completely backwards compatible as well.

Thanks for your input!

P.S. I now realize my mistake when creating this issue and writing about different things, and I'll make sure not to do that again - but what do you think about the apache_setenv() addition - would that make sense? I'm happy to make both PRs, if you think they'd be welcome.

@kelvinj
Contributor
kelvinj commented Oct 21, 2014

@levacic @vlucas

Running source .env should load the environment variables into the shell as well.

If you really want to use PHP, then you can avoid using load() and instead call Dotenv::setEnvironmentVariable() in a PHP file.

Re: the apache bug – I never realised that existed… interesting. I think it'd be difficult to get around it with a solution that doesn't then become confusing. Could it have weird knock-on consequences for Apache?

Definitely think it's worth documenting though, that could catch some people out.

Also, you can use Dotenv::findEnvironmentVariable() to get an environment variable, and it checks the $_ENV first. If that's too verbose, you could create your own wrapper function, e.g. function findenv()

@GabeMedrash
Contributor

Forgive me for reviving a long-dormant issue, but the original issue raised by @levacic regarding Apache env variables is unresolved. Considering the docs claim that Dotenv::overload overrides existing environment variables, I had thought this issue had been solved. The problem, however, is reproducible just as @levacic noted:

  1. SetEnv FOO bar in Apache configuration
  2. Set FOO not_bar in my project's .env
  3. Use $dotenv->overload();
  4. echo getenv(FOO) => 'bar' (I expect--and want--'not_bar'!)

The solution, as noted, is to check if the apache_setenv function exists (and, if you want, check if apache_getenv returns truthy), then use apache_setenv.

Is there a reason why you wouldn't want to support this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment