Replaced getenv/putenv with $_ENV and unset

[DracoBlue]

The current version of google auth library php uses getenv/putenv to read and set environment variables in php. But the function putenv is not threadsafe, which made libraries (Dotenv https://github.com/symfony/dotenv/blob/6.2/Dotenv.php#L61 and phpdotenv https://github.com/vlucas/phpdotenv#putenv-and-getenv) to disable putenv support by default. Frameworks like symfony (deprecated in 4.3 symfony/symfony#31062 and removed in 5.0) and also in laravel there was a similiar solution to not use it anymore directly (vlucas/phpdotenv#76).

Thus here is a pull request to change all the ocurences of:

• getenv($variable) to array_key_exists($variable, $_ENV) ? $_ENV[$variable] : false
• putenv($variable) to unset($_ENV[$variable])
• putenv($variable . "=") to $_ENV[$variable]=''
• putenv($variable . "=" . $value) to $_ENV[$variable]=$value

Threads about this:

• https://bugs.php.net/bug.php?id=74143 (putenv does not work if fast cgi sets the variable!)
• [Dotenv] Deprecate useage of "putenv" symfony/symfony#31062
• Changing process environment unsafe on multithreaded servers vlucas/phpdotenv#76
• .env issue with PHP thread-safe laravel/framework#7354
• laravel/framework@866a8cf
• BUG Replace phpdotenv with thread-safe replacement silverstripe/silverstripe-framework#7484
• Laravel 5.8 update breaks third-party composer libraries laravel/framework#27949

[conventional-commit-lint-gcf]

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

[vishwarajanand]

@DracoBlue thanks for making this PR. I want to check once whether there was any reason for using getenv/putenv across all libraries.
If you can, pls fix the commit message according to conventional commit guidelines.

[DracoBlue]

Done!

[bshaffer]

@DracoBlue Sorry for the confusing bot message, your previous PR title was correct, and it was just giving a warning to us (the administrators) in case we merged with the commit message instead of the PR title, which was in the incorrect format. I fixed this in #430, so this message will no longer happen.

As for this PR, unfortunately, the changes here break existing implementations. If someone is setting their credentials using putenv (as we've instructed them to do), this library would no longer work as expected. This is unacceptable.
I can see three options:

1. Wait to make these changes in a v2 version
2. Check getenv first and _ENV second. (This seems like it would cause even more issues)
3. Provide a way (similar to symphony's usePutenv function, perhaps even use an environment variable as a flag 😛) to use the threadsafe version, e.g.:

$_ENV['GOOGLE_AUTH_USE_PUTENV']=false
$_ENV['GOOGLE_APPLICATION_CREDENTIALS'] = '/path/to/credentials.json'';

There are some other implications of switching from getenv/putenv that aren't discussed in this PR, along with the implications of these functions not being threadsafe. Do you have any links to discussions of the implications?

[DracoBlue]

Thanks for your reply! You are right.

There are some other implications of switching from getenv/putenv that aren't discussed in this PR, along with the implications of these functions not being threadsafe. Do you have any links to discussions of the implications?

I added some more threads related to this to the initial post.

Check getenv first and _ENV second. (This seems like it would cause even more issues)

I think so, too.

Provide a way (similar to symphony's usePutenv function, perhaps even use an environment variable as a flag 😛) to use the threadsafe version, e.g.:

I guess for this to be interesting enough to implement, I would suggest I am supplying a test case (I don't have a proper one, yet!) which suffers from the getenv/putenv threadsafety issue - and then we can think about a way to make it possible for those using the frameworks possible to supply the credentials variable in a safe way.

[bshaffer]

I've moved this into a feature request here: #432

supplying a test case... which suffers from the getenv/putenv threadsafety issue

Yes! If you have one, or find one, please update the above issue!

I'm closing this for now since we will not be using this implementation.