mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-18 22:08:20 +01:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Eloy Lafuente (stronk7) 2015-10-26 17:59:40 +01:00
parent 9382ac38d6
commit 70ae75674d
2 changed files with 69 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
lib/filestorage/tests/zip_packer_test.php
View File

@ -376,6 +376,73 @@ class core_files_zip_packer_testcase extends advanced_testcase implements file_p
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
*/

4
lib/filestorage/zip_archive.php
View File

@ -191,7 +191,7 @@ class zip_archive extends file_archive {
}
if ($this->emptyziphack) {
$this->za->close();
@$this->za->close();
$this->za = null;
$this->mode = null;
$this->namelookup = null;
@ -202,7 +202,7 @@ class zip_archive extends file_archive {
} 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;