Merge pull request #4279 from oleibman/currsymbol

Unexpected Charset Possible for Currency Symbol

Author: oleibman

.github/workflows/main.yml

@@ -25,6 +25,12 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
+      - name: Install locales
+        run: sudo apt-get install -y language-pack-fr language-pack-de
+
+      - name: Install single-byte locale
+        run: sudo sed -i -e 's/# de_DE@euro/de_DE@euro/g' /etc/locale.gen && sudo locale-gen de_DE@euro
+
       - name: Setup PHP, with composer and extensions
         uses: shivammathur/setup-php@v2
         with:

CHANGELOG.md

@@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ### Fixed
 
+- More context options may be needed for http(s) image. [Php issue 17121](https://github.com/php/php-src/issues/17121) [PR #4276](https://github.com/PHPOffice/PhpSpreadsheet/pull/4276)
+- Avoid unexpected charset in currency symbol. [PR #4279](https://github.com/PHPOffice/PhpSpreadsheet/pull/4279)
 - Add forceFullCalc option to Xlsx Writer. [Issue #4269](https://github.com/PHPOffice/PhpSpreadsheet/issues/4269) [PR #4271](https://github.com/PHPOffice/PhpSpreadsheet/pull/4271)
 - More context options may be needed for http(s) image. [Php issue 17121](https://github.com/php/php-src/issues/17121) [PR #4276](https://github.com/PHPOffice/PhpSpreadsheet/pull/4276)
 - Coverage-related tweaks to Xls Reader. [PR #4277](https://github.com/PHPOffice/PhpSpreadsheet/pull/4277)

src/PhpSpreadsheet/Shared/StringHelper.php

@@ -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?
@@ -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;
@@ -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;
@@ -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;

tests/PhpSpreadsheetTests/Reader/Csv/CsvNumberFormatLocaleTest.php

@@ -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;
@@ -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) {
@@ -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) {

tests/PhpSpreadsheetTests/Shared/StringHelperLocaleTest.php

@@ -0,0 +1,55 @@
+<?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 ($this->currentLocale === false || !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, $this->currentLocale)) {
+            self::markTestSkipped('Unable to restore default locale.');
+        }
+        $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@euro')) {
+            self::markTestSkipped('Unable to set German single-byte locale for testing.');
+        }
+        $result = StringHelper::getCurrencyCode(true); // trim if alt symbol is used
+        self::assertSame('EUR', $result, 'non-UTF8 result ignored');
+    }
+}