Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added support for getenv() local_only parameter #6

Merged
merged 5 commits into from
Apr 3, 2019
Merged

added support for getenv() local_only parameter #6

merged 5 commits into from
Apr 3, 2019

Conversation

frugan-dev
Copy link
Contributor

added support for getenv() local_only parameter to return the value of locally-set environment variables

@oscarotero
Copy link
Owner

Hi.
Thanks for this. My only concern is this is a breaking change so I'd add this feature as an option. For example:

Env::$options |= Env::LOCAL_ONLY;

src/Env.php Show resolved Hide resolved
src/Env.php Show resolved Hide resolved
@@ -37,6 +38,8 @@ public static function get($name)
{
if (self::$options & self::USE_ENV_ARRAY) {
$value = isset($_ENV[$name]) ? $_ENV[$name] : false;
} elseif (self::$options & self::LOCAL_FIRST) {
$value = getenv($name, true) ?: getenv($name);
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry again. The first getenv can return something evaluated as false (like 0, "", etc). We need to check if the returned value is strictly false. In other words:

$value = getenv($name, true);

if ($value === false) {
    $value = getenv($name);
}

@oscarotero
Copy link
Owner

One doubt comes to my head: We check whether a value exists only in local, and if not, returns the value from global. Does it make sense? It's the same than not cheking at all, isn't? Could you make a test with both cases?

@frugan-dev
Copy link
Contributor Author

frugan-dev commented Apr 3, 2019

From what i read in the PHP documentation https://www.php.net/manual/en/function.getenv.php, in some environments (PHP running in a SAPI such as Fast CGI), getenv() function without $local_only parameter will always return the value of an environment variable set by the SAPI.

So if you change a variable with putenv(), env() function does not return the modified value.

For example:

// global value
echo getenv('DOCUMENT_ROOT');
// /home/user/public

// always return local value, NOT global value
echo getenv('DOCUMENT_ROOT', true);
//

putenv('DOCUMENT_ROOT=/tmp');

// always return global value, NOT local value
echo getenv('DOCUMENT_ROOT');
// /home/user/public

// local value
echo getenv('DOCUMENT_ROOT', true);
// /tmp
 

With your repo:

Env::init();

echo env('DOCUMENT_ROOT');
// /home/user/public

putenv('DOCUMENT_ROOT=/tmp');

// always return global value, NOT local value
echo env('DOCUMENT_ROOT');
// /home/user/public

With new env() function:

function env($name)
{
    if (Env::$options & Env::USE_ENV_ARRAY) {
        $value = isset($_ENV[$name]) ? $_ENV[$name] : false;
    } elseif (($value = getenv($name, true)) === false) {
        $value = getenv($name);
    }

    if ($value === false) {
        return Env::$default;
    }

    return Env::convert($value);
}

echo env('DOCUMENT_ROOT');
// /home/user/public

putenv('DOCUMENT_ROOT=/tmp');

echo env('DOCUMENT_ROOT');
// /tmp

@oscarotero
Copy link
Owner

Ok, thanks for your detailed explanation!

@oscarotero oscarotero merged commit 665ff11 into oscarotero:master Apr 3, 2019
@oscarotero
Copy link
Owner

Merged. I'll release a new version soon.
Thanks for your contribution!

@frugan-dev
Copy link
Contributor Author

Thank you! I think your observation is correct, and it would be better to change the code as you say:

$value = getenv($name, true);

if ($value === false) {
    $value = getenv($name);
}

@oscarotero
Copy link
Owner

Yes, I just did it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants