Skip to content

Commit

Permalink
standardized the way we handle XML errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabpot
fabpot committed Aug 28, 2012
1 parent 352e8f5 commit 865461d
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 26 deletions.
19 changes: 13 additions & 6 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Expand Up @@ -211,14 +211,18 @@ private function parseDefinition($id, $service, $file)
*/
private function parseFile($file)
{
$internalErrors = libxml_use_internal_errors(true);
libxml_clear_errors();

$dom = new \DOMDocument();
libxml_use_internal_errors(true);
$dom->validateOnParse = true;
if (!$dom->load($file, defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0)) {
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors()));
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors($internalErrors)));
}
$dom->validateOnParse = true;
$dom->normalizeDocument();
libxml_use_internal_errors(false);

libxml_use_internal_errors($internalErrors);

$this->validate($dom, $file);

return simplexml_import_dom($dom, 'Symfony\\Component\\DependencyInjection\\SimpleXMLElement');
Expand Down Expand Up @@ -360,12 +364,14 @@ private function validateSchema(\DOMDocument $dom, $file)
;

$current = libxml_use_internal_errors(true);
libxml_clear_errors();

$valid = $dom->schemaValidateSource($source);
foreach ($tmpfiles as $tmpfile) {
@unlink($tmpfile);
}
if (!$valid) {
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors()));
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors($current)));
}
libxml_use_internal_errors($current);
}
Expand Down Expand Up @@ -406,7 +412,7 @@ private function validateExtensions(\DOMDocument $dom, $file)
*
* @return array
*/
private function getXmlErrors()
private function getXmlErrors($internalErrors)
{
$errors = array();
foreach (libxml_get_errors() as $error) {
Expand All @@ -421,6 +427,7 @@ private function getXmlErrors()
}

libxml_clear_errors();
libxml_use_internal_errors($internalErrors);

return $errors;
}
Expand Down
19 changes: 13 additions & 6 deletions src/Symfony/Component/Routing/Loader/XmlFileLoader.php
View file
Expand Up @@ -150,14 +150,18 @@ protected function parseRoute(RouteCollection $collection, \DOMElement $definiti
*/
protected function loadFile($file)
{
$internalErrors = libxml_use_internal_errors(true);
libxml_clear_errors();

$dom = new \DOMDocument();
libxml_use_internal_errors(true);
$dom->validateOnParse = true;
if (!$dom->load($file, defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0)) {
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors()));
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors($internalErrors)));
}
$dom->validateOnParse = true;
$dom->normalizeDocument();
libxml_use_internal_errors(false);

libxml_use_internal_errors($internalErrors);

$this->validate($dom);

return $dom;
Expand All @@ -175,8 +179,10 @@ protected function validate(\DOMDocument $dom)
$location = __DIR__.'/schema/routing/routing-1.0.xsd';

$current = libxml_use_internal_errors(true);
libxml_clear_errors();

if (!$dom->schemaValidate($location)) {
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors()));
throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors($current)));
}
libxml_use_internal_errors($current);
}
Expand All @@ -186,7 +192,7 @@ protected function validate(\DOMDocument $dom)
*
* @return array An array of libxml error strings
*/
private function getXmlErrors()
private function getXmlErrors($internalErrors)
{
$errors = array();
foreach (libxml_get_errors() as $error) {
Expand All @@ -201,6 +207,7 @@ private function getXmlErrors()
}

libxml_clear_errors();
libxml_use_internal_errors($internalErrors);

return $errors;
}
Expand Down
17 changes: 10 additions & 7 deletions src/Symfony/Component/Translation/Loader/XliffFileLoader.php
View file
Expand Up @@ -55,10 +55,13 @@ public function load($resource, $locale, $domain = 'messages')
*/
private function parseFile($file)
{
$internalErrors = libxml_use_internal_errors(true);
libxml_clear_errors();

$dom = new \DOMDocument();
$current = libxml_use_internal_errors(true);
$dom->validateOnParse = true;
if (!@$dom->load($file, defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0)) {
throw new \RuntimeException(implode("\n", $this->getXmlErrors()));
throw new \RuntimeException(implode("\n", $this->getXmlErrors($internalErrors)));
}

$location = str_replace('\\', '/', __DIR__).'/schema/dic/xliff-core/xml.xsd';
Expand All @@ -77,11 +80,11 @@ private function parseFile($file)
$source = str_replace('http://www.w3.org/2001/xml.xsd', $location, $source);

if (!@$dom->schemaValidateSource($source)) {
throw new \RuntimeException(implode("\n", $this->getXmlErrors()));
throw new \RuntimeException(implode("\n", $this->getXmlErrors($internalErrors)));
}
$dom->validateOnParse = true;
$dom->normalizeDocument();
libxml_use_internal_errors($current);

libxml_use_internal_errors($internalErrors);

return simplexml_import_dom($dom);
}
Expand All @@ -91,7 +94,7 @@ private function parseFile($file)
*
* @return array An array of errors
*/
private function getXmlErrors()
private function getXmlErrors($internalErrors)
{
$errors = array();
foreach (libxml_get_errors() as $error) {
Expand All @@ -106,7 +109,7 @@ private function getXmlErrors()
}

libxml_clear_errors();
libxml_use_internal_errors(false);
libxml_use_internal_errors($internalErrors);

return $errors;
}
Expand Down
17 changes: 10 additions & 7 deletions src/Symfony/Component/Validator/Mapping/Loader/XmlFileLoader.php
View file
Expand Up @@ -180,22 +180,25 @@ protected function parseOptions(\SimpleXMLElement $nodes)
*/
protected function parseFile($file)
{
$internalErrors = libxml_use_internal_errors(true);
libxml_clear_errors();

$dom = new \DOMDocument();
libxml_use_internal_errors(true);
$dom->validateOnParse = true;
if (!$dom->load($file, defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0)) {
throw new MappingException(implode("\n", $this->getXmlErrors()));
throw new MappingException(implode("\n", $this->getXmlErrors($internalErrors)));
}
if (!$dom->schemaValidate(__DIR__.'/schema/dic/constraint-mapping/constraint-mapping-1.0.xsd')) {
throw new MappingException(implode("\n", $this->getXmlErrors()));
throw new MappingException(implode("\n", $this->getXmlErrors($internalErrors)));
}
$dom->validateOnParse = true;
$dom->normalizeDocument();
libxml_use_internal_errors(false);

libxml_use_internal_errors($internalErrors);

return simplexml_import_dom($dom);
}

protected function getXmlErrors()
protected function getXmlErrors($internalErrors)
{
$errors = array();
foreach (libxml_get_errors() as $error) {
Expand All @@ -210,7 +213,7 @@ protected function getXmlErrors()
}

libxml_clear_errors();
libxml_use_internal_errors(false);
libxml_use_internal_errors($internalErrors);

return $errors;
}
Expand Down

2 comments on commit 865461d

@stof
Copy link
Member

@stof stof commented on 865461d Aug 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot what about adding a base xml loader in the Config component instead of duplicating all the XML security handling in each component using the loaders ? This would avoid the code duplication in 4 different components (and they already depend on a base class in the Config component anyway)

@fabpot
Copy link
Member Author

@fabpot fabpot commented on 865461d Aug 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've discussed this possibility with others, but I first wanted to fix the security issues. The refactoring can happen later and probably not before Symfony 2.2 anyway.

Please sign in to comment.