Test scrutinizer

PHPOffice · Apr 27, 2020 · b60b1d2 · b60b1d2
1 parent 11499aa
commit b60b1d2
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 23 deletions.
diff --git a/src/PhpSpreadsheet/Writer/Ods.php b/src/PhpSpreadsheet/Writer/Ods.php
@@ -93,18 +93,18 @@ public function save($pFilename)
         // garbage collect
         $this->spreadSheet->garbageCollect();
 
-        // If $pFilename is php://output or php://stdout, make it a temporary file...
         $originalFilename = $pFilename;
         if (is_resource($pFilename)) {
             $this->fileHandle = $pFilename;
-        } elseif (strtolower($pFilename) == 'php://output' || strtolower($pFilename) == 'php://stdout') {
+        } else {
             // If $pFilename is php://output or php://stdout, make it a temporary file...
-            $pFilename = @tempnam(File::sysGetTempDir(), 'phpxltmp');
-            if ($pFilename == '') {
-                $pFilename = $originalFilename;
+            if (in_array(strtolower($pFilename), ['php://output', 'php://stdout'], true)) {
+                $pFilename = @tempnam(File::sysGetTempDir(), 'phpxltmp');
+                if ($pFilename === '') {
+                    $pFilename = $originalFilename;
+                }
             }
-            $this->fileHandle = fopen($pFilename, 'wb+');
-        } else {
+
             $this->fileHandle = fopen($pFilename, 'wb+');
         }
 
@@ -128,11 +128,19 @@ public function save($pFilename)
         rewind($this->fileHandle);
 
         // If a temporary file was used, copy it to the correct file stream
-        if ($originalFilename != $pFilename) {
-            if (stream_copy_to_stream($this->fileHandle, fopen($originalFilename, 'wb+')) === false) {
+        if ($originalFilename !== $pFilename) {
+            $destinationFileHandle = fopen($originalFilename, 'wb+');
+            if (!is_resource($destinationFileHandle)) {
+                throw new WriterException("Could not open resource $originalFilename for writing.");
+            }
+
+            if (stream_copy_to_stream($this->fileHandle, $destinationFileHandle) === false) {
                 throw new WriterException("Could not copy temporary zip file $pFilename to $originalFilename.");
             }
-            @unlink($pFilename);
+
+            if (is_string($pFilename) && !unlink($pFilename)) {
+                throw new WriterException('Could not unlink temporary zip file.');
+            }
         }
     }
 
@@ -146,7 +154,7 @@ public function save($pFilename)
     private function createZip()
     {
         // Try opening the ZIP file
-        if ($this->fileHandle === false) {
+        if (!is_resource($this->fileHandle)) {
             throw new WriterException('Could not open resource for writing.');
         }
 

diff --git a/src/PhpSpreadsheet/Writer/Xlsx.php b/src/PhpSpreadsheet/Writer/Xlsx.php
@@ -185,22 +185,21 @@ public function save($pFilename)
             $this->spreadSheet->garbageCollect();
 
             $originalFilename = $pFilename;
-
             if (is_resource($pFilename)) {
                 $this->fileHandle = $pFilename;
-            } elseif (strtolower($pFilename) == 'php://output' || strtolower($pFilename) == 'php://stdout') {
+            } else {
                 // If $pFilename is php://output or php://stdout, make it a temporary file...
-                $pFilename = @tempnam(File::sysGetTempDir(), 'phpxltmp');
-                if ($pFilename == '') {
-                    $pFilename = $originalFilename;
+                if (in_array(strtolower($pFilename), ['php://output', 'php://stdout'], true)) {
+                    $pFilename = @tempnam(File::sysGetTempDir(), 'phpxltmp');
+                    if ($pFilename === '') {
+                        $pFilename = $originalFilename;
+                    }
                 }
-                $this->fileHandle = fopen($pFilename, 'wb+');
-            } else {
+
                 $this->fileHandle = fopen($pFilename, 'wb+');
             }
 
-            // Try opening the ZIP file
-            if ($this->fileHandle === false) {
+            if (!is_resource($this->fileHandle)) {
                 throw new WriterException('Could not open resource for writing.');
             }
 
@@ -428,11 +427,19 @@ public function save($pFilename)
             rewind($this->fileHandle);
 
             // If a temporary file was used, copy it to the correct file stream
-            if ($originalFilename != $pFilename) {
-                if (stream_copy_to_stream($this->fileHandle, fopen($originalFilename, 'wb+')) === false) {
+            if ($originalFilename !== $pFilename) {
+                $destinationFileHandle = fopen($originalFilename, 'wb+');
+                if (!is_resource($destinationFileHandle)) {
+                    throw new WriterException("Could not open resource $originalFilename for writing.");
+                }
+
+                if (stream_copy_to_stream($this->fileHandle, $destinationFileHandle) === false) {
                     throw new WriterException("Could not copy temporary zip file $pFilename to $originalFilename.");
                 }
-                @unlink($pFilename);
+
+                if (is_string($pFilename) && !unlink($pFilename)) {
+                    throw new WriterException('Could not unlink temporary zip file.');
+                }
             }
         } else {
             throw new WriterException('PhpSpreadsheet object unassigned.');