From 489b8aea905a3242df2612d3726b941521be15e7 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Wed, 19 Mar 2014 16:07:56 +0100 Subject: [PATCH] Fix libxml_use_internal_errors and libxml_disable_entity_loader usage --- .../Config/Tests/Util/XmlUtilsTest.php | 39 +++++++++++++++++++ .../Component/Config/Util/XmlUtils.php | 7 +++- .../Tests/XPath/TranslatorTest.php | 4 +- src/Symfony/Component/DomCrawler/Crawler.php | 18 +++++---- .../DomCrawler/Tests/CrawlerTest.php | 8 ++-- .../Serializer/Encoder/XmlEncoder.php | 6 +++ .../Tests/Encoder/XmlEncoderTest.php | 6 +++ .../Translation/Loader/QtFileLoader.php | 39 +++++-------------- .../Translation/Loader/XliffFileLoader.php | 27 ++++--------- .../Tests/Loader/QtFileLoaderTest.php | 8 ++++ .../Tests/Loader/XliffFileLoaderTest.php | 8 ++++ .../Translation/Tests/fixtures/empty.xlf | 0 12 files changed, 107 insertions(+), 63 deletions(-) create mode 100644 src/Symfony/Component/Translation/Tests/fixtures/empty.xlf diff --git a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php index 722c4db144..d10863a922 100644 --- a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php +++ b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php @@ -130,6 +130,45 @@ class XmlUtilsTest extends \PHPUnit_Framework_TestCase array('foo', 'foo'), ); } + + public function testLoadEmptyXmlFile() + { + $file = __DIR__.'/../Fixtures/foo.xml'; + $this->setExpectedException('InvalidArgumentException', 'File '.$file.' does not contain valid XML, it is empty.'); + XmlUtils::loadFile($file); + } + + // test for issue https://github.com/symfony/symfony/issues/9731 + public function testLoadWrongEmptyXMLWithErrorHandler() + { + $originalDisableEntities = libxml_disable_entity_loader(false); + $errorReporting = error_reporting(-1); + + set_error_handler(function ($errno, $errstr) { + throw new \Exception($errstr, $errno); + }); + + $file = __DIR__.'/../Fixtures/foo.xml'; + try { + XmlUtils::loadFile($file); + $this->fail('An exception should have been raised'); + } catch (\InvalidArgumentException $e) { + $this->assertEquals(sprintf('File %s does not contain valid XML, it is empty.', $file), $e->getMessage()); + } + + restore_error_handler(); + error_reporting($errorReporting); + + $disableEntities = libxml_disable_entity_loader(true); + libxml_disable_entity_loader($disableEntities); + + libxml_disable_entity_loader($originalDisableEntities); + + $this->assertFalse($disableEntities); + + // should not throw an exception + XmlUtils::loadFile(__DIR__.'/../Fixtures/Util/valid.xml', __DIR__.'/../Fixtures/Util/schema.xsd'); + } } interface Validator diff --git a/src/Symfony/Component/Config/Util/XmlUtils.php b/src/Symfony/Component/Config/Util/XmlUtils.php index e7c0bf79d4..f5e0324f54 100644 --- a/src/Symfony/Component/Config/Util/XmlUtils.php +++ b/src/Symfony/Component/Config/Util/XmlUtils.php @@ -40,13 +40,18 @@ class XmlUtils */ public static function loadFile($file, $schemaOrCallable = null) { + $content = @file_get_contents($file); + if ('' === trim($content)) { + throw new \InvalidArgumentException(sprintf('File %s does not contain valid XML, it is empty.', $file)); + } + $internalErrors = libxml_use_internal_errors(true); $disableEntities = libxml_disable_entity_loader(true); libxml_clear_errors(); $dom = new \DOMDocument(); $dom->validateOnParse = true; - if (!$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + if (!$dom->loadXML($content, LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { libxml_disable_entity_loader($disableEntities); throw new \InvalidArgumentException(implode("\n", static::getXmlErrors($internalErrors))); diff --git a/src/Symfony/Component/CssSelector/Tests/XPath/TranslatorTest.php b/src/Symfony/Component/CssSelector/Tests/XPath/TranslatorTest.php index 7a90c3b69b..30f7189f06 100644 --- a/src/Symfony/Component/CssSelector/Tests/XPath/TranslatorTest.php +++ b/src/Symfony/Component/CssSelector/Tests/XPath/TranslatorTest.php @@ -49,7 +49,7 @@ class TranslatorTest extends \PHPUnit_Framework_TestCase $translator->registerExtension(new HtmlExtension($translator)); $document = new \DOMDocument(); $document->strictErrorChecking = false; - libxml_use_internal_errors(true); + $internalErrors = libxml_use_internal_errors(true); $document->loadHTMLFile(__DIR__.'/Fixtures/ids.html'); $document = simplexml_import_dom($document); $elements = $document->xpath($translator->cssToXPath($css)); @@ -59,6 +59,8 @@ class TranslatorTest extends \PHPUnit_Framework_TestCase $this->assertTrue(in_array($element->attributes()->id, $elementsId)); } } + libxml_clear_errors(); + libxml_use_internal_errors($internalErrors); } /** @dataProvider getHtmlShakespearTestData */ diff --git a/src/Symfony/Component/DomCrawler/Crawler.php b/src/Symfony/Component/DomCrawler/Crawler.php index c3954946ad..b6c852e691 100644 --- a/src/Symfony/Component/DomCrawler/Crawler.php +++ b/src/Symfony/Component/DomCrawler/Crawler.php @@ -141,7 +141,7 @@ class Crawler extends \SplObjectStorage */ public function addHtmlContent($content, $charset = 'UTF-8') { - $current = libxml_use_internal_errors(true); + $internalErrors = libxml_use_internal_errors(true); $disableEntities = libxml_disable_entity_loader(true); $dom = new \DOMDocument('1.0', $charset); @@ -161,9 +161,11 @@ class Crawler extends \SplObjectStorage } } - @$dom->loadHTML($content); + if ('' !== trim($content)) { + @$dom->loadHTML($content); + } - libxml_use_internal_errors($current); + libxml_use_internal_errors($internalErrors); libxml_disable_entity_loader($disableEntities); $this->addDocument($dom); @@ -200,16 +202,18 @@ class Crawler extends \SplObjectStorage */ public function addXmlContent($content, $charset = 'UTF-8') { - $current = libxml_use_internal_errors(true); + $internalErrors = libxml_use_internal_errors(true); $disableEntities = libxml_disable_entity_loader(true); $dom = new \DOMDocument('1.0', $charset); $dom->validateOnParse = true; - // remove the default namespace to make XPath expressions simpler - @$dom->loadXML(str_replace('xmlns', 'ns', $content), LIBXML_NONET); + if ('' !== trim($content)) { + // remove the default namespace to make XPath expressions simpler + @$dom->loadXML(str_replace('xmlns', 'ns', $content), LIBXML_NONET); + } - libxml_use_internal_errors($current); + libxml_use_internal_errors($internalErrors); libxml_disable_entity_loader($disableEntities); $this->addDocument($dom); diff --git a/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php b/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php index 1c22398025..07a08d0d04 100644 --- a/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php +++ b/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php @@ -129,7 +129,7 @@ class CrawlerTest extends \PHPUnit_Framework_TestCase */ public function testAddHtmlContentWithErrors() { - libxml_use_internal_errors(true); + $internalErrors = libxml_use_internal_errors(true); $crawler = new Crawler(); $crawler->addHtmlContent(<<assertEquals("Tag nav invalid\n", $errors[0]->message); libxml_clear_errors(); - libxml_use_internal_errors(false); + libxml_use_internal_errors($internalErrors); } /** @@ -179,7 +179,7 @@ EOF */ public function testAddXmlContentWithErrors() { - libxml_use_internal_errors(true); + $internalErrors = libxml_use_internal_errors(true); $crawler = new Crawler(); $crawler->addXmlContent(<<assertTrue(count(libxml_get_errors()) > 1); libxml_clear_errors(); - libxml_use_internal_errors(false); + libxml_use_internal_errors($internalErrors); } /** diff --git a/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php b/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php index fe66628521..33630079f6 100644 --- a/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php +++ b/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php @@ -66,6 +66,10 @@ class XmlEncoder extends SerializerAwareEncoder implements EncoderInterface, Dec */ public function decode($data, $format, array $context = array()) { + if ('' === trim($data)) { + throw new UnexpectedValueException('Invalid XML data, it can not be empty.'); + } + $internalErrors = libxml_use_internal_errors(true); $disableEntities = libxml_disable_entity_loader(true); libxml_clear_errors(); @@ -77,6 +81,8 @@ class XmlEncoder extends SerializerAwareEncoder implements EncoderInterface, Dec libxml_disable_entity_loader($disableEntities); if ($error = libxml_get_last_error()) { + libxml_clear_errors(); + throw new UnexpectedValueException($error->message); } diff --git a/src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php b/src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php index 2540254779..73dee7d021 100644 --- a/src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php +++ b/src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php @@ -330,6 +330,12 @@ class XmlEncoderTest extends \PHPUnit_Framework_TestCase } } + public function testDecodeEmptyXml() + { + $this->setExpectedException('Symfony\Component\Serializer\Exception\UnexpectedValueException', 'Invalid XML data, it can not be empty.'); + $this->encoder->decode(' ', 'xml'); + } + protected function getXmlSource() { return ''."\n". diff --git a/src/Symfony/Component/Translation/Loader/QtFileLoader.php b/src/Symfony/Component/Translation/Loader/QtFileLoader.php index dbb56daec6..aacfb4a55e 100644 --- a/src/Symfony/Component/Translation/Loader/QtFileLoader.php +++ b/src/Symfony/Component/Translation/Loader/QtFileLoader.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Translation\Loader; +use Symfony\Component\Config\Util\XmlUtils; use Symfony\Component\Translation\MessageCatalogue; use Symfony\Component\Translation\Exception\InvalidResourceException; use Symfony\Component\Translation\Exception\NotFoundResourceException; @@ -40,12 +41,15 @@ class QtFileLoader implements LoaderInterface throw new NotFoundResourceException(sprintf('File "%s" not found.', $resource)); } - $dom = new \DOMDocument(); - $current = libxml_use_internal_errors(true); - if (!@$dom->load($resource, defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0)) { - throw new InvalidResourceException(implode("\n", $this->getXmlErrors())); + try { + $dom = XmlUtils::loadFile($resource); + } catch (\InvalidArgumentException $e) { + throw new InvalidResourceException(sprintf('Unable to load "%s".', $resource), $e->getCode(), $e); } + $internalErrors = libxml_use_internal_errors(true); + libxml_clear_errors(); + $xpath = new \DOMXPath($dom); $nodes = $xpath->evaluate('//TS/context/name[text()="'.$domain.'"]'); @@ -67,33 +71,8 @@ class QtFileLoader implements LoaderInterface $catalogue->addResource(new FileResource($resource)); } - libxml_use_internal_errors($current); + libxml_use_internal_errors($internalErrors); return $catalogue; } - - /** - * Returns the XML errors of the internal XML parser - * - * @return array An array of errors - */ - private function getXmlErrors() - { - $errors = array(); - foreach (libxml_get_errors() as $error) { - $errors[] = sprintf('[%s %s] %s (in %s - line %d, column %d)', - LIBXML_ERR_WARNING == $error->level ? 'WARNING' : 'ERROR', - $error->code, - trim($error->message), - $error->file ? $error->file : 'n/a', - $error->line, - $error->column - ); - } - - libxml_clear_errors(); - libxml_use_internal_errors(false); - - return $errors; - } } diff --git a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php index 0cafc043ac..f46b5dfee4 100644 --- a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php +++ b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Translation\Loader; +use Symfony\Component\Config\Util\XmlUtils; use Symfony\Component\Translation\MessageCatalogue; use Symfony\Component\Translation\Exception\InvalidResourceException; use Symfony\Component\Translation\Exception\NotFoundResourceException; @@ -86,27 +87,13 @@ class XliffFileLoader implements LoaderInterface */ private function parseFile($file) { + try { + $dom = XmlUtils::loadFile($file); + } catch (\InvalidArgumentException $e) { + throw new InvalidResourceException(sprintf('Unable to load "%s": %s', $file, $e->getMessage()), $e->getCode(), $e); + } + $internalErrors = libxml_use_internal_errors(true); - $disableEntities = libxml_disable_entity_loader(true); - libxml_clear_errors(); - - $dom = new \DOMDocument(); - $dom->validateOnParse = true; - if (!@$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { - libxml_disable_entity_loader($disableEntities); - - throw new InvalidResourceException(implode("\n", $this->getXmlErrors($internalErrors))); - } - - libxml_disable_entity_loader($disableEntities); - - foreach ($dom->childNodes as $child) { - if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) { - libxml_use_internal_errors($internalErrors); - - throw new InvalidResourceException('Document types are not allowed.'); - } - } $location = str_replace('\\', '/', __DIR__).'/schema/dic/xliff-core/xml.xsd'; $parts = explode('/', $location); diff --git a/src/Symfony/Component/Translation/Tests/Loader/QtFileLoaderTest.php b/src/Symfony/Component/Translation/Tests/Loader/QtFileLoaderTest.php index c1dd7b1058..1d7d2b99d4 100644 --- a/src/Symfony/Component/Translation/Tests/Loader/QtFileLoaderTest.php +++ b/src/Symfony/Component/Translation/Tests/Loader/QtFileLoaderTest.php @@ -63,4 +63,12 @@ class QtFileLoaderTest extends \PHPUnit_Framework_TestCase $resource = __DIR__.'/../fixtures/invalid-xml-resources.xlf'; $loader->load($resource, 'en', 'domain1'); } + + public function testLoadEmptyResource() + { + $loader = new QtFileLoader(); + $resource = __DIR__.'/../fixtures/empty.xlf'; + $this->setExpectedException('Symfony\Component\Translation\Exception\InvalidResourceException', sprintf('Unable to load "%s".', $resource)); + $loader->load($resource, 'en', 'domain1'); + } } diff --git a/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php b/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php index 1d58e0ee0f..020a8721f1 100644 --- a/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php +++ b/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php @@ -110,4 +110,12 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase $loader = new XliffFileLoader(); $loader->load(__DIR__.'/../fixtures/withdoctype.xlf', 'en', 'domain1'); } + + public function testParseEmptyFile() + { + $loader = new XliffFileLoader(); + $resource = __DIR__.'/../fixtures/empty.xlf'; + $this->setExpectedException('Symfony\Component\Translation\Exception\InvalidResourceException', sprintf('Unable to load "%s":', $resource)); + $loader->load($resource, 'en', 'domain1'); + } } diff --git a/src/Symfony/Component/Translation/Tests/fixtures/empty.xlf b/src/Symfony/Component/Translation/Tests/fixtures/empty.xlf new file mode 100644 index 0000000000..e69de29bb2