Settings: deleted libxml_disable_entity_loader() calls (#113)

Settings: deleted libxml_disable_entity_loader() calls since they're not necessary.
Prevent setLibXmlLoaderOptions() and getLibXmlLoaderOptions() to call the libxml_disable_entity_loader() function which alter the global state of libxml.
See the discussion here https://github.com/PHPOffice/PhpSpreadsheet/issues/74
This commit is contained in:
Paolo Agostinetto 2017-03-12 14:00:46 +01:00 committed by Adrien Crivelli
parent de57bf722c
commit 9767112b23
3 changed files with 15 additions and 4 deletions

View File

@ -12,8 +12,7 @@ spreadsheet files. This can lead to:
- Server Side Request Forgery - Server Side Request Forgery
- Command Execution (depending on the installed PHP wrappers) - Command Execution (depending on the installed PHP wrappers)
To prevent this, PhpSpreadsheet sets `libxml_disable_entity_loader` to To prevent this, by default every XML-based Reader looks for XML entities declared inside the DOCTYPE and if any is found an exception is raised.
`true` for the XML-based Readers by default.
## Loading a Spreadsheet File ## Loading a Spreadsheet File

View File

@ -237,7 +237,6 @@ class Settings
if (is_null($options) && defined('LIBXML_DTDLOAD')) { if (is_null($options) && defined('LIBXML_DTDLOAD')) {
$options = LIBXML_DTDLOAD | LIBXML_DTDATTR; $options = LIBXML_DTDLOAD | LIBXML_DTDATTR;
} }
@libxml_disable_entity_loader((bool) $options);
self::$libXmlLoaderOptions = $options; self::$libXmlLoaderOptions = $options;
} }
@ -254,7 +253,6 @@ class Settings
} elseif (is_null(self::$libXmlLoaderOptions)) { } elseif (is_null(self::$libXmlLoaderOptions)) {
self::$libXmlLoaderOptions = true; self::$libXmlLoaderOptions = true;
} }
@libxml_disable_entity_loader((bool) self::$libXmlLoaderOptions);
return self::$libXmlLoaderOptions; return self::$libXmlLoaderOptions;
} }

View File

@ -4,14 +4,27 @@ namespace PhpOffice\PhpSpreadsheetTests;
class SettingsTest extends \PHPUnit_Framework_TestCase class SettingsTest extends \PHPUnit_Framework_TestCase
{ {
/**
* @var string
*/
protected $prevValue;
public function setUp() public function setUp()
{ {
$this->prevValue = libxml_disable_entity_loader();
libxml_disable_entity_loader(false); // Enable entity loader
}
protected function tearDown()
{
libxml_disable_entity_loader($this->prevValue);
} }
public function testGetXMLSettings() public function testGetXMLSettings()
{ {
$result = \PhpOffice\PhpSpreadsheet\Settings::getLibXmlLoaderOptions(); $result = \PhpOffice\PhpSpreadsheet\Settings::getLibXmlLoaderOptions();
$this->assertTrue((bool) ((LIBXML_DTDLOAD | LIBXML_DTDATTR) & $result)); $this->assertTrue((bool) ((LIBXML_DTDLOAD | LIBXML_DTDATTR) & $result));
$this->assertFalse(libxml_disable_entity_loader());
} }
public function testSetXMLSettings() public function testSetXMLSettings()
@ -19,5 +32,6 @@ class SettingsTest extends \PHPUnit_Framework_TestCase
\PhpOffice\PhpSpreadsheet\Settings::setLibXmlLoaderOptions(LIBXML_DTDLOAD | LIBXML_DTDATTR | LIBXML_DTDVALID); \PhpOffice\PhpSpreadsheet\Settings::setLibXmlLoaderOptions(LIBXML_DTDLOAD | LIBXML_DTDATTR | LIBXML_DTDVALID);
$result = \PhpOffice\PhpSpreadsheet\Settings::getLibXmlLoaderOptions(); $result = \PhpOffice\PhpSpreadsheet\Settings::getLibXmlLoaderOptions();
$this->assertTrue((bool) ((LIBXML_DTDLOAD | LIBXML_DTDATTR | LIBXML_DTDVALID) & $result)); $this->assertTrue((bool) ((LIBXML_DTDLOAD | LIBXML_DTDATTR | LIBXML_DTDVALID) & $result));
$this->assertFalse(libxml_disable_entity_loader());
} }
} }