Skip to content

Commit

Permalink
Unexpected Charset Possible for Currency Symbol
Browse files Browse the repository at this point in the history
We do not recommend it, but users can call the Php function `setlocale` and that might affect the character used as currency symbol. A problem arises when the caller to setlocale does not specify a character set - in that case, Php will attempt to return its `localeconv()` values in a single-byte character set rather than UTF-8. This is particularly problematic for currency symbols. PhpSpreadsheet till now has accepted such a character, and that can lead to corrupt spreadsheets. It is changed to validate the currency symbol as UTF-8, and fall back to a different choice if not (e.g. EUR rather than 0x80, which is how the euro symbol is depicted in Win-1252).

An additional problem arises because Linux systems seem to return the alternate symbol with a trailing blank, but Windows systems do not. To allow callers to get a consistent result, a parameter is added to `getCurrencyCode` which will trim or not (default) the currency code.
  • Loading branch information
oleibman committed Dec 14, 2024
1 parent eccbcce commit 86a87e0
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 38 deletions.
59 changes: 25 additions & 34 deletions src/PhpSpreadsheet/Shared/StringHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ class StringHelper
/**
* Decimal separator.
*/
private static ?string $decimalSeparator;
private static ?string $decimalSeparator = null;

/**
* Thousands separator.
*/
private static ?string $thousandsSeparator;
private static ?string $thousandsSeparator = null;

/**
* Currency code.
*/
private static ?string $currencyCode;
private static ?string $currencyCode = null;

/**
* Is iconv extension avalable?
Expand Down Expand Up @@ -511,21 +511,31 @@ public static function strCaseReverse(string $textValue): string
return implode('', $characters);
}

private static function useAlt(string $altValue, string $default, bool $trimAlt): string
{
return ($trimAlt ? trim($altValue) : $altValue) ?: $default;
}

private static function getLocaleValue(string $key, string $altKey, string $default, bool $trimAlt = false): string
{
$localeconv = localeconv();
$rslt = $localeconv[$key];
// win-1252 implements Euro as 0x80 plus other symbols
if (preg_match('//u', $rslt) !== 1) {
$rslt = '';
}

return $rslt ?: self::useAlt($localeconv[$altKey], $default, $trimAlt);
}

/**
* Get the decimal separator. If it has not yet been set explicitly, try to obtain number
* formatting information from locale.
*/
public static function getDecimalSeparator(): string
{
if (!isset(self::$decimalSeparator)) {
$localeconv = localeconv();
self::$decimalSeparator = ($localeconv['decimal_point'] != '')
? $localeconv['decimal_point'] : $localeconv['mon_decimal_point'];

if (self::$decimalSeparator == '') {
// Default to .
self::$decimalSeparator = '.';
}
self::$decimalSeparator = self::getLocaleValue('decimal_point', 'mon_decimal_point', '.');
}

return self::$decimalSeparator;
Expand All @@ -549,14 +559,7 @@ public static function setDecimalSeparator(?string $separator): void
public static function getThousandsSeparator(): string
{
if (!isset(self::$thousandsSeparator)) {
$localeconv = localeconv();
self::$thousandsSeparator = ($localeconv['thousands_sep'] != '')
? $localeconv['thousands_sep'] : $localeconv['mon_thousands_sep'];

if (self::$thousandsSeparator == '') {
// Default to .
self::$thousandsSeparator = ',';
}
self::$thousandsSeparator = self::getLocaleValue('thousands_sep', 'mon_thousands_sep', ',');
}

return self::$thousandsSeparator;
Expand All @@ -577,22 +580,10 @@ public static function setThousandsSeparator(?string $separator): void
* Get the currency code. If it has not yet been set explicitly, try to obtain the
* symbol information from locale.
*/
public static function getCurrencyCode(): string
public static function getCurrencyCode(bool $trimAlt = false): string
{
if (!empty(self::$currencyCode)) {
return self::$currencyCode;
}
self::$currencyCode = '$';
$localeconv = localeconv();
if (!empty($localeconv['currency_symbol'])) {
self::$currencyCode = $localeconv['currency_symbol'];

return self::$currencyCode;
}
if (!empty($localeconv['int_curr_symbol'])) {
self::$currencyCode = $localeconv['int_curr_symbol'];

return self::$currencyCode;
if (!isset(self::$currencyCode)) {
self::$currencyCode = self::getLocaleValue('currency_symbol', 'int_curr_symbol', '$', $trimAlt);
}

return self::$currencyCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@

use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Reader\Csv;
use PHPUnit\Framework\Attributes;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;

// separate processes due to setLocale
#[Attributes\RunTestsInSeparateProcesses]
class CsvNumberFormatLocaleTest extends TestCase
{
private bool $localeAdjusted;
Expand Down Expand Up @@ -44,8 +48,7 @@ protected function tearDown(): void
}
}

#[\PHPUnit\Framework\Attributes\DataProvider('providerNumberFormatNoConversionTest')]
#[\PHPUnit\Framework\Attributes\RunInSeparateProcess]
#[DataProvider('providerNumberFormatNoConversionTest')]
public function testNumberFormatNoConversion(mixed $expectedValue, string $expectedFormat, string $cellAddress): void
{
if (!$this->localeAdjusted) {
Expand Down Expand Up @@ -85,8 +88,7 @@ public static function providerNumberFormatNoConversionTest(): array
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('providerNumberValueConversionTest')]
#[\PHPUnit\Framework\Attributes\RunInSeparateProcess]
#[DataProvider('providerNumberValueConversionTest')]
public function testNumberValueConversion(mixed $expectedValue, string $cellAddress): void
{
if (!$this->localeAdjusted) {
Expand Down
56 changes: 56 additions & 0 deletions tests/PhpSpreadsheetTests/Shared/StringHelperLocaleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

declare(strict_types=1);

namespace PhpOffice\PhpSpreadsheetTests\Shared;

use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
use PHPUnit\Framework\Attributes;
use PHPUnit\Framework\TestCase;

// separate processes due to setLocale
#[Attributes\RunTestsInSeparateProcesses]
class StringHelperLocaleTest extends TestCase
{
/**
* @var false|string
*/
private $currentLocale;

protected function setUp(): void
{
$this->currentLocale = setlocale(LC_ALL, '0');
}

protected function tearDown(): void
{
if (is_string($this->currentLocale)) {
setlocale(LC_ALL, $this->currentLocale);
}
StringHelper::setCurrencyCode(null);
}

public function testCurrency(): void
{
if (!setlocale(LC_ALL, 'de_DE.UTF-8', 'deu_deu.utf8')) {
self::markTestSkipped('Unable to set German UTF8 locale for testing.');
}
$result = StringHelper::getCurrencyCode();
self::assertSame('', $result);
if (!setlocale(LC_ALL, 'en_us')) {
self::markTestSkipped('Unable to set US locale for testing.');
}
$result = StringHelper::getCurrencyCode();
self::assertSame('', $result, 'result persists despite locale change');
StringHelper::setCurrencyCode(null);
$result = StringHelper::getCurrencyCode();
self::assertSame('$', $result, 'locale now used');
StringHelper::setCurrencyCode(null);
if (!setlocale(LC_ALL, 'deu_deu', 'de_DE')) {
self::markTestSkipped('Unable to set German single-byte locale for testing.');
}
// Seems like Linux returns trailing blank, Win doesn't
$result = StringHelper::getCurrencyCode(true); // trim if alt symbol is used
self::assertSame('EUR', $result, 'non-UTF8 result ignored');
}
}

0 comments on commit 86a87e0

Please sign in to comment.