Merge branch 'MDL-51863' of git://github.com/stronk7/moodle

ralferlebach · Oct 27, 2015 · 61257ae · 61257ae
2 parents 101df9a + 70ae756
commit 61257ae
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 2 deletions.
diff --git a/lib/filestorage/tests/zip_packer_test.php b/lib/filestorage/tests/zip_packer_test.php
@@ -376,6 +376,73 @@ public function test_add_files() {
         unlink($archive);
     }
 
+    public function test_close_archive() {
+        global $CFG;
+
+        $this->resetAfterTest(true);
+
+        $archive = "$CFG->tempdir/archive.zip";
+        $textfile = "$CFG->tempdir/textfile.txt";
+        touch($textfile);
+
+        $this->assertFileNotExists($archive);
+        $this->assertFileExists($textfile);
+
+        // Create archive and close it without files.
+        // (returns true, without any warning).
+        $zip_archive = new zip_archive();
+        $result = $zip_archive->open($archive, file_archive::CREATE);
+        $this->assertTrue($result);
+        $result = $zip_archive->close();
+        $this->assertTrue($result);
+        unlink($archive);
+
+        // Create archive and close it with files.
+        // (returns true, without any warning).
+        $zip_archive = new zip_archive();
+        $result = $zip_archive->open($archive, file_archive::CREATE);
+        $this->assertTrue($result);
+        $result = $zip_archive->add_file_from_string('test.txt', 'test');
+        $this->assertTrue($result);
+        $result = $zip_archive->add_file_from_pathname('test2.txt', $textfile);
+        $result = $zip_archive->close();
+        $this->assertTrue($result);
+        unlink($archive);
+
+        // Create archive and close if forcing error.
+        // (returns true for old PHP versions and
+        // false with warnings for new PHP versions). MDL-51863.
+        $zip_archive = new zip_archive();
+        $result = $zip_archive->open($archive, file_archive::CREATE);
+        $this->assertTrue($result);
+        $result = $zip_archive->add_file_from_string('test.txt', 'test');
+        $this->assertTrue($result);
+        $result = $zip_archive->add_file_from_pathname('test2.txt', $textfile);
+        $this->assertTrue($result);
+        // Delete the file before closing does force close() to fail.
+        unlink($textfile);
+        // Behavior is different between old PHP versions and new ones. Let's detect it.
+        $result = false;
+        try {
+            // Old PHP versions were not printing any warning.
+            $result = $zip_archive->close();
+        } catch (Exception $e) {
+            // New PHP versions print PHP Warning.
+            $this->assertInstanceOf('PHPUnit_Framework_Error_Warning', $e);
+            $this->assertContains('ZipArchive::close', $e->getMessage());
+        }
+        // This is crazy, but it shows how some PHP versions do return true.
+        try {
+            // And some PHP versions do return correctly false (5.4.25, 5.6.14...)
+            $this->assertFalse($result);
+        } catch (Exception $e) {
+            // But others do insist into returning true (5.6.13...). Only can accept them.
+            $this->assertInstanceOf('PHPUnit_Framework_ExpectationFailedException', $e);
+            $this->assertTrue($result);
+        }
+        $this->assertFileNotExists($archive);
+    }
+
     /**
      * @depends test_add_files
      */

diff --git a/lib/filestorage/zip_archive.php b/lib/filestorage/zip_archive.php
@@ -191,7 +191,7 @@ public function close() {
         }
 
         if ($this->emptyziphack) {
-            $this->za->close();
+            @$this->za->close();
             $this->za = null;
             $this->mode = null;
             $this->namelookup = null;
@@ -202,7 +202,7 @@ public function close() {
 
         } else if ($this->za->numFiles == 0) {
             // PHP can not create empty archives, so let's fake it.
-            $this->za->close();
+            @$this->za->close();
             $this->za = null;
             $this->mode = null;
             $this->namelookup = null;