Add wrappers around XMLReader and SimpleXMLElement to improve error handling

src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php

@@ -3,6 +3,9 @@
 namespace Box\Spout\Reader\XLSX\Helper;
 
 use Box\Spout\Common\Exception\IOException;
+use Box\Spout\Reader\Exception\XMLProcessingException;
+use Box\Spout\Reader\Wrapper\SimpleXMLElement;
+use Box\Spout\Reader\Wrapper\XMLReader;
 use Box\Spout\Reader\XLSX\Helper\SharedStringsCaching\CachingStrategyFactory;
 use Box\Spout\Reader\XLSX\Helper\SharedStringsCaching\CachingStrategyInterface;
 
@@ -74,7 +77,7 @@ class SharedStringsHelper
      */
     public function extractSharedStrings()
     {
-        $xmlReader = new \XMLReader();
+        $xmlReader = new XMLReader();
         $sharedStringIndex = 0;
         $escaper = new \Box\Spout\Common\Escaper\XLSX();
 
@@ -83,39 +86,44 @@ class SharedStringsHelper
             throw new IOException('Could not open "' . self::SHARED_STRINGS_XML_FILE_PATH . '".');
         }
 
-        $sharedStringsUniqueCount = $this->getSharedStringsUniqueCount($xmlReader);
-        $this->cachingStrategy = $this->getBestSharedStringsCachingStrategy($sharedStringsUniqueCount);
+        try {
+            $sharedStringsUniqueCount = $this->getSharedStringsUniqueCount($xmlReader);
+            $this->cachingStrategy = $this->getBestSharedStringsCachingStrategy($sharedStringsUniqueCount);
 
-        while ($xmlReader->read() && $xmlReader->name !== 'si') {
-            // do nothing until a 'si' tag is reached
-        }
-
-        while ($xmlReader->name === 'si') {
-            $node = $this->getSimpleXmlElementNodeFromXMLReader($xmlReader);
-            $node->registerXPathNamespace('ns', self::MAIN_NAMESPACE_FOR_SHARED_STRINGS_XML);
-
-            // removes nodes that should not be read, like the pronunciation of the Kanji characters
-            $cleanNode = $this->removeSuperfluousTextNodes($node);
-
-            // find all text nodes 't'; there can be multiple if the cell contains formatting
-            $textNodes = $cleanNode->xpath('//ns:t');
-
-            $textValue = '';
-            foreach ($textNodes as $textNode) {
-                if ($this->shouldPreserveWhitespace($textNode)) {
-                    $textValue .= $textNode->__toString();
-                } else {
-                    $textValue .= trim($textNode->__toString());
-                }
+            while ($xmlReader->read() && $xmlReader->name !== 'si') {
+                // do nothing until a 'si' tag is reached
             }
 
-            $unescapedTextValue = $escaper->unescape($textValue);
-            $this->cachingStrategy->addStringForIndex($unescapedTextValue, $sharedStringIndex);
+            while ($xmlReader->name === 'si') {
+                $node = $this->getSimpleXmlElementNodeFromXMLReader($xmlReader);
+                $node->registerXPathNamespace('ns', self::MAIN_NAMESPACE_FOR_SHARED_STRINGS_XML);
 
-            $sharedStringIndex++;
+                // removes nodes that should not be read, like the pronunciation of the Kanji characters
+                $cleanNode = $this->removeSuperfluousTextNodes($node);
 
-            // jump to the next 'si' tag
-            $xmlReader->next('si');
+                // find all text nodes 't'; there can be multiple if the cell contains formatting
+                $textNodes = $cleanNode->xpath('//ns:t');
+
+                $textValue = '';
+                foreach ($textNodes as $textNode) {
+                    if ($this->shouldPreserveWhitespace($textNode)) {
+                        $textValue .= $textNode->__toString();
+                    } else {
+                        $textValue .= trim($textNode->__toString());
+                    }
+                }
+
+                $unescapedTextValue = $escaper->unescape($textValue);
+                $this->cachingStrategy->addStringForIndex($unescapedTextValue, $sharedStringIndex);
+
+                $sharedStringIndex++;
+
+                // jump to the next 'si' tag
+                $xmlReader->next('si');
+            }
+
+        } catch (XMLProcessingException $exception) {
+            throw new IOException("The sharedStrings.xml file is invalid and cannot be read. [{$exception->getMessage()}]");
         }
 
         $this->cachingStrategy->closeCache();
@@ -134,33 +142,19 @@ class SharedStringsHelper
     /**
      * Returns the shared strings unique count, as specified in <sst> tag.
      *
-     * @param \XMLReader $xmlReader XMLReader instance
+     * @param \Box\Spout\Reader\Wrapper\XMLReader $xmlReader XMLReader instance
      * @return int Number of unique shared strings in the sharedStrings.xml file
      * @throws \Box\Spout\Common\Exception\IOException If sharedStrings.xml is invalid and can't be read
      */
     protected function getSharedStringsUniqueCount($xmlReader)
     {
-        // Use internal errors to avoid displaying lots of warning messages in case of invalid file
-        // For instance, if the file is used to perform a "Billion Laughs" or "Quadratic Blowup" attacks
-        libxml_clear_errors();
-        libxml_use_internal_errors(true);
-
         $xmlReader->next('sst');
 
         // Iterate over the "sst" elements to get the actual "sst ELEMENT" (skips any DOCTYPE)
-        while ($xmlReader->name === 'sst' && $xmlReader->nodeType !== \XMLReader::ELEMENT) {
+        while ($xmlReader->name === 'sst' && $xmlReader->nodeType !== XMLReader::ELEMENT) {
             $xmlReader->read();
         }
 
-        $readError = libxml_get_last_error();
-        if ($readError !== false) {
-            $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
-        libxml_use_internal_errors(false);
-
         return intval($xmlReader->getAttribute('uniqueCount'));
     }
 
@@ -180,29 +174,19 @@ class SharedStringsHelper
      * Returns a SimpleXMLElement node from the current node in the given XMLReader instance.
      * This is to simplify the parsing of the subtree.
      *
-     * @param \XMLReader $xmlReader
-     * @return \SimpleXMLElement
+     * @param \Box\Spout\Reader\Wrapper\XMLReader $xmlReader
+     * @return \Box\Spout\Reader\Wrapper\SimpleXMLElement
      * @throws \Box\Spout\Common\Exception\IOException If the current node cannot be read
      */
     protected function getSimpleXmlElementNodeFromXMLReader($xmlReader)
     {
-        // Use internal errors to avoid displaying lots of warning messages in case of error found in the XML node.
-        // For instance, if the file is used to perform a "Billion Laughs" or "Quadratic Blowup" attacks
-        libxml_clear_errors();
-        libxml_use_internal_errors(true);
-
         $node = null;
         try {
-            $node = new \SimpleXMLElement($xmlReader->readOuterXml());
-        } catch (\Exception $exception) {
-            $error = libxml_get_last_error();
-            libxml_use_internal_errors(false);
-
-            throw new IOException('The sharedStrings.xml file contains unreadable data [' . trim($error->message) . '].');
+            $node = new SimpleXMLElement($xmlReader->readOuterXml());
+        } catch (XMLProcessingException $exception) {
+            throw new IOException("The sharedStrings.xml file contains unreadable data [{$exception->getMessage()}].");
         }
 
-        libxml_use_internal_errors(false);
-
         return $node;
     }
 
@@ -210,8 +194,8 @@ class SharedStringsHelper
      * Removes nodes that should not be read, like the pronunciation of the Kanji characters.
      * By keeping them, their text content would be added to the read string.
      *
-     * @param \SimpleXMLElement $parentNode Parent node that may contain nodes to remove
-     * @return \SimpleXMLElement Cleaned parent node
+     * @param \Box\Spout\Reader\Wrapper\SimpleXMLElement $parentNode Parent node that may contain nodes to remove
+     * @return \Box\Spout\Reader\Wrapper\SimpleXMLElement Cleaned parent node
      */
     protected function removeSuperfluousTextNodes($parentNode)
     {
@@ -221,12 +205,7 @@ class SharedStringsHelper
 
         foreach ($tagsToRemove as $tagToRemove) {
             $xpath = '//ns:' . $tagToRemove;
-            $nodesToRemove = $parentNode->xpath($xpath);
-
-            foreach ($nodesToRemove as $nodeToRemove) {
-                // This is how to remove a node from the XML
-                unset($nodeToRemove[0]);
-            }
+            $parentNode->removeNodesMatchingXPath($xpath);
         }
 
         return $parentNode;
@@ -235,24 +214,13 @@ class SharedStringsHelper
     /**
      * If the text node has the attribute 'xml:space="preserve"', then preserve whitespace.
      *
-     * @param \SimpleXMLElement $textNode The text node element (<t>) whitespace may be preserved
+     * @param \Box\Spout\Reader\Wrapper\SimpleXMLElement $textNode The text node element (<t>) whitespace may be preserved
      * @return bool Whether whitespace should be preserved
      */
     protected function shouldPreserveWhitespace($textNode)
     {
-        $shouldPreserveWhitespace = false;
-
-        $attributes = $textNode->attributes('xml', true);
-        if ($attributes) {
-            foreach ($attributes as $attributeName => $attributeValue) {
-                if ($attributeName === 'space' && $attributeValue->__toString() === 'preserve') {
-                    $shouldPreserveWhitespace = true;
-                    break;
-                }
-            }
-        }
-
-        return $shouldPreserveWhitespace;
+        $spaceValue = $textNode->getAttribute('space', 'xml');
+        return ($spaceValue === 'preserve');
     }
 
     /**

src/Spout/Reader/XLSX/Helper/SheetHelper.php

@@ -2,6 +2,7 @@
 
 namespace Box\Spout\Reader\XLSX\Helper;
 
+use Box\Spout\Reader\Wrapper\SimpleXMLElement;
 use Box\Spout\Reader\XLSX\Sheet;
 
 /**
@@ -37,10 +38,10 @@ class SheetHelper
     /** @var \Box\Spout\Common\Helper\GlobalFunctionsHelper Helper to work with global functions */
     protected $globalFunctionsHelper;
 
-    /** @var \SimpleXMLElement XML element representing the workbook.xml.rels file */
+    /** @var \Box\Spout\Reader\Wrapper\SimpleXMLElement XML element representing the workbook.xml.rels file */
     protected $workbookXMLRelsAsXMLElement;
 
-    /** @var \SimpleXMLElement XML element representing the workbook.xml file */
+    /** @var \Box\Spout\Reader\Wrapper\SimpleXMLElement XML element representing the workbook.xml file */
     protected $workbookXMLAsXMLElement;
 
     /**
@@ -76,7 +77,7 @@ class SheetHelper
 
         for ($i = 0; $i < $numSheetNodes; $i++) {
             $sheetNode = $sheetNodes[$i];
-            $sheetDataXMLFilePath = (string) $sheetNode->attributes()->PartName;
+            $sheetDataXMLFilePath = $sheetNode->getAttribute('PartName');
 
             $sheets[] = $this->getSheetFromXML($sheetDataXMLFilePath, $i);
         }
@@ -115,15 +116,15 @@ class SheetHelper
 
         if (count($relationshipNodes) === 1) {
             $relationshipNode = $relationshipNodes[0];
-            $sheetId = (string) $relationshipNode->attributes()->Id;
+            $sheetId = $relationshipNode->getAttribute('Id');
 
             $workbookXMLElement = $this->getWorkbookXMLAsXMLElement();
             $sheetNodes = $workbookXMLElement->xpath('//ns:sheet[@r:id="' . $sheetId . '"]');
 
             if (count($sheetNodes) === 1) {
                 $sheetNode = $sheetNodes[0];
-                $sheetId = (int) $sheetNode->attributes()->sheetId;
-                $escapedSheetName = (string) $sheetNode->attributes()->name;
+                $sheetId = (int) $sheetNode->getAttribute('sheetId');
+                $escapedSheetName = $sheetNode->getAttribute('name');
 
                 $escaper = new \Box\Spout\Common\Escaper\XLSX();
                 $sheetName = $escaper->unescape($escapedSheetName);
@@ -149,7 +150,7 @@ class SheetHelper
      * Returns a representation of the workbook.xml.rels file, ready to be parsed.
      * The returned value is cached.
      *
-     * @return \SimpleXMLElement XML element representating the workbook.xml.rels file
+     * @return \Box\Spout\Reader\Wrapper\SimpleXMLElement XML element representating the workbook.xml.rels file
      */
     protected function getWorkbookXMLRelsAsXMLElement()
     {
@@ -167,7 +168,7 @@ class SheetHelper
      * Returns a representation of the workbook.xml file, ready to be parsed.
      * The returned value is cached.
      *
-     * @return \SimpleXMLElement XML element representating the workbook.xml.rels file
+     * @return \Box\Spout\Reader\Wrapper\SimpleXMLElement XML element representating the workbook.xml.rels file
      */
     protected function getWorkbookXMLAsXMLElement()
     {
@@ -186,13 +187,13 @@ class SheetHelper
      *
      * @param string $xmlFilePath The path of the XML file inside the XLSX file
      * @param string $mainNamespace The main XPath namespace to register
-     * @return \SimpleXMLElement The XML element representing the file
+     * @return \Box\Spout\Reader\Wrapper\SimpleXMLElement The XML element representing the file
      */
     protected function getFileAsXMLElementWithNamespace($xmlFilePath, $mainNamespace)
     {
         $xmlContents = $this->globalFunctionsHelper->file_get_contents('zip://' . $this->filePath . '#' . $xmlFilePath);
 
-        $xmlElement = new \SimpleXMLElement($xmlContents);
+        $xmlElement = new SimpleXMLElement($xmlContents);
         $xmlElement->registerXPathNamespace('ns', $mainNamespace);
 
         return $xmlElement;

src/Spout/Reader/XLSX/Reader.php

@@ -61,7 +61,7 @@ class Reader extends AbstractReader
 
             $this->sheetIterator = new SheetIterator($filePath, $this->sharedStringsHelper, $this->globalFunctionsHelper);
         } else {
-            throw new IOException('Could not open ' . $filePath . ' for reading.');
+            throw new IOException("Could not open $filePath for reading.");
         }
     }
 

src/Spout/Reader/XLSX/RowIterator.php

@@ -3,7 +3,9 @@
 namespace Box\Spout\Reader\XLSX;
 
 use Box\Spout\Common\Exception\IOException;
+use Box\Spout\Reader\Exception\XMLProcessingException;
 use Box\Spout\Reader\IteratorInterface;
+use Box\Spout\Reader\Wrapper\XMLReader;
 use Box\Spout\Reader\XLSX\Helper\CellHelper;
 
 /**
@@ -45,7 +47,7 @@ class RowIterator implements IteratorInterface
     /** @var Helper\SharedStringsHelper Helper to work with shared strings */
     protected $sharedStringsHelper;
 
-    /** @var \XMLReader The XMLReader object that will help read sheet's XML data */
+    /** @var \Box\Spout\Reader\Wrapper\XMLReader The XMLReader object that will help read sheet's XML data */
     protected $xmlReader;
 
     /** @var \Box\Spout\Common\Escaper\XLSX Used to unescape XML data */
@@ -74,7 +76,7 @@ class RowIterator implements IteratorInterface
         $this->sheetDataXMLFilePath = $this->normalizeSheetDataXMLFilePath($sheetDataXMLFilePath);
         $this->sharedStringsHelper = $sharedStringsHelper;
 
-        $this->xmlReader = new \XMLReader();
+        $this->xmlReader = new XMLReader();
         $this->escaper = new \Box\Spout\Common\Escaper\XLSX();
     }
 
@@ -102,8 +104,8 @@ class RowIterator implements IteratorInterface
         $this->xmlReader->close();
 
         $sheetDataFilePath = 'zip://' . $this->filePath . '#' . $this->sheetDataXMLFilePath;
-        if ($this->xmlReader->open($sheetDataFilePath, null, LIBXML_NONET) === false) {
-            throw new IOException('Could not open "' . $this->sheetDataXMLFilePath . '".');
+        if ($this->xmlReader->open($sheetDataFilePath) === false) {
+            throw new IOException("Could not open \"{$this->sheetDataXMLFilePath}\".");
         }
 
         $this->numReadRows = 0;
@@ -138,59 +140,52 @@ class RowIterator implements IteratorInterface
         $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);
+        try {
+            while ($this->xmlReader->read()) {
+                if ($this->xmlReader->nodeType == XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_DIMENSION) {
+                    // Read dimensions of the sheet
+                    $dimensionRef = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_REF); // returns 'A1:M13' for instance (or 'A1' for empty sheet)
+                    if (preg_match('/[A-Z\d]+:([A-Z\d]+)/', $dimensionRef, $matches)) {
+                        $lastCellIndex = $matches[1];
+                        $this->numColumns = CellHelper::getColumnIndexFromCellIndex($lastCellIndex) + 1;
+                    }
 
-        while ($this->xmlReader->read()) {
-            if ($this->xmlReader->nodeType == \XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_DIMENSION) {
-                // Read dimensions of the sheet
-                $dimensionRef = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_REF); // returns 'A1:M13' for instance (or 'A1' for empty sheet)
-                if (preg_match('/[A-Z\d]+:([A-Z\d]+)/', $dimensionRef, $matches)) {
-                    $lastCellIndex = $matches[1];
-                    $this->numColumns = CellHelper::getColumnIndexFromCellIndex($lastCellIndex) + 1;
+                } else if ($this->xmlReader->nodeType == XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_ROW) {
+                    // Start of the row description
+                    $isInsideRowTag = true;
+
+                    // Read spans info if present
+                    $numberOfColumnsForRow = $this->numColumns;
+                    $spans = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_SPANS); // returns '1:5' for instance
+                    if ($spans) {
+                        list(, $numberOfColumnsForRow) = explode(':', $spans);
+                        $numberOfColumnsForRow = intval($numberOfColumnsForRow);
+                    }
+                    $rowData = ($numberOfColumnsForRow !== 0) ? array_fill(0, $numberOfColumnsForRow, '') : [];
+
+                } else if ($isInsideRowTag && $this->xmlReader->nodeType == XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_CELL) {
+                    // Start of a cell description
+                    $currentCellIndex = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_CELL_INDEX);
+                    $currentColumnIndex = CellHelper::getColumnIndexFromCellIndex($currentCellIndex);
+
+                    $node = $this->xmlReader->expand();
+                    $rowData[$currentColumnIndex] = $this->getCellValue($node);
+
+                } else if ($this->xmlReader->nodeType == XMLReader::END_ELEMENT && $this->xmlReader->name === self::XML_NODE_ROW) {
+                    // End of the row description
+                    // If needed, we fill the empty cells
+                    $rowData = ($this->numColumns !== 0) ? $rowData : CellHelper::fillMissingArrayIndexes($rowData);
+                    $this->numReadRows++;
+                    break;
+
+                } else if ($this->xmlReader->nodeType == XMLReader::END_ELEMENT && $this->xmlReader->name === self::XML_NODE_WORKSHEET) {
+                    // The closing "</worksheet>" marks the end of the file
+                    $this->hasReachedEndOfFile = true;
                 }
-
-            } else if ($this->xmlReader->nodeType == \XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_ROW) {
-                // Start of the row description
-                $isInsideRowTag = true;
-
-                // Read spans info if present
-                $numberOfColumnsForRow = $this->numColumns;
-                $spans = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_SPANS); // returns '1:5' for instance
-                if ($spans) {
-                    list(, $numberOfColumnsForRow) = explode(':', $spans);
-                    $numberOfColumnsForRow = intval($numberOfColumnsForRow);
-                }
-                $rowData = ($numberOfColumnsForRow !== 0) ? array_fill(0, $numberOfColumnsForRow, '') : [];
-
-            } else if ($isInsideRowTag && $this->xmlReader->nodeType == \XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_CELL) {
-                // Start of a cell description
-                $currentCellIndex = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_CELL_INDEX);
-                $currentColumnIndex = CellHelper::getColumnIndexFromCellIndex($currentCellIndex);
-
-                $node = $this->xmlReader->expand();
-                $rowData[$currentColumnIndex] = $this->getCellValue($node);
-
-            } else if ($this->xmlReader->nodeType == \XMLReader::END_ELEMENT && $this->xmlReader->name === self::XML_NODE_ROW) {
-                // End of the row description
-                // If needed, we fill the empty cells
-                $rowData = ($this->numColumns !== 0) ? $rowData : CellHelper::fillMissingArrayIndexes($rowData);
-                $this->numReadRows++;
-                break;
-
-            } else if ($this->xmlReader->nodeType == \XMLReader::END_ELEMENT && $this->xmlReader->name === self::XML_NODE_WORKSHEET) {
-                // The closing "</worksheet>" marks the end of the file
-                $this->hasReachedEndOfFile = true;
             }
-        }
 
-        $readError = libxml_get_last_error();
-        if ($readError !== false) {
-            $readErrorMessage = trim($readError->message);
-            throw new IOException("The {$this->sheetDataXMLFilePath} file cannot be read. [{$readErrorMessage}]");
+        } catch (XMLProcessingException $exception) {
+            throw new IOException("The {$this->sheetDataXMLFilePath} file cannot be read. [{$exception->getMessage()}]");
         }
 
         $this->rowDataBuffer = $rowData;