Skip to content
Permalink
Browse files

MDL-51863 packer: ensure empty zip files behavior remains consistent

With PHP bug #70322 fixed, ZipArchive::close() did start returning false
and throwing PHP Warnings with recent PHP versions (5.6.14 and up).
Previously (5.6.13 verified) it was returning true, and false in older
versions (5.4.x verified).

This change does silent the 2 "hacky" calls to close() that we perform
in core leaving the 3rd one (used for files having files) unmodified.

A new unit test has been created to cover the close() behavior, ideally
supporting both old and new PHP versions without harcoding any PHP
version.

Note that we don't use to rely much on results coming from close(), and
that's a good thing given the buggy behavior commented above. This just
keeps empty zips working like they were before.
  • Loading branch information...
stronk7 authored and danpoltawski committed Oct 26, 2015
1 parent 6f3a4fb commit 2ff58a3b052dbe5cf8165b60ce6f41954593b78f
Showing with 69 additions and 2 deletions.
  1. +67 −0 lib/filestorage/tests/zip_packer_test.php
  2. +2 −2 lib/filestorage/zip_archive.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
*/
@@ -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;

0 comments on commit 2ff58a3

Please sign in to comment.
You can’t perform that action at this time.