From 71cf0fe33927dfb65a835d2360791436c2c42384 Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Sat, 26 Jan 2019 16:08:20 +0100 Subject: [PATCH] Fix sheet name escaping Sheet names are stored as attributes of an XML entity. We therefore need a different escaping strategy, escaping quotes. --- src/Spout/Common/Helper/Escaper/ODS.php | 9 ++++----- src/Spout/Common/Helper/Escaper/XLSX.php | 6 +++--- tests/Spout/Common/Helper/Escaper/ODSTest.php | 2 +- tests/Spout/Common/Helper/Escaper/XLSXTest.php | 4 ++-- tests/Spout/Writer/ODS/WriterTest.php | 2 +- tests/Spout/Writer/XLSX/WriterTest.php | 2 +- 6 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/Spout/Common/Helper/Escaper/ODS.php b/src/Spout/Common/Helper/Escaper/ODS.php index cf7d5be..b1e27fa 100644 --- 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 index 01b71c6..3885d5e 100644 --- a/src/Spout/Common/Helper/Escaper/XLSX.php +++ b/src/Spout/Common/Helper/Escaper/XLSX.php @@ -45,9 +45,9 @@ class XLSX implements EscaperInterface $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 index 3be0f3f..858eb68 100644 --- a/tests/Spout/Common/Helper/Escaper/ODSTest.php +++ b/tests/Spout/Common/Helper/Escaper/ODSTest.php @@ -17,7 +17,7 @@ class ODSTest extends TestCase { return [ ['test', 'test'], - ['carl\'s "pokemon"', 'carl\'s "pokemon"'], + ['carl\'s "pokemon"', 'carl's "pokemon"'], ["\n", "\n"], ["\r", "\r"], ["\t", "\t"], diff --git a/tests/Spout/Common/Helper/Escaper/XLSXTest.php b/tests/Spout/Common/Helper/Escaper/XLSXTest.php index 255c7c5..3efe0e7 100644 --- a/tests/Spout/Common/Helper/Escaper/XLSXTest.php +++ b/tests/Spout/Common/Helper/Escaper/XLSXTest.php @@ -17,7 +17,7 @@ class XLSXTest extends TestCase { return [ ['test', 'test'], - ['adam\'s "car"', 'adam\'s "car"'], + ['adam\'s "car"', 'adam's "car"'], ["\n", "\n"], ["\r", "\r"], ["\t", "\t"], @@ -26,7 +26,7 @@ class XLSXTest extends TestCase ['_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's _x0015_ "character"'], ]; } diff --git a/tests/Spout/Writer/ODS/WriterTest.php b/tests/Spout/Writer/ODS/WriterTest.php index 8527249..a362d5b 100644 --- a/tests/Spout/Writer/ODS/WriterTest.php +++ b/tests/Spout/Writer/ODS/WriterTest.php @@ -441,7 +441,7 @@ class WriterTest extends TestCase $this->writeToODSFile($dataRows, $fileName); - $this->assertValueWasWritten($fileName, 'I\'m in "great" mood', 'Quotes should not be escaped'); + $this->assertValueWasWritten($fileName, 'I'm in "great" mood', 'Quotes should not be escaped'); $this->assertValueWasWritten($fileName, 'This <must> be escaped & tested', '<, > and & should be escaped'); } diff --git a/tests/Spout/Writer/XLSX/WriterTest.php b/tests/Spout/Writer/XLSX/WriterTest.php index fa6fc54..bfb4747 100644 --- a/tests/Spout/Writer/XLSX/WriterTest.php +++ b/tests/Spout/Writer/XLSX/WriterTest.php @@ -498,7 +498,7 @@ class WriterTest extends TestCase $this->writeToXLSXFile($dataRows, $fileName); - $this->assertInlineDataWasWrittenToSheet($fileName, 1, 'I\'m in "great" mood', 'Quotes should not be escaped'); + $this->assertInlineDataWasWrittenToSheet($fileName, 1, 'I'm in "great" mood', 'Quotes should not be escaped'); $this->assertInlineDataWasWrittenToSheet($fileName, 1, 'This <must> be escaped & tested', '<, > and & should be escaped'); }