Fix sheet name escaping

Sheet names are stored as attributes of an XML entity. We therefore need a different escaping strategy, escaping quotes.
jmsche · Jan 26, 2019 · 71cf0fe · 71cf0fe
1 parent ee998f7
commit 71cf0fe
Show file tree

Hide file tree

Showing 6 changed files with 12 additions and 13 deletions.
diff --git a/src/Spout/Common/Helper/Escaper/ODS.php b/src/Spout/Common/Helper/Escaper/ODS.php
@@ -16,17 +16,16 @@ class ODS implements EscaperInterface
      */
     public function escape($string)
     {
+        // @NOTE: Using ENT_QUOTES as XML entities ('<', '>', '&') as well as
+        //        single/double quotes (for XML attributes) need to be encoded.
         if (defined('ENT_DISALLOWED')) {
             // 'ENT_DISALLOWED' ensures that invalid characters in the given document type are replaced.
             // Otherwise control characters like a vertical tab "\v" will make the XML document unreadable by the XML processor
             // @link https://github.com/box/spout/issues/329
-            $replacedString = htmlspecialchars($string, ENT_NOQUOTES | ENT_DISALLOWED);
+            $replacedString = htmlspecialchars($string, ENT_QUOTES | ENT_DISALLOWED);
         } else {
             // We are on hhvm or any other engine that does not support ENT_DISALLOWED.
-            //
-            // @NOTE: Using ENT_NOQUOTES as only XML entities ('<', '>', '&') need to be encoded.
-            //        Single and double quotes can be left as is.
-            $escapedString =  htmlspecialchars($string, ENT_NOQUOTES);
+            $escapedString =  htmlspecialchars($string, ENT_QUOTES);
 
             // control characters values are from 0 to 1F (hex values) in the ASCII table
             // some characters should not be escaped though: "\t", "\r" and "\n".

diff --git a/src/Spout/Common/Helper/Escaper/XLSX.php b/src/Spout/Common/Helper/Escaper/XLSX.php
@@ -45,9 +45,9 @@ public function escape($string)
         $this->initIfNeeded();
 
         $escapedString = $this->escapeControlCharacters($string);
-        // @NOTE: Using ENT_NOQUOTES as only XML entities ('<', '>', '&') need to be encoded.
-        //        Single and double quotes can be left as is.
-        $escapedString = htmlspecialchars($escapedString, ENT_NOQUOTES);
+        // @NOTE: Using ENT_QUOTES as XML entities ('<', '>', '&') as well as
+        //        single/double quotes (for XML attributes) need to be encoded.
+        $escapedString = htmlspecialchars($escapedString, ENT_QUOTES);
 
         return $escapedString;
     }

diff --git a/tests/Spout/Common/Helper/Escaper/ODSTest.php b/tests/Spout/Common/Helper/Escaper/ODSTest.php
@@ -17,7 +17,7 @@ public function dataProviderForTestEscape()
     {
         return [
             ['test', 'test'],
-            ['carl\'s "pokemon"', 'carl\'s "pokemon"'],
+            ['carl\'s "pokemon"', 'carl&#039;s &quot;pokemon&quot;'],
             ["\n", "\n"],
             ["\r", "\r"],
             ["\t", "\t"],

diff --git a/tests/Spout/Common/Helper/Escaper/XLSXTest.php b/tests/Spout/Common/Helper/Escaper/XLSXTest.php
@@ -17,7 +17,7 @@ public function dataProviderForTestEscape()
     {
         return [
             ['test', 'test'],
-            ['adam\'s "car"', 'adam\'s "car"'],
+            ['adam\'s "car"', 'adam&#039;s &quot;car&quot;'],
             ["\n", "\n"],
             ["\r", "\r"],
             ["\t", "\t"],
@@ -26,7 +26,7 @@ public function dataProviderForTestEscape()
             ['_x0000_', '_x005F_x0000_'],
             [chr(21), '_x0015_'],
             ['control ' . chr(21) . ' character', 'control _x0015_ character'],
-            ['control\'s ' . chr(21) . ' "character"', 'control\'s _x0015_ "character"'],
+            ['control\'s ' . chr(21) . ' "character"', 'control&#039;s _x0015_ &quot;character&quot;'],
         ];
     }
 

diff --git a/tests/Spout/Writer/ODS/WriterTest.php b/tests/Spout/Writer/ODS/WriterTest.php
@@ -441,7 +441,7 @@ public function testAddRowShouldEscapeHtmlSpecialCharacters()
 
         $this->writeToODSFile($dataRows, $fileName);
 
-        $this->assertValueWasWritten($fileName, 'I\'m in "great" mood', 'Quotes should not be escaped');
+        $this->assertValueWasWritten($fileName, 'I&#039;m in &quot;great&quot; mood', 'Quotes should not be escaped');
         $this->assertValueWasWritten($fileName, 'This &lt;must&gt; be escaped &amp; tested', '<, > and & should be escaped');
     }
 

diff --git a/tests/Spout/Writer/XLSX/WriterTest.php b/tests/Spout/Writer/XLSX/WriterTest.php
@@ -498,7 +498,7 @@ public function testAddRowShouldEscapeHtmlSpecialCharacters()
 
         $this->writeToXLSXFile($dataRows, $fileName);
 
-        $this->assertInlineDataWasWrittenToSheet($fileName, 1, 'I\'m in "great" mood', 'Quotes should not be escaped');
+        $this->assertInlineDataWasWrittenToSheet($fileName, 1, 'I&#039;m in &quot;great&quot; mood', 'Quotes should not be escaped');
         $this->assertInlineDataWasWrittenToSheet($fileName, 1, 'This &lt;must&gt; be escaped &amp; tested', '<, > and & should be escaped');
     }