From 6ab969e9cc6cefe6772942b787f0b5d50b2c0dfa Mon Sep 17 00:00:00 2001
From: rtek <rtek.rtek.rtek@gmail.com>
Date: Wed, 3 Jul 2019 03:53:43 -0400
Subject: [PATCH] Allow XmlScanner to correctly restore libxml entity_loader
 setting (#1050)

XmlScanner was not restoring libxml_disable_entity_loader since
destruct was not being called until script shutdown. This is because
the shutdown handler required an XmlScanner instance.

Also fix an unrelated bug where the UTF-8 encoding test was
case sensitive.
---
 src/PhpSpreadsheet/Reader/BaseReader.php      | 17 +----------
 .../Reader/Security/XmlScanner.php            |  9 +++---
 .../Reader/Security/XmlScannerTest.php        | 28 +++++++++++++++++--
 3 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/src/PhpSpreadsheet/Reader/BaseReader.php b/src/PhpSpreadsheet/Reader/BaseReader.php
index 18bee1b8..f7af1557 100644
--- a/src/PhpSpreadsheet/Reader/BaseReader.php
+++ b/src/PhpSpreadsheet/Reader/BaseReader.php
@@ -58,21 +58,6 @@ abstract class BaseReader implements IReader
     public function __construct()
     {
         $this->readFilter = new DefaultReadFilter();
-
-        // A fatal error will bypass the destructor, so we register a shutdown here
-        register_shutdown_function([$this, '__destruct']);
-    }
-
-    private function shutdown()
-    {
-        if ($this->securityScanner !== null) {
-            $this->securityScanner = null;
-        }
-    }
-
-    public function __destruct()
-    {
-        $this->shutdown();
     }
 
     public function getReadDataOnly()
@@ -146,7 +131,7 @@ abstract class BaseReader implements IReader
         return $this;
     }
 
-    public function getSecuritySCanner()
+    public function getSecurityScanner()
     {
         if (property_exists($this, 'securityScanner')) {
             return $this->securityScanner;
diff --git a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
index cc90ece9..62247618 100644
--- a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
+++ b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
@@ -25,7 +25,7 @@ class XmlScanner
         $this->disableEntityLoaderCheck();
 
         // A fatal error will bypass the destructor, so we register a shutdown here
-        register_shutdown_function([$this, '__destruct']);
+        register_shutdown_function([__CLASS__, 'shutdown']);
     }
 
     public static function getInstance(Reader\IReader $reader)
@@ -72,16 +72,17 @@ class XmlScanner
         }
     }
 
-    private function shutdown()
+    public static function shutdown()
     {
         if (self::$libxmlDisableEntityLoaderValue !== null) {
             libxml_disable_entity_loader(self::$libxmlDisableEntityLoaderValue);
+            self::$libxmlDisableEntityLoaderValue = null;
         }
     }
 
     public function __destruct()
     {
-        $this->shutdown();
+        self::shutdown();
     }
 
     public function setAdditionalCallback(callable $callback)
@@ -93,7 +94,7 @@ class XmlScanner
     {
         $pattern = '/encoding="(.*?)"/';
         $result = preg_match($pattern, $xml, $matches);
-        $charset = $result ? $matches[1] : 'UTF-8';
+        $charset = strtoupper($result ? $matches[1] : 'UTF-8');
 
         if ($charset !== 'UTF-8') {
             $xml = mb_convert_encoding($xml, 'UTF-8', $charset);
diff --git a/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php b/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php
index 473e47da..c1d7fa38 100644
--- a/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php
+++ b/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php
@@ -9,6 +9,11 @@ use PHPUnit\Framework\TestCase;
 
 class XmlScannerTest extends TestCase
 {
+    protected function setUp()
+    {
+        libxml_disable_entity_loader(false);
+    }
+
     /**
      * @dataProvider providerValidXML
      *
@@ -74,7 +79,7 @@ class XmlScannerTest extends TestCase
     public function testGetSecurityScannerForXmlBasedReader()
     {
         $fileReader = new Xlsx();
-        $scanner = $fileReader->getSecuritySCanner();
+        $scanner = $fileReader->getSecurityScanner();
 
         //    Must return an object...
         $this->assertInternalType('object', $scanner);
@@ -85,7 +90,7 @@ class XmlScannerTest extends TestCase
     public function testGetSecurityScannerForNonXmlBasedReader()
     {
         $fileReader = new Xls();
-        $scanner = $fileReader->getSecuritySCanner();
+        $scanner = $fileReader->getSecurityScanner();
         //    Must return a null...
         $this->assertNull($scanner);
     }
@@ -99,7 +104,7 @@ class XmlScannerTest extends TestCase
     public function testSecurityScanWithCallback($filename, $expectedResult)
     {
         $fileReader = new Xlsx();
-        $scanner = $fileReader->getSecuritySCanner();
+        $scanner = $fileReader->getSecurityScanner();
         $scanner->setAdditionalCallback('strrev');
         $xml = $scanner->scanFile($filename);
 
@@ -115,4 +120,21 @@ class XmlScannerTest extends TestCase
 
         return $tests;
     }
+
+    public function testLibxmlDisableEntityLoaderIsRestoredWithoutShutdown()
+    {
+        $reader = new Xlsx();
+        unset($reader);
+
+        $reader = new \XMLReader();
+        $opened = $reader->open(__DIR__ . '/../../../data/Reader/Xml/SecurityScannerWithCallbackExample.xml');
+        $this->assertTrue($opened);
+    }
+
+    public function testEncodingAllowsMixedCase()
+    {
+        $scanner = new XmlScanner();
+        $output = $scanner->scan($input = '<?xml version="1.0" encoding="utf-8"?><foo>bar</foo>');
+        $this->assertSame($input, $output);
+    }
 }