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

Env vars with non-ASCII values are mangled when read from $_SERVER on Windows #7896

Closed
Seldaek opened this issue Jan 6, 2022 · 7 comments
Closed

Comments

@Seldaek
Copy link

Seldaek commented Jan 6, 2022

Description

The following code:

<?php
var_dump(
    $_SERVER['FOO'] ?? 'unset', 
    $_ENV['FOO'] ?? 'unset', 
    getenv('FOO')
);

When run with variables_order = "EGPCS" and FOO=GüИter传 set in the environment variables in Windows (or using $env:FOO = 'GüИter传' in PowerShell before executing the test file above).

Resulted in this output:

string(7) "G?ter?"
string(7) "G?ter?"
string(11) "GüИter传"

But I expected this output instead:

string(11) "GüИter传"
string(11) "GüИter传"
string(11) "GüИter传"

Doing the same in WSL/Ubuntu seems to yield the correct result, so I assume it's Windows specific.

Regarding PHP version, it appears all versions <7.2 fail completely, with even getenv() returning string(7) "G?ter?" or string(7) "Gⁿ?ter?" for 5.x. From PHP 7.2 onwards I get the behavior described above.

PHP Version

8.0.13

Operating System

Windows 10

@cmb69
Copy link
Contributor

cmb69 commented Jan 6, 2022

From PHP 7.2 onwards I get the behavior described above.

This is likely due to fixing https://bugs.php.net/75574, so should be fixed as of PHP 7.1.13 and 7.2.1, respectively. That fix only affected getenv(), though, but not _php_import_environment_variables (and possibly other places).

@Seldaek
Copy link
Author

Seldaek commented Jan 6, 2022

Thanks, do you see any way to mitigate/detect this in userland?

I see that doing this when starting up to ensure $_SERVER is in a good state appears to work.. not sure if there's a better way:

foreach ($_SERVER as $var => $val) {
    if (($val = getenv($var)) !== false) {
        $_SERVER[$var] = $val;
    }
}

Once fixed in a release I'll at least be able to exclude this hackery from newer PHP versions.

@cmb69
Copy link
Contributor

cmb69 commented Jan 6, 2022

Nope, no better idea per se, but note that this wouldn't help with env var names containing non-ASCII characters (e.g. FÖÖ=bar). If that's important, you could traverse getenv() instead (that's likely a bit faster anyway).

@Seldaek
Copy link
Author

Seldaek commented Jan 6, 2022

Ok yes using getenv() once makes more sense, although we're really only interested in ascii vars in the context of Composer.

@Seldaek
Copy link
Author

Seldaek commented Jan 7, 2022

@cmb69 it's been brought to my attention that calling getenv() also still suffers from the same issue and mangles unicode values. Only getenv('FOO') appears to work correctly.

So here's a working workaround: https://github.com/composer/composer/blob/d9619985dbabb58255fb27cf78fe702345d31793/bin/composer#L68-L76

@cmb69
Copy link
Contributor

cmb69 commented Jan 7, 2022

Argh! getenv() without parameter also calls php_import_environment_variables(), and I tested with an already patched version.

cmb69 added a commit to cmb69/php-src that referenced this issue Jan 11, 2022
When bug 77574[1] has been fixed, the fix only catered to variables
retrieved via `getenv()` with a `$varname` passed, but neither to
`getenv()` without arguments nor to the general import of environment
variables into `$_ENV` and `$_SERVER`.  We catch up on this by using
`GetEnvironmentStringsW()` in `_php_import_environment_variables()` and
converting the encoding to whatever had been chosen by the user.

[1] <https://bugs.php.net/bug.php?id=75574>
@cmb69 cmb69 linked a pull request Jan 11, 2022 that will close this issue
cmb69 added a commit that referenced this issue Jan 17, 2022
* PHP-8.0:
  Fix GH-7896: Environment vars may be mangled on Windows
@cmb69 cmb69 closed this as completed in 93a3c71 Jan 17, 2022
cmb69 added a commit that referenced this issue Jan 17, 2022
* PHP-8.1:
  Fix GH-7896: Environment vars may be mangled on Windows
@Seldaek
Copy link
Author

Seldaek commented Jan 21, 2022

Thanks @cmb69 !

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

Successfully merging a pull request may close this issue.

3 participants
@Seldaek @cmb69 and others