Reading Xlsx With Supplied Palette (#2595)

oleibman · web-flow · commit 9cf526a92046 · 2022-02-23T22:09:22.000-08:00
Fix #2499, which see for details of an obscure problem affecting both PhpSpreadsheet and Excel. Add support for palette contained in workbook styles. This seems to be a very rare occurrence, so allow it only when the palette contains exactly 64 entries. If there are other possibilities, we'll presumably have a new workbook to guide us how to handle them. Also add some tests for specification of indexed color without palette, another rarity (no in-range examples amongst our current files). Also change one private static array, initialized once at run-time and never changed, to a constant.
diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -541,6 +541,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                         $xmlStyles = $this->loadZip("$dir/$xpath[Target]", $mainNS);
                     }
 
+                    $palette = self::extractPalette($xmlStyles);
+                    $this->styleReader->setWorkbookPalette($palette);
                     $fills = self::extractStyles($xmlStyles, 'fills', 'fill');
                     $fonts = self::extractStyles($xmlStyles, 'fonts', 'font');
                     $borders = self::extractStyles($xmlStyles, 'borders', 'border');
@@ -2103,4 +2105,21 @@ private static function extractStyles(?SimpleXMLElement $sxml, string $node1, st
 
         return $array;
     }
+
+    private static function extractPalette(?SimpleXMLElement $sxml): array
+    {
+        $array = [];
+        if ($sxml && $sxml->colors->indexedColors) {
+            foreach ($sxml->colors->indexedColors->rgbColor as $node) {
+                if ($node !== null) {
+                    $attr = $node->attributes();
+                    if (isset($attr['rgb'])) {
+                        $array[] = (string) $attr['rgb'];
+                    }
+                }
+            }
+        }
+
+        return (count($array) === 64) ? $array : [];
+    }
 }
diff --git a/src/PhpSpreadsheet/Reader/Xlsx/Styles.php b/src/PhpSpreadsheet/Reader/Xlsx/Styles.php
@@ -24,6 +24,9 @@ class Styles extends BaseParserClass
      */
     private $theme;
 
+    /** @var array */
+    private $workbookPalette = [];
+
     /** @var array */
     private $styles = [];
 
@@ -41,6 +44,11 @@ public function setNamespace(string $namespace): void
         $this->namespace = $namespace;
     }
 
+    public function setWorkbookPalette(array $palette): void
+    {
+        $this->workbookPalette = $palette;
+    }
+
     /**
      * Cast SimpleXMLElement to bool to overcome Scrutinizer problem.
      *
@@ -355,7 +363,11 @@ public function readColor(SimpleXMLElement $color, bool $background = false): st
             return (string) $attr['rgb'];
         }
         if (isset($attr['indexed'])) {
-            return Color::indexedColor((int) ($attr['indexed'] - 7), $background)->getARGB() ?? '';
+            if (empty($this->workbookPalette)) {
+                return Color::indexedColor((int) ($attr['indexed'] - 7), $background)->getARGB() ?? '';
+            }
+
+            return Color::indexedColor((int) ($attr['indexed']), $background, $this->workbookPalette)->getARGB() ?? '';
         }
         if (isset($attr['theme'])) {
             if ($this->theme !== null) {
diff --git a/src/PhpSpreadsheet/Style/Color.php b/src/PhpSpreadsheet/Style/Color.php
@@ -45,12 +45,64 @@ class Color extends Supervisor
     const VALIDATE_COLOR_6 = '/^[A-F0-9]{6}$/i';
     const VALIDATE_COLOR_8 = '/^[A-F0-9]{8}$/i';
 
-    /**
-     * Indexed colors array.
-     *
-     * @var array
-     */
-    protected static $indexedColors;
+    private const INDEXED_COLORS = [
+        1 => 'FF000000', //  System Colour #1 - Black
+        2 => 'FFFFFFFF', //  System Colour #2 - White
+        3 => 'FFFF0000', //  System Colour #3 - Red
+        4 => 'FF00FF00', //  System Colour #4 - Green
+        5 => 'FF0000FF', //  System Colour #5 - Blue
+        6 => 'FFFFFF00', //  System Colour #6 - Yellow
+        7 => 'FFFF00FF', //  System Colour #7- Magenta
+        8 => 'FF00FFFF', //  System Colour #8- Cyan
+        9 => 'FF800000', //  Standard Colour #9
+        10 => 'FF008000', //  Standard Colour #10
+        11 => 'FF000080', //  Standard Colour #11
+        12 => 'FF808000', //  Standard Colour #12
+        13 => 'FF800080', //  Standard Colour #13
+        14 => 'FF008080', //  Standard Colour #14
+        15 => 'FFC0C0C0', //  Standard Colour #15
+        16 => 'FF808080', //  Standard Colour #16
+        17 => 'FF9999FF', //  Chart Fill Colour #17
+        18 => 'FF993366', //  Chart Fill Colour #18
+        19 => 'FFFFFFCC', //  Chart Fill Colour #19
+        20 => 'FFCCFFFF', //  Chart Fill Colour #20
+        21 => 'FF660066', //  Chart Fill Colour #21
+        22 => 'FFFF8080', //  Chart Fill Colour #22
+        23 => 'FF0066CC', //  Chart Fill Colour #23
+        24 => 'FFCCCCFF', //  Chart Fill Colour #24
+        25 => 'FF000080', //  Chart Line Colour #25
+        26 => 'FFFF00FF', //  Chart Line Colour #26
+        27 => 'FFFFFF00', //  Chart Line Colour #27
+        28 => 'FF00FFFF', //  Chart Line Colour #28
+        29 => 'FF800080', //  Chart Line Colour #29
+        30 => 'FF800000', //  Chart Line Colour #30
+        31 => 'FF008080', //  Chart Line Colour #31
+        32 => 'FF0000FF', //  Chart Line Colour #32
+        33 => 'FF00CCFF', //  Standard Colour #33
+        34 => 'FFCCFFFF', //  Standard Colour #34
+        35 => 'FFCCFFCC', //  Standard Colour #35
+        36 => 'FFFFFF99', //  Standard Colour #36
+        37 => 'FF99CCFF', //  Standard Colour #37
+        38 => 'FFFF99CC', //  Standard Colour #38
+        39 => 'FFCC99FF', //  Standard Colour #39
+        40 => 'FFFFCC99', //  Standard Colour #40
+        41 => 'FF3366FF', //  Standard Colour #41
+        42 => 'FF33CCCC', //  Standard Colour #42
+        43 => 'FF99CC00', //  Standard Colour #43
+        44 => 'FFFFCC00', //  Standard Colour #44
+        45 => 'FFFF9900', //  Standard Colour #45
+        46 => 'FFFF6600', //  Standard Colour #46
+        47 => 'FF666699', //  Standard Colour #47
+        48 => 'FF969696', //  Standard Colour #48
+        49 => 'FF003366', //  Standard Colour #49
+        50 => 'FF339966', //  Standard Colour #50
+        51 => 'FF003300', //  Standard Colour #51
+        52 => 'FF333300', //  Standard Colour #52
+        53 => 'FF993300', //  Standard Colour #53
+        54 => 'FF993366', //  Standard Colour #54
+        55 => 'FF333399', //  Standard Colour #55
+        56 => 'FF333333', //  Standard Colour #56
+    ];
 
     /**
      * ARGB - Alpha RGB.
@@ -335,75 +387,19 @@ public static function changeBrightness($hexColourValue, $adjustPercentage)
      *
      * @return Color
      */
-    public static function indexedColor($colorIndex, $background = false): self
+    public static function indexedColor($colorIndex, $background = false, ?array $palette = null): self
     {
         // Clean parameter
         $colorIndex = (int) $colorIndex;
 
-        // Indexed colors
-        if (self::$indexedColors === null) {
-            self::$indexedColors = [
-                1 => 'FF000000', //  System Colour #1 - Black
-                2 => 'FFFFFFFF', //  System Colour #2 - White
-                3 => 'FFFF0000', //  System Colour #3 - Red
-                4 => 'FF00FF00', //  System Colour #4 - Green
-                5 => 'FF0000FF', //  System Colour #5 - Blue
-                6 => 'FFFFFF00', //  System Colour #6 - Yellow
-                7 => 'FFFF00FF', //  System Colour #7- Magenta
-                8 => 'FF00FFFF', //  System Colour #8- Cyan
-                9 => 'FF800000', //  Standard Colour #9
-                10 => 'FF008000', //  Standard Colour #10
-                11 => 'FF000080', //  Standard Colour #11
-                12 => 'FF808000', //  Standard Colour #12
-                13 => 'FF800080', //  Standard Colour #13
-                14 => 'FF008080', //  Standard Colour #14
-                15 => 'FFC0C0C0', //  Standard Colour #15
-                16 => 'FF808080', //  Standard Colour #16
-                17 => 'FF9999FF', //  Chart Fill Colour #17
-                18 => 'FF993366', //  Chart Fill Colour #18
-                19 => 'FFFFFFCC', //  Chart Fill Colour #19
-                20 => 'FFCCFFFF', //  Chart Fill Colour #20
-                21 => 'FF660066', //  Chart Fill Colour #21
-                22 => 'FFFF8080', //  Chart Fill Colour #22
-                23 => 'FF0066CC', //  Chart Fill Colour #23
-                24 => 'FFCCCCFF', //  Chart Fill Colour #24
-                25 => 'FF000080', //  Chart Line Colour #25
-                26 => 'FFFF00FF', //  Chart Line Colour #26
-                27 => 'FFFFFF00', //  Chart Line Colour #27
-                28 => 'FF00FFFF', //  Chart Line Colour #28
-                29 => 'FF800080', //  Chart Line Colour #29
-                30 => 'FF800000', //  Chart Line Colour #30
-                31 => 'FF008080', //  Chart Line Colour #31
-                32 => 'FF0000FF', //  Chart Line Colour #32
-                33 => 'FF00CCFF', //  Standard Colour #33
-                34 => 'FFCCFFFF', //  Standard Colour #34
-                35 => 'FFCCFFCC', //  Standard Colour #35
-                36 => 'FFFFFF99', //  Standard Colour #36
-                37 => 'FF99CCFF', //  Standard Colour #37
-                38 => 'FFFF99CC', //  Standard Colour #38
-                39 => 'FFCC99FF', //  Standard Colour #39
-                40 => 'FFFFCC99', //  Standard Colour #40
-                41 => 'FF3366FF', //  Standard Colour #41
-                42 => 'FF33CCCC', //  Standard Colour #42
-                43 => 'FF99CC00', //  Standard Colour #43
-                44 => 'FFFFCC00', //  Standard Colour #44
-                45 => 'FFFF9900', //  Standard Colour #45
-                46 => 'FFFF6600', //  Standard Colour #46
-                47 => 'FF666699', //  Standard Colour #47
-                48 => 'FF969696', //  Standard Colour #48
-                49 => 'FF003366', //  Standard Colour #49
-                50 => 'FF339966', //  Standard Colour #50
-                51 => 'FF003300', //  Standard Colour #51
-                52 => 'FF333300', //  Standard Colour #52
-                53 => 'FF993300', //  Standard Colour #53
-                54 => 'FF993366', //  Standard Colour #54
-                55 => 'FF333399', //  Standard Colour #55
-                56 => 'FF333333', //  Standard Colour #56
-            ];
-        }
-
-        if (isset(self::$indexedColors[$colorIndex])) {
-            return new self(self::$indexedColors[$colorIndex]);
+        if (empty($palette)) {
+            if (isset(self::INDEXED_COLORS[$colorIndex])) {
+                return new self(self::INDEXED_COLORS[$colorIndex]);
+            }
+        } else {
+            if (isset($palette[$colorIndex])) {
+                return new self($palette[$colorIndex]);
+            }
         }
 
         return ($background) ? new self(self::COLOR_WHITE) : new self(self::COLOR_BLACK);
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2490Test.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2490Test.php
@@ -0,0 +1,41 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PHPUnit\Framework\TestCase;
+
+class Issue2490Test extends TestCase
+{
+    /**
+     * @var string
+     */
+    private static $testbook = 'tests/data/Reader/XLSX/issue.2490.xlsx';
+
+    public function testPreliminaries(): void
+    {
+        $file = 'zip://';
+        $file .= self::$testbook;
+        $file .= '#xl/styles.xml';
+        $data = file_get_contents($file);
+        // confirm that file contains expected color index tag
+        if ($data === false) {
+            self::fail('Unable to read file');
+        } else {
+            self::assertStringContainsString('<colors><indexedColors><rgbColor rgb="00000000"/>', $data);
+        }
+    }
+
+    public function testIssue2490(): void
+    {
+        // Spreadsheet with its own color palette.
+        $filename = self::$testbook;
+        $reader = IOFactory::createReader('Xlsx');
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame('00FFFFFF', $sheet->getCell('A3')->getStyle()->getFill()->getStartColor()->getArgb());
+        self::assertSame('00F0FBFF', $sheet->getCell('A1')->getStyle()->getFill()->getStartColor()->getArgb());
+        self::assertSame('00F0F0F0', $sheet->getCell('B1')->getStyle()->getFill()->getStartColor()->getArgb());
+        $spreadsheet->disconnectWorksheets();
+    }
+}
diff --git a/tests/PhpSpreadsheetTests/Style/ColorIndexTest.php b/tests/PhpSpreadsheetTests/Style/ColorIndexTest.php
@@ -0,0 +1,34 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Style;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx\Styles;
+use PHPUnit\Framework\TestCase;
+
+class ColorIndexTest extends TestCase
+{
+    /**
+     * @dataProvider providerColorIndexes
+     */
+    public function testColorIndex(string $expectedResult, string $xml, bool $background = false): void
+    {
+        $sxml = simplexml_load_string($xml);
+        if ($sxml === false) {
+            self::fail('Unable to parse xml');
+        } else {
+            $styles = new Styles();
+            $result = $styles->readColor($sxml, $background);
+            self::assertSame($expectedResult, $result);
+        }
+    }
+
+    public function providerColorIndexes(): array
+    {
+        return [
+            'subtract 7 to return system color 4' => ['FF00FF00', '<fgColor indexed="11"/>'],
+            'default foreground color when out of range' => ['FF000000', '<color indexed="81"/>'],
+            'default background color when out of range' => ['FFFFFFFF', '<bgColor indexed="81"/>', true],
+            'rgb specified' => ['FF123456', '<bgColor rgb="FF123456"/>', true],
+        ];
+    }
+}
diff --git a/tests/data/Reader/XLSX/issue.2490.xlsx b/tests/data/Reader/XLSX/issue.2490.xlsx
