fix stylemanager with escaper and also escape functions and errors, f…

…ixes openspout#261 (openspout#263)
Slamdunk · Jul 26, 2024 · 393299a · 393299a
1 parent 02fe500
commit 393299a
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 7 deletions.
diff --git a/src/Writer/XLSX/Manager/Style/StyleManager.php b/src/Writer/XLSX/Manager/Style/StyleManager.php
@@ -7,6 +7,7 @@
 use OpenSpout\Common\Entity\Style\BorderPart;
 use OpenSpout\Common\Entity\Style\Color;
 use OpenSpout\Common\Entity\Style\Style;
+use OpenSpout\Common\Helper\Escaper\XLSX as XLSXEscaper;
 use OpenSpout\Writer\Common\Manager\Style\AbstractStyleManager as CommonStyleManager;
 use OpenSpout\Writer\XLSX\Helper\BorderHelper;
 
@@ -17,9 +18,13 @@
  */
 final class StyleManager extends CommonStyleManager
 {
-    public function __construct(StyleRegistry $styleRegistry)
+    /** @var XLSXEscaper Strings escaper */
+    private readonly XLSXEscaper $stringsEscaper;
+
+    public function __construct(StyleRegistry $styleRegistry, XLSXEscaper $stringsEscaper)
     {
         parent::__construct($styleRegistry);
+        $this->stringsEscaper = $stringsEscaper;
     }
 
     /**
@@ -91,7 +96,7 @@ private function getFormatsSectionContent(): string
             /** @var Style $style */
             $style = $this->styleRegistry->getStyleFromStyleId($styleId);
             $format = $style->getFormat();
-            $tags[] = '<numFmt numFmtId="'.$numFmtId.'" formatCode="'.$format.'"/>';
+            $tags[] = '<numFmt numFmtId="'.$numFmtId.'" formatCode="'.$this->stringsEscaper->escape($format).'"/>';
         }
         $content = '<numFmts count="'.\count($tags).'">';
         $content .= implode('', $tags);

diff --git a/src/Writer/XLSX/Manager/WorksheetManager.php b/src/Writer/XLSX/Manager/WorksheetManager.php
@@ -198,14 +198,14 @@ private function getCellXML(int $rowIndexOneBased, int $columnIndexZeroBased, Ce
         } elseif ($cell instanceof Cell\NumericCell) {
             $cellXML .= '><v>'.$cell->getValue().'</v></c>';
         } elseif ($cell instanceof Cell\FormulaCell) {
-            $cellXML .= '><f>'.substr($cell->getValue(), 1).'</f></c>';
+            $cellXML .= '><f>'.$this->stringsEscaper->escape(substr($cell->getValue(), 1)).'</f></c>';
         } elseif ($cell instanceof Cell\DateTimeCell) {
             $cellXML .= '><v>'.DateHelper::toExcel($cell->getValue()).'</v></c>';
         } elseif ($cell instanceof Cell\DateIntervalCell) {
             $cellXML .= '><v>'.DateIntervalHelper::toExcel($cell->getValue()).'</v></c>';
         } elseif ($cell instanceof Cell\ErrorCell) {
             // only writes the error value if it's a string
-            $cellXML .= ' t="e"><v>'.$cell->getRawValue().'</v></c>';
+            $cellXML .= ' t="e"><v>'.$this->stringsEscaper->escape($cell->getRawValue()).'</v></c>';
         } elseif ($cell instanceof Cell\EmptyCell) {
             if ($this->styleManager->shouldApplyStyleOnEmptyCell($styleId)) {
                 $cellXML .= '/>';

diff --git a/src/Writer/XLSX/Writer.php b/src/Writer/XLSX/Writer.php
@@ -51,7 +51,12 @@ protected function createWorkbookManager(): WorkbookManager
         $sharedStringsManager = new SharedStringsManager($xlFolder, new XLSX());
 
         $styleMerger = new StyleMerger();
-        $styleManager = new StyleManager(new StyleRegistry($this->options->DEFAULT_ROW_STYLE));
+        $escaper = new XLSX();
+
+        $styleManager = new StyleManager(
+            new StyleRegistry($this->options->DEFAULT_ROW_STYLE),
+            $escaper
+        );
 
         $commentsManager = new CommentsManager($xlFolder, new XLSX());
 
@@ -61,7 +66,7 @@ protected function createWorkbookManager(): WorkbookManager
             $styleMerger,
             $commentsManager,
             $sharedStringsManager,
-            new XLSX(),
+            $escaper,
             StringHelper::factory()
         );
 

diff --git a/tests/Writer/XLSX/Manager/Style/StyleManagerTest.php b/tests/Writer/XLSX/Manager/Style/StyleManagerTest.php
@@ -4,6 +4,8 @@
 
 namespace OpenSpout\Writer\XLSX\Manager\Style;
 
+use OpenSpout\Common\Entity\Style\Style;
+use OpenSpout\Common\Helper\Escaper\XLSX as XLSXEscaper;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
@@ -47,9 +49,20 @@ public function testShouldApplyStyleOnEmptyCell(?int $fillId, ?int $borderId, bo
             ->willReturn($borderId)
         ;
 
-        $styleManager = new StyleManager($styleRegistryMock);
+        $styleManager = new StyleManager($styleRegistryMock, new XLSXEscaper());
         $shouldApply = $styleManager->shouldApplyStyleOnEmptyCell(99);
 
         self::assertSame($expectedResult, $shouldApply);
     }
+
+    public function testFormatCodeEscapeInSectionContent(): void
+    {
+        $registry = new StyleRegistry(new Style());
+
+        $registry->registerStyle((new Style())->setId(1)->setFormat('"€"* #,##0.00_-'));
+
+        $styleManager = new StyleManager($registry, new XLSXEscaper());
+        $output = $styleManager->getStylesXMLFileContent();
+        self::assertStringContainsString('formatCode="&quot;€&quot;* #,##0.00_-"', $output);
+    }
 }