From f82a18e6f4970576a12d693c916aa2e99c2bd800 Mon Sep 17 00:00:00 2001 From: Tim Wentzell Date: Wed, 6 Jan 2021 14:54:40 -0400 Subject: [PATCH] Make injection protection optional (#429) --- docs/reference/symfony.rst | 2 ++ .../DependencyInjection/Configuration.php | 8 ++++++ .../Symfony/Resources/config/services.xml | 2 ++ src/Writer/CsvWriter.php | 23 +++++++++++----- src/Writer/XlsWriter.php | 26 ++++++++++++++----- .../DependencyInjection/ConfigurationTest.php | 2 ++ .../SonataExporterExtensionTest.php | 2 ++ 7 files changed, 51 insertions(+), 14 deletions(-) diff --git a/docs/reference/symfony.rst b/docs/reference/symfony.rst index 5238fe0d..5f8bee46 100644 --- a/docs/reference/symfony.rst +++ b/docs/reference/symfony.rst @@ -46,6 +46,7 @@ This service can be configured throught the following parameters: * ``sonata.exporter.writer.csv.escape``: defaults to ``\\`` * ``sonata.exporter.writer.csv.show_headers``: defaults to ``true`` * ``sonata.exporter.writer.csv.with_bom``: defaults to ``false`` +* ``sonata.exporter.writer.csv.safe_cells``: defaults to ``false`` The JSON writer service ~~~~~~~~~~~~~~~~~~~~~~~ @@ -60,6 +61,7 @@ This service can be configured throught the following parameters: * ``sonata.exporter.writer.xls.filename``: defaults to ``php://output`` * ``sonata.exporter.writer.xls.show_headers``: defaults to ``true`` +* ``sonata.exporter.writer.xls.safe_cells``: defaults to ``false`` The XML writer service ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/Bridge/Symfony/DependencyInjection/Configuration.php b/src/Bridge/Symfony/DependencyInjection/Configuration.php index dff87d2d..9947f37d 100644 --- a/src/Bridge/Symfony/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/DependencyInjection/Configuration.php @@ -69,6 +69,10 @@ public function getConfigTreeBuilder(): TreeBuilder ->defaultValue(false) ->info('include the byte order mark') ->end() + ->booleanNode('safe_cells') + ->defaultValue(false) + ->info('escapes data cells that that may be interpreted as formulas in spreadsheet software') + ->end() ->end() ->end() ->arrayNode('json') @@ -91,6 +95,10 @@ public function getConfigTreeBuilder(): TreeBuilder ->defaultValue(true) ->info('add column names as the first line') ->end() + ->booleanNode('safe_cells') + ->defaultValue(false) + ->info('escapes data cells that that may be interpreted as formulas in spreadsheet software') + ->end() ->end() ->end() ->arrayNode('xml') diff --git a/src/Bridge/Symfony/Resources/config/services.xml b/src/Bridge/Symfony/Resources/config/services.xml index 1cc45450..d0ac330d 100644 --- a/src/Bridge/Symfony/Resources/config/services.xml +++ b/src/Bridge/Symfony/Resources/config/services.xml @@ -8,6 +8,7 @@ %sonata.exporter.writer.csv.escape% %sonata.exporter.writer.csv.show_headers% %sonata.exporter.writer.csv.with_bom% + %sonata.exporter.writer.csv.safe_cells% %sonata.exporter.writer.json.filename% @@ -15,6 +16,7 @@ %sonata.exporter.writer.xls.filename% %sonata.exporter.writer.xls.show_headers% + %sonata.exporter.writer.xls.safe_cells% %sonata.exporter.writer.xml.filename% diff --git a/src/Writer/CsvWriter.php b/src/Writer/CsvWriter.php index d0c72496..354b8dd9 100644 --- a/src/Writer/CsvWriter.php +++ b/src/Writer/CsvWriter.php @@ -65,6 +65,11 @@ final class CsvWriter implements TypedWriterInterface */ private $terminate; + /** + * @var bool + */ + private $safeCells; + /** * @throws \RuntimeException */ @@ -75,7 +80,8 @@ public function __construct( string $escape = '\\', bool $showHeaders = true, bool $withBom = false, - string $terminate = "\n" + string $terminate = "\n", + bool $safeCells = false ) { $this->filename = $filename; $this->delimiter = $delimiter; @@ -85,6 +91,7 @@ public function __construct( $this->terminate = $terminate; $this->position = 0; $this->withBom = $withBom; + $this->safeCells = $safeCells; if (is_file($filename)) { throw new \RuntimeException(sprintf('The file %s already exist', $filename)); @@ -138,12 +145,14 @@ public function write(array $data): void } // prevent csv injection - foreach ($data as $key => $value) { - $data[$key] = preg_replace( - ['/^=/', '/^\+/', '/^-/', '/^@/'], - ['\'=', '\'+', '\'-', '\'@'], - $value - ); + if (true === $this->safeCells) { + foreach ($data as $key => $value) { + $data[$key] = preg_replace( + ['/^=/', '/^\+/', '/^-/', '/^@/'], + ['\'=', '\'+', '\'-', '\'@'], + $value + ); + } } $result = @fputcsv($this->file, $data, $this->delimiter, $this->enclosure, $this->escape); diff --git a/src/Writer/XlsWriter.php b/src/Writer/XlsWriter.php index 4a0ab464..e6311274 100644 --- a/src/Writer/XlsWriter.php +++ b/src/Writer/XlsWriter.php @@ -38,13 +38,19 @@ final class XlsWriter implements TypedWriterInterface */ private $position = 0; + /** + * @var bool + */ + private $safeCells; + /** * @throws \RuntimeException */ - public function __construct(string $filename, bool $showHeaders = true) + public function __construct(string $filename, bool $showHeaders = true, bool $safeCells = false) { $this->filename = $filename; $this->showHeaders = $showHeaders; + $this->safeCells = $safeCells; if (is_file($filename)) { throw new \RuntimeException(sprintf('The file %s already exists', $filename)); @@ -79,12 +85,18 @@ public function write(array $data): void fwrite($this->file, ''); // prevent xls injection - foreach ($data as $value) { - fwrite($this->file, sprintf('%s', preg_replace( - ['/^=/', '/^\+/', '/^-/', '/^@/'], - ['\'=', '\'+', '\'-', '\'@'], - $value - ))); + if (true === $this->safeCells) { + foreach ($data as $value) { + fwrite($this->file, sprintf('%s', preg_replace( + ['/^=/', '/^\+/', '/^-/', '/^@/'], + ['\'=', '\'+', '\'-', '\'@'], + $value + ))); + } + } else { + foreach ($data as $value) { + fwrite($this->file, sprintf('%s', $value)); + } } fwrite($this->file, ''); diff --git a/tests/Bridge/Symfony/DependencyInjection/ConfigurationTest.php b/tests/Bridge/Symfony/DependencyInjection/ConfigurationTest.php index 3b1533fe..9041dbc9 100644 --- a/tests/Bridge/Symfony/DependencyInjection/ConfigurationTest.php +++ b/tests/Bridge/Symfony/DependencyInjection/ConfigurationTest.php @@ -42,6 +42,7 @@ public function testDefault(): void 'escape' => '\\', 'show_headers' => true, 'with_bom' => false, + 'safe_cells' => false, ], 'json' => [ 'filename' => 'php://output', @@ -49,6 +50,7 @@ public function testDefault(): void 'xls' => [ 'filename' => 'php://output', 'show_headers' => true, + 'safe_cells' => false, ], 'xml' => [ 'filename' => 'php://output', diff --git a/tests/Bridge/Symfony/DependencyInjection/SonataExporterExtensionTest.php b/tests/Bridge/Symfony/DependencyInjection/SonataExporterExtensionTest.php index f5b6c6bb..1f580346 100644 --- a/tests/Bridge/Symfony/DependencyInjection/SonataExporterExtensionTest.php +++ b/tests/Bridge/Symfony/DependencyInjection/SonataExporterExtensionTest.php @@ -34,9 +34,11 @@ public function testServiceParametersArePresent(): void 'sonata.exporter.writer.csv.escape', 'sonata.exporter.writer.csv.show_headers', 'sonata.exporter.writer.csv.with_bom', + 'sonata.exporter.writer.csv.safe_cells', 'sonata.exporter.writer.json.filename', 'sonata.exporter.writer.xls.filename', 'sonata.exporter.writer.xls.show_headers', + 'sonata.exporter.writer.xls.safe_cells', 'sonata.exporter.writer.xml.filename', 'sonata.exporter.writer.xml.main_element', 'sonata.exporter.writer.xml.child_element',