diff --git a/.travis.yml b/.travis.yml index d4306c9..9e90442 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,7 @@ install: script: - mkdir -p build/logs - - php vendor/bin/phpunit --coverage-clover build/logs/clover.xml + - php vendor/bin/phpunit --coverage-clover build/logs/clover.xml --coverage-text after_script: - if [[ $TRAVIS_PHP_VERSION != 'hhvm' && $TRAVIS_PHP_VERSION != '7.0' ]]; then php vendor/bin/ocular code-coverage:upload --format=php-clover build/logs/clover.xml; fi diff --git a/phpunit.xml b/phpunit.xml index fc6d657..06ddf63 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -5,7 +5,6 @@ colors="true" convertErrorsToExceptions="false" convertWarningsToExceptions="false" - strict="false" verbose="false"> diff --git a/src/Spout/Common/Helper/FileSystemHelper.php b/src/Spout/Common/Helper/FileSystemHelper.php index d7ca64f..6186822 100644 --- a/src/Spout/Common/Helper/FileSystemHelper.php +++ b/src/Spout/Common/Helper/FileSystemHelper.php @@ -63,7 +63,7 @@ class FileSystemHelper $filePath = $parentFolderPath . '/' . $fileName; $wasCreationSuccessful = file_put_contents($filePath, $fileContents); - if (!$wasCreationSuccessful) { + if ($wasCreationSuccessful === false) { throw new IOException('Unable to create file: ' . $filePath); } diff --git a/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php b/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php index 5c8fb46..75f8989 100644 --- a/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php +++ b/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php @@ -154,7 +154,8 @@ class SharedStringsHelper $readError = libxml_get_last_error(); if ($readError !== false) { - throw new IOException("The sharedStrings.xml file is invalid and cannot be read. [{$readError->message}]"); + $readErrorMessage = trim($readError->message); + throw new IOException("The sharedStrings.xml file is invalid and cannot be read. [{$readErrorMessage}]"); } // reset the setting to display XML warnings/errors diff --git a/src/Spout/Reader/XLSX/RowIterator.php b/src/Spout/Reader/XLSX/RowIterator.php index e96898f..6fc1dde 100644 --- a/src/Spout/Reader/XLSX/RowIterator.php +++ b/src/Spout/Reader/XLSX/RowIterator.php @@ -131,12 +131,19 @@ class RowIterator implements IteratorInterface * * @return void * @throws \Box\Spout\Reader\Exception\SharedStringNotFoundException If a shared string was not found + * @throws \Box\Spout\Common\Exception\IOException If unable to read the sheet data XML */ public function next() { $isInsideRowTag = false; $rowData = []; + // Use internal errors to avoid displaying lots of warning messages in case of invalid file + // For instance on HHVM, XMLReader->open() won't fail when trying to read a unexisting file within a zip... + // But the XMLReader->read() will fail! + libxml_clear_errors(); + libxml_use_internal_errors(true); + while ($this->xmlReader->read()) { if ($this->xmlReader->nodeType == \XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_DIMENSION) { // Read dimensions of the sheet @@ -180,6 +187,12 @@ class RowIterator implements IteratorInterface } } + $readError = libxml_get_last_error(); + if ($readError !== false) { + $readErrorMessage = trim($readError->message); + throw new IOException("The {$this->sheetDataXMLFilePath} file cannot be read. [{$readErrorMessage}]"); + } + $this->rowDataBuffer = $rowData; } diff --git a/tests/Spout/Reader/CSV/ReaderTest.php b/tests/Spout/Reader/CSV/ReaderTest.php index 922c61b..932633f 100644 --- a/tests/Spout/Reader/CSV/ReaderTest.php +++ b/tests/Spout/Reader/CSV/ReaderTest.php @@ -54,6 +54,25 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $reader->open($resourcePath); } + /** + * @expectedException \Box\Spout\Common\Exception\IOException + * + * @return void + */ + public function testOpenShouldThrowExceptionIfCannotOpenFile() + { + $helperStub = $this->getMockBuilder('\Box\Spout\Common\Helper\GlobalFunctionsHelper') + ->setMethods(['fopen']) + ->getMock(); + $helperStub->method('fopen')->willReturn(false); + + $resourcePath = $this->getResourcePath('csv_standard.csv'); + + $reader = ReaderFactory::create(Type::CSV); + $reader->setGlobalFunctionsHelper($helperStub); + $reader->open($resourcePath); + } + /** * @return void @@ -161,6 +180,50 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expectedRows, $allRows); } + /** + * @return void + */ + public function testReadMultipleTimesShouldRewindReader() + { + $allRows = []; + $resourcePath = $this->getResourcePath('csv_standard.csv'); + + $reader = ReaderFactory::create(Type::CSV); + $reader->open($resourcePath); + + foreach ($reader->getSheetIterator() as $sheet) { + // do nothing + } + + foreach ($reader->getSheetIterator() as $sheet) { + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + } + + foreach ($reader->getSheetIterator() as $sheet) { + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + } + + $reader->close(); + + $expectedRows = [ + ['csv--11', 'csv--12', 'csv--13'], + ['csv--11', 'csv--12', 'csv--13'], + ['csv--11', 'csv--12', 'csv--13'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * @param string $fileName * @param string|void $fieldDelimiter diff --git a/tests/Spout/Reader/XLSX/ReaderTest.php b/tests/Spout/Reader/XLSX/ReaderTest.php index 9643d54..8c8376b 100644 --- a/tests/Spout/Reader/XLSX/ReaderTest.php +++ b/tests/Spout/Reader/XLSX/ReaderTest.php @@ -6,6 +6,7 @@ use Box\Spout\Common\Exception\IOException; use Box\Spout\Common\Type; use Box\Spout\Reader\ReaderFactory; use Box\Spout\TestUsingResource; +use Symfony\Component\Config\Definition\Exception\Exception; /** * Class ReaderTest @@ -24,6 +25,7 @@ class ReaderTest extends \PHPUnit_Framework_TestCase return [ ['/path/to/fake/file.xlsx'], ['file_with_no_sheets_in_content_types.xlsx'], + ['file_with_sheet_xml_not_matching_content_types.xlsx'], ['file_corrupted.xlsx'], ]; } @@ -37,7 +39,8 @@ class ReaderTest extends \PHPUnit_Framework_TestCase */ public function testReadShouldThrowException($filePath) { - $this->getAllRowsForFile($filePath); + // using @ to prevent warnings/errors from being displayed + @$this->getAllRowsForFile($filePath); } /** @@ -240,7 +243,8 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $startTime = microtime(true); try { - $this->getAllRowsForFile($fileName); + // using @ to prevent warnings/errors from being displayed + @$this->getAllRowsForFile($fileName); $this->fail('An exception should have been thrown'); } catch (IOException $exception) { $duration = microtime(true) - $startTime; @@ -275,6 +279,60 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expectedRows, $allRows); } + /** + * @return void + */ + public function testReadMultipleTimesShouldRewindReader() + { + $allRows = []; + $resourcePath = $this->getResourcePath('two_sheets_with_inline_strings.xlsx'); + + $reader = ReaderFactory::create(Type::XLSX); + $reader->open($resourcePath); + + foreach ($reader->getSheetIterator() as $sheet) { + // do nothing + } + + foreach ($reader->getSheetIterator() as $sheet) { + // this loop should only add the first row of the first sheet + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + // this loop should rewind the iterator and restart reading from the 1st row again + // therefore, it should only add the first row of the first sheet + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + // not reading any more sheets + break; + } + + foreach ($reader->getSheetIterator() as $sheet) { + // this loop should only add the first row of the current sheet + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + // not breaking, so we keep reading the next sheets + } + + $reader->close(); + + $expectedRows = [ + ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'], + ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'], + ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'], + ['s2 - A1', 's2 - B1', 's2 - C1', 's2 - D1', 's2 - E1'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * @param string $fileName * @return array All the read rows the given file diff --git a/tests/resources/xlsx/file_with_sheet_xml_not_matching_content_types.xlsx b/tests/resources/xlsx/file_with_sheet_xml_not_matching_content_types.xlsx new file mode 100644 index 0000000..f33c9d7 Binary files /dev/null and b/tests/resources/xlsx/file_with_sheet_xml_not_matching_content_types.xlsx differ