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

Number format may be changed inadvertently #24

Closed
kAlvaro opened this Issue Jan 10, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@kAlvaro

kAlvaro commented Jan 10, 2018

At \PhpMyAdmin\MoTranslator\Loader::setlocale() we can find this:

        $this->locale = $locale;
        // Set system locales as well
        if (function_exists('setlocale')) {
            setlocale(0, $locale);
        }

In my Windows box with the official PHP/5.6.21 binaries (php-5.6.21-Win32-VC11-x86) the LC_... constant that has zero as value is LC_ALL. This basically breaks all code that assumes that numbers gets serialised in locale-independent way, e.g.:

/* @var SimpleXMLElement $xml */
/* @var float $amount */
$xml->addChild('amount', $amount); // Implicit cast to string produces: <amount>3,14</amount>

setlocale() should be called with constants rather than plain integers.

Helper script and sample output:

<?php
echo php_uname(), PHP_EOL;
echo PHP_VERSION, PHP_EOL;
$constants = get_defined_constants(true);
asort($constants['standard']);
foreach ($constants['standard'] as $name => $value) {
	if (mb_substr($name, 0, 3) === 'LC_') {
		printf('% -11s = %s', $name, $value); echo PHP_EOL;
	}
}
Windows NT SOFT2121B 6.2 build 9200 (Unknow Windows version Business Edition) i586
5.3.28
LC_ALL      = 0
LC_COLLATE  = 1
LC_CTYPE    = 2
LC_MONETARY = 3
LC_NUMERIC  = 4
LC_TIME     = 5

Linux SOFT2121b 4.4.0-43-Microsoft #1-Microsoft Wed Dec 31 14:42:53 PST 2014 x86_64
5.5.9-1ubuntu4.21
LC_CTYPE    = 0
LC_NUMERIC  = 1
LC_TIME     = 2
LC_COLLATE  = 3
LC_MONETARY = 4
LC_MESSAGES = 5
LC_ALL      = 6
@kAlvaro

This comment has been minimized.

kAlvaro commented Jan 10, 2018

Follow-up question... Does the library even need locales at all? This library is particularly useful on Windows when running PHP as Apache module because locales are broken in this architecture (they leak across threads). Fiddling with system locales kind of beats the point.

@nijel nijel self-assigned this Feb 12, 2018

@nijel nijel added the enhancement label Feb 12, 2018

@nijel nijel closed this in 0c3f475 Feb 12, 2018

@nijel

This comment has been minimized.

Member

nijel commented Feb 12, 2018

You're right, it should not be part of the library. I've just removed it and released 4.0 with this change.

nijel added a commit to phpmyadmin/phpmyadmin that referenced this issue Feb 12, 2018

Set system locales separately
The motranslator no longer does this, see
phpmyadmin/motranslator#24

Signed-off-by: Michal Čihař <michal@cihar.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment