Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1a26d28

Browse filesBrowse files
fabpotfabpot
committed
bug #10493 [2.3] Fix libxml_use_internal_errors and libxml_disable_entity_loader usage (romainneutron)
This PR was merged into the 2.3 branch. Discussion ---------- [2.3] Fix libxml_use_internal_errors and libxml_disable_entity_loader usage | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #9731, #9483 | License | MIT This should fix #9731 and #9483 that seems to be triggered when `libxml_disable_entity_loader` has been called with `true` (see https://bugs.php.net/bug.php?id=62577) As `libxml_disable_entity_loader` is non thread safe (see https://bugs.php.net/bug.php?id=64938) and as we have some calls that might leave the setting to `true`, I think the bug should be fixed. I've checked the use of both `libxml_use_internal_errors` and `libxml_disable_entity_loader` among symfony code. You can see I prefered to skip DomDocument::loadXML warnings using the `@` instead of using `LIBXML_NOERROR | LIBXML_NO_WARNING` because we can log these errors whereas libxml errors would be furtives. - [x] Check calls to DOMDocument::load - [x] Check calls to DOMDocument::loadXML - [x] Check calls to DOMDocument::loadHTML - [x] Check calls to DOMDocument::loadHTMLFile - [x] Add more tests Commits ------- 489b8ae Fix libxml_use_internal_errors and libxml_disable_entity_loader usage
2 parents 88f670c + 489b8ae commit 1a26d28
Copy full SHA for 1a26d28

File tree

Expand file treeCollapse file tree

12 files changed

+106
-62
lines changed
Filter options
Expand file treeCollapse file tree

12 files changed

+106
-62
lines changed

‎src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php
+39Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,45 @@ public function getDataForPhpize()
130130
array('foo', 'foo'),
131131
);
132132
}
133+
134+
public function testLoadEmptyXmlFile()
135+
{
136+
$file = __DIR__.'/../Fixtures/foo.xml';
137+
$this->setExpectedException('InvalidArgumentException', 'File '.$file.' does not contain valid XML, it is empty.');
138+
XmlUtils::loadFile($file);
139+
}
140+
141+
// test for issue https://github.com/symfony/symfony/issues/9731
142+
public function testLoadWrongEmptyXMLWithErrorHandler()
143+
{
144+
$originalDisableEntities = libxml_disable_entity_loader(false);
145+
$errorReporting = error_reporting(-1);
146+
147+
set_error_handler(function ($errno, $errstr) {
148+
throw new \Exception($errstr, $errno);
149+
});
150+
151+
$file = __DIR__.'/../Fixtures/foo.xml';
152+
try {
153+
XmlUtils::loadFile($file);
154+
$this->fail('An exception should have been raised');
155+
} catch (\InvalidArgumentException $e) {
156+
$this->assertEquals(sprintf('File %s does not contain valid XML, it is empty.', $file), $e->getMessage());
157+
}
158+
159+
restore_error_handler();
160+
error_reporting($errorReporting);
161+
162+
$disableEntities = libxml_disable_entity_loader(true);
163+
libxml_disable_entity_loader($disableEntities);
164+
165+
libxml_disable_entity_loader($originalDisableEntities);
166+
167+
$this->assertFalse($disableEntities);
168+
169+
// should not throw an exception
170+
XmlUtils::loadFile(__DIR__.'/../Fixtures/Util/valid.xml', __DIR__.'/../Fixtures/Util/schema.xsd');
171+
}
133172
}
134173

135174
interface Validator

‎src/Symfony/Component/Config/Util/XmlUtils.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Config/Util/XmlUtils.php
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,18 @@ private function __construct()
4040
*/
4141
public static function loadFile($file, $schemaOrCallable = null)
4242
{
43+
$content = @file_get_contents($file);
44+
if ('' === trim($content)) {
45+
throw new \InvalidArgumentException(sprintf('File %s does not contain valid XML, it is empty.', $file));
46+
}
47+
4348
$internalErrors = libxml_use_internal_errors(true);
4449
$disableEntities = libxml_disable_entity_loader(true);
4550
libxml_clear_errors();
4651

4752
$dom = new \DOMDocument();
4853
$dom->validateOnParse = true;
49-
if (!$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) {
54+
if (!$dom->loadXML($content, LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) {
5055
libxml_disable_entity_loader($disableEntities);
5156

5257
throw new \InvalidArgumentException(implode("\n", static::getXmlErrors($internalErrors)));

‎src/Symfony/Component/CssSelector/Tests/XPath/TranslatorTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/CssSelector/Tests/XPath/TranslatorTest.php
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function testHtmlIds($css, array $elementsId)
4949
$translator->registerExtension(new HtmlExtension($translator));
5050
$document = new \DOMDocument();
5151
$document->strictErrorChecking = false;
52-
libxml_use_internal_errors(true);
52+
$internalErrors = libxml_use_internal_errors(true);
5353
$document->loadHTMLFile(__DIR__.'/Fixtures/ids.html');
5454
$document = simplexml_import_dom($document);
5555
$elements = $document->xpath($translator->cssToXPath($css));
@@ -59,6 +59,8 @@ public function testHtmlIds($css, array $elementsId)
5959
$this->assertTrue(in_array($element->attributes()->id, $elementsId));
6060
}
6161
}
62+
libxml_clear_errors();
63+
libxml_use_internal_errors($internalErrors);
6264
}
6365

6466
/** @dataProvider getHtmlShakespearTestData */

‎src/Symfony/Component/DomCrawler/Crawler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DomCrawler/Crawler.php
+11-7Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public function addContent($content, $type = null)
141141
*/
142142
public function addHtmlContent($content, $charset = 'UTF-8')
143143
{
144-
$current = libxml_use_internal_errors(true);
144+
$internalErrors = libxml_use_internal_errors(true);
145145
$disableEntities = libxml_disable_entity_loader(true);
146146

147147
$dom = new \DOMDocument('1.0', $charset);
@@ -161,9 +161,11 @@ public function addHtmlContent($content, $charset = 'UTF-8')
161161
}
162162
}
163163

164-
@$dom->loadHTML($content);
164+
if ('' !== trim($content)) {
165+
@$dom->loadHTML($content);
166+
}
165167

166-
libxml_use_internal_errors($current);
168+
libxml_use_internal_errors($internalErrors);
167169
libxml_disable_entity_loader($disableEntities);
168170

169171
$this->addDocument($dom);
@@ -200,16 +202,18 @@ public function addHtmlContent($content, $charset = 'UTF-8')
200202
*/
201203
public function addXmlContent($content, $charset = 'UTF-8')
202204
{
203-
$current = libxml_use_internal_errors(true);
205+
$internalErrors = libxml_use_internal_errors(true);
204206
$disableEntities = libxml_disable_entity_loader(true);
205207

206208
$dom = new \DOMDocument('1.0', $charset);
207209
$dom->validateOnParse = true;
208210

209-
// remove the default namespace to make XPath expressions simpler
210-
@$dom->loadXML(str_replace('xmlns', 'ns', $content), LIBXML_NONET);
211+
if ('' !== trim($content)) {
212+
// remove the default namespace to make XPath expressions simpler
213+
@$dom->loadXML(str_replace('xmlns', 'ns', $content), LIBXML_NONET);
214+
}
211215

212-
libxml_use_internal_errors($current);
216+
libxml_use_internal_errors($internalErrors);
213217
libxml_disable_entity_loader($disableEntities);
214218

215219
$this->addDocument($dom);

‎src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public function testAddHtmlContentCharsetGbk()
129129
*/
130130
public function testAddHtmlContentWithErrors()
131131
{
132-
libxml_use_internal_errors(true);
132+
$internalErrors = libxml_use_internal_errors(true);
133133

134134
$crawler = new Crawler();
135135
$crawler->addHtmlContent(<<<EOF
@@ -149,7 +149,7 @@ public function testAddHtmlContentWithErrors()
149149
$this->assertEquals("Tag nav invalid\n", $errors[0]->message);
150150

151151
libxml_clear_errors();
152-
libxml_use_internal_errors(false);
152+
libxml_use_internal_errors($internalErrors);
153153
}
154154

155155
/**
@@ -179,7 +179,7 @@ public function testAddXmlContentCharset()
179179
*/
180180
public function testAddXmlContentWithErrors()
181181
{
182-
libxml_use_internal_errors(true);
182+
$internalErrors = libxml_use_internal_errors(true);
183183

184184
$crawler = new Crawler();
185185
$crawler->addXmlContent(<<<EOF
@@ -197,7 +197,7 @@ public function testAddXmlContentWithErrors()
197197
$this->assertTrue(count(libxml_get_errors()) > 1);
198198

199199
libxml_clear_errors();
200-
libxml_use_internal_errors(false);
200+
libxml_use_internal_errors($internalErrors);
201201
}
202202

203203
/**

‎src/Symfony/Component/Serializer/Encoder/XmlEncoder.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Serializer/Encoder/XmlEncoder.php
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ public function encode($data, $format, array $context = array())
6666
*/
6767
public function decode($data, $format, array $context = array())
6868
{
69+
if ('' === trim($data)) {
70+
throw new UnexpectedValueException('Invalid XML data, it can not be empty.');
71+
}
72+
6973
$internalErrors = libxml_use_internal_errors(true);
7074
$disableEntities = libxml_disable_entity_loader(true);
7175
libxml_clear_errors();
@@ -77,6 +81,8 @@ public function decode($data, $format, array $context = array())
7781
libxml_disable_entity_loader($disableEntities);
7882

7983
if ($error = libxml_get_last_error()) {
84+
libxml_clear_errors();
85+
8086
throw new UnexpectedValueException($error->message);
8187
}
8288

‎src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,12 @@ public function testPreventsComplexExternalEntities()
330330
}
331331
}
332332

333+
public function testDecodeEmptyXml()
334+
{
335+
$this->setExpectedException('Symfony\Component\Serializer\Exception\UnexpectedValueException', 'Invalid XML data, it can not be empty.');
336+
$this->encoder->decode(' ', 'xml');
337+
}
338+
333339
protected function getXmlSource()
334340
{
335341
return '<?xml version="1.0"?>'."\n".

‎src/Symfony/Component/Translation/Loader/QtFileLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Translation/Loader/QtFileLoader.php
+9-30Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Translation\Loader;
1313

14+
use Symfony\Component\Config\Util\XmlUtils;
1415
use Symfony\Component\Translation\MessageCatalogue;
1516
use Symfony\Component\Translation\Exception\InvalidResourceException;
1617
use Symfony\Component\Translation\Exception\NotFoundResourceException;
@@ -40,12 +41,15 @@ public function load($resource, $locale, $domain = 'messages')
4041
throw new NotFoundResourceException(sprintf('File "%s" not found.', $resource));
4142
}
4243

43-
$dom = new \DOMDocument();
44-
$current = libxml_use_internal_errors(true);
45-
if (!@$dom->load($resource, defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0)) {
46-
throw new InvalidResourceException(implode("\n", $this->getXmlErrors()));
44+
try {
45+
$dom = XmlUtils::loadFile($resource);
46+
} catch (\InvalidArgumentException $e) {
47+
throw new InvalidResourceException(sprintf('Unable to load "%s".', $resource), $e->getCode(), $e);
4748
}
4849

50+
$internalErrors = libxml_use_internal_errors(true);
51+
libxml_clear_errors();
52+
4953
$xpath = new \DOMXPath($dom);
5054
$nodes = $xpath->evaluate('//TS/context/name[text()="'.$domain.'"]');
5155

@@ -67,33 +71,8 @@ public function load($resource, $locale, $domain = 'messages')
6771
$catalogue->addResource(new FileResource($resource));
6872
}
6973

70-
libxml_use_internal_errors($current);
74+
libxml_use_internal_errors($internalErrors);
7175

7276
return $catalogue;
7377
}
74-
75-
/**
76-
* Returns the XML errors of the internal XML parser
77-
*
78-
* @return array An array of errors
79-
*/
80-
private function getXmlErrors()
81-
{
82-
$errors = array();
83-
foreach (libxml_get_errors() as $error) {
84-
$errors[] = sprintf('[%s %s] %s (in %s - line %d, column %d)',
85-
LIBXML_ERR_WARNING == $error->level ? 'WARNING' : 'ERROR',
86-
$error->code,
87-
trim($error->message),
88-
$error->file ? $error->file : 'n/a',
89-
$error->line,
90-
$error->column
91-
);
92-
}
93-
94-
libxml_clear_errors();
95-
libxml_use_internal_errors(false);
96-
97-
return $errors;
98-
}
9978
}

‎src/Symfony/Component/Translation/Loader/XliffFileLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Translation/Loader/XliffFileLoader.php
+6-19Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Translation\Loader;
1313

14+
use Symfony\Component\Config\Util\XmlUtils;
1415
use Symfony\Component\Translation\MessageCatalogue;
1516
use Symfony\Component\Translation\Exception\InvalidResourceException;
1617
use Symfony\Component\Translation\Exception\NotFoundResourceException;
@@ -86,27 +87,13 @@ public function load($resource, $locale, $domain = 'messages')
8687
*/
8788
private function parseFile($file)
8889
{
89-
$internalErrors = libxml_use_internal_errors(true);
90-
$disableEntities = libxml_disable_entity_loader(true);
91-
libxml_clear_errors();
92-
93-
$dom = new \DOMDocument();
94-
$dom->validateOnParse = true;
95-
if (!@$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) {
96-
libxml_disable_entity_loader($disableEntities);
97-
98-
throw new InvalidResourceException(implode("\n", $this->getXmlErrors($internalErrors)));
90+
try {
91+
$dom = XmlUtils::loadFile($file);
92+
} catch (\InvalidArgumentException $e) {
93+
throw new InvalidResourceException(sprintf('Unable to load "%s": %s', $file, $e->getMessage()), $e->getCode(), $e);
9994
}
10095

101-
libxml_disable_entity_loader($disableEntities);
102-
103-
foreach ($dom->childNodes as $child) {
104-
if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) {
105-
libxml_use_internal_errors($internalErrors);
106-
107-
throw new InvalidResourceException('Document types are not allowed.');
108-
}
109-
}
96+
$internalErrors = libxml_use_internal_errors(true);
11097

11198
$location = str_replace('\\', '/', __DIR__).'/schema/dic/xliff-core/xml.xsd';
11299
$parts = explode('/', $location);

‎src/Symfony/Component/Translation/Tests/Loader/QtFileLoaderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Translation/Tests/Loader/QtFileLoaderTest.php
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,12 @@ public function testLoadInvalidResource()
6363
$resource = __DIR__.'/../fixtures/invalid-xml-resources.xlf';
6464
$loader->load($resource, 'en', 'domain1');
6565
}
66+
67+
public function testLoadEmptyResource()
68+
{
69+
$loader = new QtFileLoader();
70+
$resource = __DIR__.'/../fixtures/empty.xlf';
71+
$this->setExpectedException('Symfony\Component\Translation\Exception\InvalidResourceException', sprintf('Unable to load "%s".', $resource));
72+
$loader->load($resource, 'en', 'domain1');
73+
}
6674
}

‎src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,12 @@ public function testDocTypeIsNotAllowed()
110110
$loader = new XliffFileLoader();
111111
$loader->load(__DIR__.'/../fixtures/withdoctype.xlf', 'en', 'domain1');
112112
}
113+
114+
public function testParseEmptyFile()
115+
{
116+
$loader = new XliffFileLoader();
117+
$resource = __DIR__.'/../fixtures/empty.xlf';
118+
$this->setExpectedException('Symfony\Component\Translation\Exception\InvalidResourceException', sprintf('Unable to load "%s":', $resource));
119+
$loader->load($resource, 'en', 'domain1');
120+
}
113121
}

‎src/Symfony/Component/Translation/Tests/fixtures/empty.xlf

Copy file name to clipboardExpand all lines: src/Symfony/Component/Translation/Tests/fixtures/empty.xlf
Whitespace-only changes.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.