From f74fde155f84c41ae4bb6533813d6c85c7e2c6d2 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Sun, 27 Nov 2016 14:29:25 +0900 Subject: [PATCH] Remove non-obvious BaseReader's dependency BaseReader called `isValidFormat` despite it doesn't require it to be implemented. Instead we replaced `isValidFormat` by a proper implementation of `canRead` in each classes. This also remove the semantic amibguity between those two methods. FIX #32 --- src/PhpSpreadsheet/Reader/BaseReader.php | 22 ------------- src/PhpSpreadsheet/Reader/CSV.php | 41 +++++++++++++++--------- src/PhpSpreadsheet/Reader/HTML.php | 36 ++++++++++----------- src/PhpSpreadsheet/Reader/SYLK.php | 33 +++++++++++-------- 4 files changed, 61 insertions(+), 71 deletions(-) diff --git a/src/PhpSpreadsheet/Reader/BaseReader.php b/src/PhpSpreadsheet/Reader/BaseReader.php index 5e3362ac..2efa31c1 100644 --- a/src/PhpSpreadsheet/Reader/BaseReader.php +++ b/src/PhpSpreadsheet/Reader/BaseReader.php @@ -244,28 +244,6 @@ abstract class BaseReader implements IReader } } - /** - * Can the current IReader read the file? - * - * @param string $pFilename - * @throws Exception - * @return bool - */ - public function canRead($pFilename) - { - // Check if file exists - try { - $this->openFile($pFilename); - } catch (Exception $e) { - return false; - } - - $readable = $this->isValidFormat(); - fclose($this->fileHandle); - - return $readable; - } - /** * Scan theXML for use of readFilter = new DefaultReadFilter(); } - /** - * Validate that the current file is a CSV file - * - * @return bool - */ - protected function isValidFormat() - { - return true; - } - /** * Set input encoding * @@ -171,11 +161,10 @@ class CSV extends BaseReader implements IReader public function listWorksheetInfo($pFilename) { // Open file - $this->openFile($pFilename); - if (!$this->isValidFormat()) { - fclose($this->fileHandle); + if (!$this->canRead($pFilename)) { throw new Exception($pFilename . ' is an Invalid Spreadsheet file.'); } + $this->openFile($pFilename); $fileHandle = $this->fileHandle; // Skip BOM, if any @@ -236,11 +225,10 @@ class CSV extends BaseReader implements IReader ini_set('auto_detect_line_endings', true); // Open file - $this->openFile($pFilename); - if (!$this->isValidFormat()) { - fclose($this->fileHandle); + if (!$this->canRead($pFilename)) { throw new Exception($pFilename . ' is an Invalid Spreadsheet file.'); } + $this->openFile($pFilename); $fileHandle = $this->fileHandle; // Skip BOM, if any @@ -394,4 +382,25 @@ class CSV extends BaseReader implements IReader { return $this->contiguous; } + + /** + * Can the current IReader read the file? + * + * @param string $pFilename + * @throws Exception + * @return bool + */ + public function canRead($pFilename) + { + // Check if file exists + try { + $this->openFile($pFilename); + } catch (Exception $e) { + return false; + } + + fclose($this->fileHandle); + + return true; + } } diff --git a/src/PhpSpreadsheet/Reader/HTML.php b/src/PhpSpreadsheet/Reader/HTML.php index 983a3937..a84dd738 100644 --- a/src/PhpSpreadsheet/Reader/HTML.php +++ b/src/PhpSpreadsheet/Reader/HTML.php @@ -127,25 +127,27 @@ class HTML extends BaseReader implements IReader /** * Validate that the current file is an HTML file * + * @param string $pFilename + * @throws Exception * @return bool */ - protected function isValidFormat() + public function canRead($pFilename) { + // Check if file exists + try { + $this->openFile($pFilename); + } catch (Exception $e) { + return false; + } + $beginning = $this->readBeginning(); + $startWithTag = self::startsWithTag($beginning); + $containsTags = self::containsTags($beginning); + $endsWithTag = self::endsWithTag($this->readEnding()); - if (!self::startsWithTag($beginning)) { - return false; - } + fclose($this->fileHandle); - if (!self::containsTags($beginning)) { - return false; - } - - if (!self::endsWithTag($this->readEnding())) { - return false; - } - - return true; + return $startWithTag && $containsTags && $endsWithTag; } private function readBeginning() @@ -492,14 +494,10 @@ class HTML extends BaseReader implements IReader */ public function loadIntoExisting($pFilename, Spreadsheet $spreadsheet) { - // Open file to validate - $this->openFile($pFilename); - if (!$this->isValidFormat()) { - fclose($this->fileHandle); + // Validate + if (!$this->canRead($pFilename)) { throw new Exception($pFilename . ' is an Invalid HTML file.'); } - // Close after validating - fclose($this->fileHandle); // Create new sheet while ($spreadsheet->getSheetCount() <= $this->sheetIndex) { diff --git a/src/PhpSpreadsheet/Reader/SYLK.php b/src/PhpSpreadsheet/Reader/SYLK.php index 4832e892..aa9ed591 100644 --- a/src/PhpSpreadsheet/Reader/SYLK.php +++ b/src/PhpSpreadsheet/Reader/SYLK.php @@ -65,26 +65,33 @@ class SYLK extends BaseReader implements IReader /** * Validate that the current file is a SYLK file * + * @param string $pFilename + * @throws Exception * @return bool */ - protected function isValidFormat() + public function canRead($pFilename) { + // Check if file exists + try { + $this->openFile($pFilename); + } catch (Exception $e) { + return false; + } + // Read sample data (first 2 KB will do) $data = fread($this->fileHandle, 2048); // Count delimiters in file $delimiterCount = substr_count($data, ';'); - if ($delimiterCount < 1) { - return false; - } + $hasDelimiter = $delimiterCount > 0; // Analyze first line looking for ID; signature $lines = explode("\n", $data); - if (substr($lines[0], 0, 4) != 'ID;P') { - return false; - } + $hasId = substr($lines[0], 0, 4) === 'ID;P'; - return true; + fclose($this->fileHandle); + + return $hasDelimiter && $hasId; } /** @@ -118,11 +125,10 @@ class SYLK extends BaseReader implements IReader public function listWorksheetInfo($pFilename) { // Open file - $this->openFile($pFilename); - if (!$this->isValidFormat()) { - fclose($this->fileHandle); + if (!$this->canRead($pFilename)) { throw new Exception($pFilename . ' is an Invalid Spreadsheet file.'); } + $this->openFile($pFilename); $fileHandle = $this->fileHandle; rewind($fileHandle); @@ -205,11 +211,10 @@ class SYLK extends BaseReader implements IReader public function loadIntoExisting($pFilename, \PhpOffice\PhpSpreadsheet\Spreadsheet $spreadsheet) { // Open file - $this->openFile($pFilename); - if (!$this->isValidFormat()) { - fclose($this->fileHandle); + if (!$this->canRead($pFilename)) { throw new Exception($pFilename . ' is an Invalid Spreadsheet file.'); } + $this->openFile($pFilename); $fileHandle = $this->fileHandle; rewind($fileHandle);