From c708411529f39c79f2965e71ddbf84141523179c Mon Sep 17 00:00:00 2001
From: MarkBaker <mark@lange.demon.co.uk>
Date: Sun, 25 Nov 2018 12:14:54 +0100
Subject: [PATCH] Refactor scanner into base reader class

---
 src/PhpSpreadsheet/Reader/BaseReader.php      | 14 +++++++++++++
 src/PhpSpreadsheet/Reader/Gnumeric.php        |  5 -----
 src/PhpSpreadsheet/Reader/Html.php            |  6 ------
 src/PhpSpreadsheet/Reader/Ods.php             |  5 -----
 .../Reader/Security/XmlScanner.php            | 10 +++++++++
 src/PhpSpreadsheet/Reader/Xlsx.php            |  5 -----
 src/PhpSpreadsheet/Reader/Xml.php             |  5 -----
 .../Reader/Security/XmlScannerTest.php        | 21 +++++++++++++++++++
 8 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/src/PhpSpreadsheet/Reader/BaseReader.php b/src/PhpSpreadsheet/Reader/BaseReader.php
index 59a5a9d8..f1e9b1d1 100644
--- a/src/PhpSpreadsheet/Reader/BaseReader.php
+++ b/src/PhpSpreadsheet/Reader/BaseReader.php
@@ -2,6 +2,7 @@
 
 namespace PhpOffice\PhpSpreadsheet\Reader;
 
+use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
 use PhpOffice\PhpSpreadsheet\Shared\File;
 
 abstract class BaseReader implements IReader
@@ -49,6 +50,11 @@ abstract class BaseReader implements IReader
 
     protected $fileHandle;
 
+    /**
+     * @var XmlScanner
+     */
+    protected $securityScanner;
+
     /**
      * Read data only?
      *        If this is true, then the Reader will only read data values for cells, it will not read any formatting information.
@@ -204,6 +210,14 @@ abstract class BaseReader implements IReader
         return $this;
     }
 
+    public function getSecuritySCanner()
+    {
+        if (property_exists($this, 'securityScanner')) {
+            return $this->securityScanner;
+        }
+        return null;
+    }
+
     /**
      * Open file for reading.
      *
diff --git a/src/PhpSpreadsheet/Reader/Gnumeric.php b/src/PhpSpreadsheet/Reader/Gnumeric.php
index 8837e7da..16f1925a 100644
--- a/src/PhpSpreadsheet/Reader/Gnumeric.php
+++ b/src/PhpSpreadsheet/Reader/Gnumeric.php
@@ -31,11 +31,6 @@ class Gnumeric extends BaseReader
 
     private $referenceHelper;
 
-    /**
-     * @var XmlScanner
-     */
-    private $securityScanner;
-
     /**
      * Create a new Gnumeric.
      */
diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php
index e6543592..1ce65347 100644
--- a/src/PhpSpreadsheet/Reader/Html.php
+++ b/src/PhpSpreadsheet/Reader/Html.php
@@ -7,7 +7,6 @@ use DOMElement;
 use DOMNode;
 use DOMText;
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
-use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Style\Border;
 use PhpOffice\PhpSpreadsheet\Style\Color;
@@ -17,11 +16,6 @@ use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
 /** PhpSpreadsheet root directory */
 class Html extends BaseReader
 {
-    /**
-     * @var XmlScanner
-     */
-    private $securityScanner;
-
     /**
      * Sample size to read to determine if it's HTML or not.
      */
diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php
index 80fab980..2fb4b7f8 100644
--- a/src/PhpSpreadsheet/Reader/Ods.php
+++ b/src/PhpSpreadsheet/Reader/Ods.php
@@ -20,11 +20,6 @@ use ZipArchive;
 
 class Ods extends BaseReader
 {
-    /**
-     * @var XmlScanner
-     */
-    private $securityScanner;
-
     /**
      * Create a new Ods Reader instance.
      */
diff --git a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
index 658d3dbe..a9093cb4 100644
--- a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
+++ b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
@@ -13,8 +13,18 @@ class XmlScanner
      */
     private $libxmlDisableEntityLoader = false;
 
+    /**
+     * Store the initial setting of libxmlDisableEntityLoader so that we can resore t later.
+     *
+     * @var bool
+     */
     private $previousLibxmlDisableEntityLoaderValue;
 
+    /**
+     * String used to identify risky xml elements.
+     *
+     * @var string
+     */
     private $pattern;
 
     private function __construct($pattern = '<!DOCTYPE')
diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
index 806a6d4f..eb527475 100644
--- a/src/PhpSpreadsheet/Reader/Xlsx.php
+++ b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -47,11 +47,6 @@ class Xlsx extends BaseReader
      */
     private static $theme = null;
 
-    /**
-     * @var XmlScanner
-     */
-    private $securityScanner;
-
     /**
      * Create a new Xlsx Reader instance.
      */
diff --git a/src/PhpSpreadsheet/Reader/Xml.php b/src/PhpSpreadsheet/Reader/Xml.php
index 3781c427..2b7959f4 100644
--- a/src/PhpSpreadsheet/Reader/Xml.php
+++ b/src/PhpSpreadsheet/Reader/Xml.php
@@ -36,11 +36,6 @@ class Xml extends BaseReader
      */
     protected $charSet = 'UTF-8';
 
-    /**
-     * @var XmlScanner
-     */
-    private $securityScanner;
-
     /**
      * Create a new Excel2003XML Reader instance.
      */
diff --git a/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php b/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php
index 8eaa56f8..ef2126ee 100644
--- a/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php
+++ b/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php
@@ -3,6 +3,8 @@
 namespace PhpOffice\PhpSpreadsheetTests\Reader\Security;
 
 use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
+use PhpOffice\PhpSpreadsheet\Reader\Xls;
 use PHPUnit\Framework\TestCase;
 
 class XmlScannerTest extends TestCase
@@ -54,4 +56,23 @@ class XmlScannerTest extends TestCase
 
         return $tests;
     }
+
+    public function testGetSecurityScannerForXmlBasedReader()
+    {
+        $fileReader = new Xlsx();
+        $scanner = $fileReader->getSecuritySCanner();
+
+        //    Must return an object...
+        $this->assertTrue(is_object($scanner));
+        //    ... of the correct type
+        $this->assertInstanceOf(XmlScanner::class, $scanner);
+    }
+
+    public function testGetSecurityScannerForNonXmlBasedReader()
+    {
+        $fileReader = new Xls();
+        $scanner = $fileReader->getSecuritySCanner();
+        //    Must return a null...
+        $this->assertNull($scanner);
+    }
 }