diff --git a/lib/filestorage/file_archive.php b/lib/filestorage/file_archive.php index d27d2c6a6da..1aa688d6f0e 100644 --- a/lib/filestorage/file_archive.php +++ b/lib/filestorage/file_archive.php @@ -47,7 +47,7 @@ abstract class file_archive implements Iterator { protected $encoding = 'utf-8'; /** - * Open or create archive (depending on $mode) + * Open or create archive (depending on $mode). * * @param string $archivepathname archive path name * @param int $mode OPEN, CREATE or OVERWRITE constant @@ -57,14 +57,14 @@ abstract class file_archive implements Iterator { public abstract function open($archivepathname, $mode=file_archive::CREATE, $encoding='utf-8'); /** - * Close archive + * Close archive. * * @return bool success */ public abstract function close(); /** - * Returns file stream for reading of content + * Returns file stream for reading of content. * * @param int $index index of file * @return stream|bool stream or false if error @@ -72,7 +72,7 @@ abstract class file_archive implements Iterator { public abstract function get_stream($index); /** - * Returns file information + * Returns file information. * * @param int $index index of file * @return stdClass|bool object or false if error @@ -80,21 +80,21 @@ abstract class file_archive implements Iterator { public abstract function get_info($index); /** - * Returns array of info about all files in archive + * Returns array of info about all files in archive. * * @return array of file infos */ public abstract function list_files(); /** - * Returns number of files in archive + * Returns number of files in archive. * * @return int number of files */ public abstract function count(); /** - * Add file into archive + * Add file into archive. * * @param string $localname name of file in archive * @param string $pathname location of file @@ -103,7 +103,7 @@ abstract class file_archive implements Iterator { public abstract function add_file_from_pathname($localname, $pathname); /** - * Add content of string into archive + * Add content of string into archive. * * @param string $localname name of file in archive * @param string $contents contents @@ -112,7 +112,7 @@ abstract class file_archive implements Iterator { public abstract function add_file_from_string($localname, $contents); /** - * Add empty directory into archive + * Add empty directory into archive. * * @param string $localname name of file in archive * @return bool success @@ -182,25 +182,25 @@ abstract class file_archive implements Iterator { } /** - * Returns current file info + * Returns current file info. * @return object */ //public abstract function current(); /** - * Returns the index of current file + * Returns the index of current file. * @return int current file index */ //public abstract function key(); /** - * Moves forward to next file + * Moves forward to next file. * @return void */ //public abstract function next(); /** - * Rewinds back to the first file + * Rewinds back to the first file. * @return void */ //public abstract function rewind(); diff --git a/lib/filestorage/file_exceptions.php b/lib/filestorage/file_exceptions.php index 425e3040a0f..92e39e5dbe1 100644 --- a/lib/filestorage/file_exceptions.php +++ b/lib/filestorage/file_exceptions.php @@ -25,7 +25,7 @@ defined('MOODLE_INTERNAL') || die(); /** - * Basic file related exception class + * Basic file related exception class. * * @package core_files * @category files @@ -46,7 +46,7 @@ class file_exception extends moodle_exception { } /** - * Can not create file exception + * Can not create file exception. * * @package core_files * @category files @@ -55,7 +55,7 @@ class file_exception extends moodle_exception { */ class stored_file_creation_exception extends file_exception { /** - * Constructor + * Constructor. * * @param int $contextid context ID * @param string $component component @@ -87,7 +87,7 @@ class stored_file_creation_exception extends file_exception { */ class file_access_exception extends file_exception { /** - * Constructor + * Constructor. * * @param string $debuginfo extra debug info */ @@ -106,7 +106,7 @@ class file_access_exception extends file_exception { */ class file_pool_content_exception extends file_exception { /** - * Constructor + * Constructor. * * @param string $contenthash content hash * @param string $debuginfo extra debug info @@ -118,7 +118,7 @@ class file_pool_content_exception extends file_exception { /** - * Problem with records in the {files_reference} table + * Problem with records in the {files_reference} table. * * @package core_files * @catehory files @@ -127,7 +127,7 @@ class file_pool_content_exception extends file_exception { */ class file_reference_exception extends file_exception { /** - * Constructor + * Constructor. * * @param int $repositoryid the id of the repository that provides the referenced file * @param string $reference the information for the repository to locate the file diff --git a/lib/filestorage/file_packer.php b/lib/filestorage/file_packer.php index 76aeead83ca..3fbfb626677 100644 --- a/lib/filestorage/file_packer.php +++ b/lib/filestorage/file_packer.php @@ -14,7 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . - /** * Abstraction of general file packer. * @@ -35,7 +34,7 @@ defined('MOODLE_INTERNAL') || die(); abstract class file_packer { /** - * Archive files and store the result in file storage + * Archive files and store the result in file storage. * * @param array $files array with zip paths as keys (archivepath=>ospathname or archivepath=>stored_file) * @param int $contextid context ID @@ -45,21 +44,23 @@ abstract class file_packer { * @param string $filepath file path * @param string $filename file name * @param int $userid user ID - * @return stored_file|bool false if error stored file instance if ok + * @param bool $ignoreinvalidfiles true means ignore missing or invalid files, false means abort on any error + * @return stored_file|bool false if error stored_file instance if ok */ - public abstract function archive_to_storage($files, $contextid, $component, $filearea, $itemid, $filepath, $filename, $userid = NULL); + public abstract function archive_to_storage(array $files, $contextid, $component, $filearea, $itemid, $filepath, $filename, $userid = NULL, $ignoreinvalidfiles=true); /** - * Archive files and store the result in os file + * Archive files and store the result in os file. * * @param array $files array with zip paths as keys (archivepath=>ospathname or archivepath=>stored_file) * @param string $archivefile path to target zip file - * @return bool success + * @param bool $ignoreinvalidfiles true means ignore missing or invalid files, false means abort on any error + * @return bool true if file created, false if not */ - public abstract function archive_to_pathname($files, $archivefile); + public abstract function archive_to_pathname(array $files, $archivefile, $ignoreinvalidfiles=true); /** - * Extract file to given file path (real OS filesystem), existing files are overwrited + * Extract file to given file path (real OS filesystem), existing files are overwritten. * * @param stored_file|string $archivefile full pathname of zip file or stored_file instance * @param string $pathname target directory @@ -69,7 +70,7 @@ abstract class file_packer { public abstract function extract_to_pathname($archivefile, $pathname, array $onlyfiles = NULL); /** - * Extract file to given file path (real OS filesystem), existing files are overwrited + * Extract file to given file path (real OS filesystem), existing files are overwritten. * * @param string|stored_file $archivefile full pathname of zip file or stored_file instance * @param int $contextid context ID @@ -83,7 +84,7 @@ abstract class file_packer { public abstract function extract_to_storage($archivefile, $contextid, $component, $filearea, $itemid, $pathbase, $userid = NULL); /** - * Returns array of info about all files in archive + * Returns array of info about all files in archive. * * @param string|file_archive $archivefile * @return array of file infos diff --git a/lib/filestorage/tests/fixtures/empty.zip b/lib/filestorage/tests/fixtures/empty.zip new file mode 100644 index 00000000000..15cb0ecb3e2 Binary files /dev/null and b/lib/filestorage/tests/fixtures/empty.zip differ diff --git a/lib/filestorage/tests/fixtures/zip_info.php b/lib/filestorage/tests/fixtures/zip_info.php index f04dbe3570f..098223f9ecc 100644 --- a/lib/filestorage/tests/fixtures/zip_info.php +++ b/lib/filestorage/tests/fixtures/zip_info.php @@ -36,14 +36,24 @@ $archive = $_SERVER['argv'][1]; // Note: the ZIP structure is described at http://www.pkware.com/documents/casestudies/APPNOTE.TXT if (!$filesize = filesize($archive) or !$fp = fopen($archive, 'rb+')) { - cli_error("Can not open file file $archive"); + cli_error("Can not open ZIP archive: $archive"); +} + +if ($filesize == 22) { + $info = unpack('Vsig', fread($fp, 4)); + fclose($fp); + if ($info['sig'] == 0x6054b50) { + cli_error("This ZIP archive is empty: $archive"); + } else { + cli_error("This is not a ZIP archive: $archive"); + } } fseek($fp, 0); $info = unpack('Vsig', fread($fp, 4)); if ($info['sig'] !== 0x04034b50) { fclose($fp); - cli_error("This is not a zip file: $archive"); + cli_error("This is not a ZIP archive: $archive"); } // Find end of central directory record. diff --git a/lib/filestorage/tests/zip_packer_test.php b/lib/filestorage/tests/zip_packer_test.php index d98183f643b..d71b798c432 100644 --- a/lib/filestorage/tests/zip_packer_test.php +++ b/lib/filestorage/tests/zip_packer_test.php @@ -116,6 +116,11 @@ class zip_packer_testcase extends advanced_testcase { $this->assertTrue($file->pathname === 'Žluťoučký/Koníček.txt' or $file->pathname === 'testíček.txt' or $file->pathname === 'test.test'); } $zip_archive->close(); + + // Empty archive extraction. + $archive = __DIR__.'/fixtures/empty.zip'; + $archivefiles = $packer->list_files($archive); + $this->assertSame(array(), $archivefiles); } /** @@ -129,10 +134,10 @@ class zip_packer_testcase extends advanced_testcase { $packer = get_file_packer('application/zip'); $archive = "$CFG->tempdir/archive.zip"; - $this->assertFalse(file_exists($archive)); + $this->assertFileNotExists($archive); $result = $packer->archive_to_pathname($this->files, $archive); $this->assertTrue($result); - $this->assertTrue(file_exists($archive)); + $this->assertFileExists($archive); $archivefiles = $packer->list_files($archive); $this->assertTrue(is_array($archivefiles)); @@ -143,31 +148,37 @@ class zip_packer_testcase extends advanced_testcase { // Test invalid files parameter. $archive = "$CFG->tempdir/archive2.zip"; - $this->assertFalse(file_exists($archive)); + $this->assertFileNotExists($archive); - $this->assertFalse(file_exists(__DIR__.'/xx/yy/ee.txt')); + $this->assertFileNotExists(__DIR__.'/xx/yy/ee.txt'); $files = array('xtest.txt'=>__DIR__.'/xx/yy/ee.txt'); - // TODO MDL-37429 Ugly hack to handle multiple debugging message. To be - // removed once this issue has been integrated. - ob_start(); - $result = $packer->archive_to_pathname($files, $archive); - $d = ob_end_clean(); - $this->assertTrue($d !== ''); + $result = $packer->archive_to_pathname($files, $archive, false); $this->assertFalse($result); - // TODO MDL-37429 Uncomment the following line and remove the ugly hack above once this - // issue has been integrated. In a nutshell, we need support for multiple debug messages. - // $this->assertDebuggingCalled(); + $this->assertDebuggingCalled(); + $this->assertFileNotExists($archive); - $this->assertTrue(file_exists(__DIR__.'/fixtures/test.txt')); - $files = array(); - $files['""""'] = null; // Invalid directory name. - $files['test.txt'] = __DIR__.'/fixtures/test.txt'; $result = $packer->archive_to_pathname($files, $archive); $this->assertTrue($result); - $this->resetDebugging(); + $this->assertFileExists($archive); + $this->assertDebuggingCalled(); + $archivefiles = $packer->list_files($archive); + $this->assertSame(array(), $archivefiles); + unlink($archive); - @unlink($archive); + $this->assertFileNotExists(__DIR__.'/xx/yy/ee.txt'); + $this->assertFileExists(__DIR__.'/fixtures/test.txt'); + $files = array('xtest.txt'=>__DIR__.'/xx/yy/ee.txt', 'test.txt'=>__DIR__.'/fixtures/test.txt', 'ytest.txt'=>__DIR__.'/xx/yy/yy.txt'); + $result = $packer->archive_to_pathname($files, $archive); + $this->assertTrue($result); + $this->assertFileExists($archive); + $archivefiles = $packer->list_files($archive); + $this->assertCount(1, $archivefiles); + $this->assertEquals('test.txt', $archivefiles[0]->pathname); + $dms = $this->getDebuggingMessages(); + $this->assertCount(2, $dms); + $this->resetDebugging(); + unlink($archive); } /** @@ -212,13 +223,13 @@ class zip_packer_testcase extends advanced_testcase { $this->assertTrue(is_dir($target)); $archive = "$CFG->tempdir/archive.zip"; - $this->assertTrue(file_exists($archive)); + $this->assertFileExists($archive); $result = $packer->extract_to_pathname($archive, $target); $this->assertTrue(is_array($result)); $this->assertEquals(count($this->files), count($result)); foreach($this->files as $file=>$unused) { $this->assertTrue($result[$file]); - $this->assertTrue(file_exists($target.$file)); + $this->assertFileExists($target.$file); $this->assertSame($testcontent, file_get_contents($target.$file)); } @@ -229,7 +240,7 @@ class zip_packer_testcase extends advanced_testcase { $this->assertEquals(count($this->files), count($result)); foreach($this->files as $file=>$unused) { $this->assertTrue($result[$file]); - $this->assertTrue(file_exists($target.$file)); + $this->assertFileExists($target.$file); $this->assertSame($testcontent, file_get_contents($target.$file)); } } @@ -257,19 +268,19 @@ class zip_packer_testcase extends advanced_testcase { $donotextract = array_diff(array_keys($this->files), $onlyfiles); $archive = "$CFG->tempdir/archive.zip"; - $this->assertTrue(file_exists($archive)); + $this->assertFileExists($archive); $result = $packer->extract_to_pathname($archive, $target, $onlyfiles); $this->assertTrue(is_array($result)); $this->assertEquals(count($willbeextracted), count($result)); foreach($willbeextracted as $file) { $this->assertTrue($result[$file]); - $this->assertTrue(file_exists($target.$file)); + $this->assertFileExists($target.$file); $this->assertSame($testcontent, file_get_contents($target.$file)); } foreach($donotextract as $file) { $this->assertFalse(isset($result[$file])); - $this->assertFalse(file_exists($target.$file)); + $this->assertFileNotExists($target.$file); } } @@ -280,7 +291,7 @@ class zip_packer_testcase extends advanced_testcase { public function test_extract_to_storage() { global $CFG; - $this->resetAfterTest(true); + $this->resetAfterTest(false); $packer = get_file_packer('application/zip'); $fs = get_file_storage(); @@ -301,7 +312,7 @@ class zip_packer_testcase extends advanced_testcase { } $archive = "$CFG->tempdir/archive.zip"; - $this->assertTrue(file_exists($archive)); + $this->assertFileExists($archive); $result = $packer->extract_to_storage($archive, $context->id, 'phpunit', 'target', 0, '/'); $this->assertTrue(is_array($result)); $this->assertEquals(count($this->files), count($result)); @@ -311,5 +322,90 @@ class zip_packer_testcase extends advanced_testcase { $this->assertInstanceOf('stored_file', $stored_file); $this->assertSame($testcontent, $stored_file->get_content()); } + unlink($archive); + } + + /** + * @depends test_extract_to_storage + */ + public function test_add_files() { + global $CFG; + + $this->resetAfterTest(false); + + $packer = get_file_packer('application/zip'); + $archive = "$CFG->tempdir/archive.zip"; + + $this->assertFileNotExists($archive); + $packer->archive_to_pathname(array(), $archive); + $this->assertFileExists($archive); + + $zip_archive = new zip_archive(); + $zip_archive->open($archive, file_archive::OPEN); + $this->assertEquals(0, $zip_archive->count()); + + $zip_archive->add_file_from_string('test.txt', 'test'); + $zip_archive->close(); + $zip_archive->open($archive, file_archive::OPEN); + $this->assertEquals(1, $zip_archive->count()); + + $zip_archive->add_directory('test2'); + $zip_archive->close(); + $zip_archive->open($archive, file_archive::OPEN); + $files = $zip_archive->list_files(); + $this->assertCount(2, $files); + $this->assertEquals('test.txt', $files[0]->pathname); + $this->assertEquals('test2/', $files[1]->pathname); + + $result = $zip_archive->add_file_from_pathname('test.txt', __DIR__.'/nonexistent/file.txt'); + $this->assertFalse($result); + $zip_archive->close(); + $zip_archive->open($archive, file_archive::OPEN); + $this->assertEquals(2, $zip_archive->count()); + + unlink($archive); + } + + /** + * @depends test_add_files + */ + public function test_open_archive() { + global $CFG; + + $this->resetAfterTest(true); + + $archive = "$CFG->tempdir/archive.zip"; + + $this->assertFileNotExists($archive); + + $zip_archive = new zip_archive(); + $result = $zip_archive->open($archive, file_archive::OPEN); + $this->assertFalse($result); + $this->assertDebuggingCalled(); + + $zip_archive = new zip_archive(); + $result = $zip_archive->open($archive, file_archive::CREATE); + $this->assertTrue($result); + $zip_archive->add_file_from_string('test.txt', 'test'); + $zip_archive->close(); + $zip_archive->open($archive, file_archive::OPEN); + $this->assertEquals(1, $zip_archive->count()); + + $zip_archive = new zip_archive(); + $result = $zip_archive->open($archive, file_archive::OVERWRITE); + $this->assertTrue($result); + $zip_archive->add_file_from_string('test2.txt', 'test'); + $zip_archive->close(); + $zip_archive->open($archive, file_archive::OPEN); + $this->assertEquals(1, $zip_archive->count()); + + unlink($archive); + $zip_archive = new zip_archive(); + $result = $zip_archive->open($archive, file_archive::OVERWRITE); + $this->assertTrue($result); + $zip_archive->add_file_from_string('test2.txt', 'test'); + $zip_archive->close(); + $zip_archive->open($archive, file_archive::OPEN); + $this->assertEquals(1, $zip_archive->count()); } } diff --git a/lib/filestorage/zip_archive.php b/lib/filestorage/zip_archive.php index 435660ecd94..7f140da2ae7 100644 --- a/lib/filestorage/zip_archive.php +++ b/lib/filestorage/zip_archive.php @@ -14,7 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . - /** * Implementation of zip file archive. * @@ -28,7 +27,7 @@ defined('MOODLE_INTERNAL') || die(); require_once("$CFG->libdir/filestorage/file_archive.php"); /** - * zip file archive class. + * Zip file archive class. * * @package core_files * @category files @@ -55,7 +54,7 @@ class zip_archive extends file_archive { /** @var bool was this archive modified? */ protected $modified = false; - /** @var array unicode decoding array, created by decoding zip file*/ + /** @var array unicode decoding array, created by decoding zip file */ protected $namelookup = null; /** @@ -66,7 +65,7 @@ class zip_archive extends file_archive { } /** - * Open or create archive (depending on $mode) + * Open or create archive (depending on $mode). * * @todo MDL-31048 return error message * @param string $archivepathname @@ -102,9 +101,21 @@ class zip_archive extends file_archive { return true; } else { + $message = 'Unknown error.'; + switch ($result) { + case ZIPARCHIVE::ER_EXISTS: $message = 'File already exists.'; break; + case ZIPARCHIVE::ER_INCONS: $message = 'Zip archive inconsistent.'; break; + case ZIPARCHIVE::ER_INVAL: $message = 'Invalid argument.'; break; + case ZIPARCHIVE::ER_MEMORY: $message = 'Malloc failure.'; break; + case ZIPARCHIVE::ER_NOENT: $message = 'No such file.'; break; + case ZIPARCHIVE::ER_NOZIP: $message = 'Not a zip archive.'; break; + case ZIPARCHIVE::ER_OPEN: $message = 'Can\'t open file.'; break; + case ZIPARCHIVE::ER_READ: $message = 'Read error.'; break; + case ZIPARCHIVE::ER_SEEK: $message = 'Seek error.'; break; + } + debugging($message.': '.$archivepathname, DEBUG_DEVELOPER); $this->za = null; $this->archivepathname = null; - // TODO: maybe we should return some error info return false; } } @@ -140,7 +151,7 @@ class zip_archive extends file_archive { if (!isset($this->namelookup[$localname])) { $name = $localname; - // This should not happen + // This should not happen. if (!empty($this->encoding) and $this->encoding !== 'utf-8') { $name = @textlib::convert($name, $this->encoding, 'utf-8'); } @@ -153,7 +164,7 @@ class zip_archive extends file_archive { } /** - * Close archive + * Close archive, write changes to disk. * * @return bool success */ @@ -162,6 +173,21 @@ class zip_archive extends file_archive { return false; } + if ($this->za->numFiles == 0) { + // PHP can not create empty archives, so let's fake it. + $this->za->close(); + $this->za = null; + $this->mode = null; + $this->namelookup = null; + $this->modified = false; + @unlink($this->archivepathname); + $data = base64_decode('UEsFBgAAAAAAAAAAAAAAAAAAAAAAAA=='); + if (!file_put_contents($this->archivepathname, $data)) { + return false; + } + return true; + } + $res = $this->za->close(); $this->za = null; $this->mode = null; @@ -176,7 +202,7 @@ class zip_archive extends file_archive { } /** - * Returns file stream for reading of content + * Returns file stream for reading of content. * * @param int $index index of file * @return resource|bool file handle or false if error @@ -195,7 +221,7 @@ class zip_archive extends file_archive { } /** - * Returns file information + * Returns file information. * * @param int $index index of file * @return stdClass|bool info object or false if error @@ -233,7 +259,7 @@ class zip_archive extends file_archive { } /** - * Returns array of info about all files in archive + * Returns array of info about all files in archive. * * @return array of file infos */ @@ -256,7 +282,7 @@ class zip_archive extends file_archive { } /** - * Returns number of files in archive + * Returns number of files in archive. * * @return int number of files */ @@ -269,7 +295,7 @@ class zip_archive extends file_archive { } /** - * Add file into archive + * Add file into archive. * * @param string $localname name of file in archive * @param string $pathname location of file @@ -281,32 +307,25 @@ class zip_archive extends file_archive { } if ($this->archivepathname === realpath($pathname)) { - // do not add self into archive + // Do not add self into archive. + return false; + } + + if (!is_readable($pathname) or is_dir($pathname)) { return false; } if (is_null($localname)) { $localname = clean_param($pathname, PARAM_PATH); } - $localname = trim($localname, '/'); // no leading slashes in archives + $localname = trim($localname, '/'); // No leading slashes in archives! $localname = $this->mangle_pathname($localname); if ($localname === '') { - //sorry - conversion failed badly + // Sorry - conversion failed badly. return false; } - if (!check_php_version('5.2.8')) { - // workaround for open file handles problem, ZipArchive uses file locking in order to prevent file modifications before the close() (strange, eh?) - if ($this->count() > 0 and $this->count() % 500 === 0) { - $this->close(); - $res = $this->open($this->archivepathname, file_archive::OPEN, $this->encoding); - if ($res !== true) { - print_error('cannotopenzip'); - } - } - } - if (!$this->za->addFile($pathname, $localname)) { return false; } @@ -315,7 +334,7 @@ class zip_archive extends file_archive { } /** - * Add content of string into archive + * Add content of string into archive. * * @param string $localname name of file in archive * @param string $contents contents @@ -326,16 +345,16 @@ class zip_archive extends file_archive { return false; } - $localname = trim($localname, '/'); // no leading slashes in archives + $localname = trim($localname, '/'); // No leading slashes in archives! $localname = $this->mangle_pathname($localname); if ($localname === '') { - //sorry - conversion failed badly + // Sorry - conversion failed badly. return false; } if ($this->usedmem > 2097151) { - // this prevents running out of memory when adding many large files using strings + // This prevents running out of memory when adding many large files using strings. $this->close(); $res = $this->open($this->archivepathname, file_archive::OPEN, $this->encoding); if ($res !== true) { @@ -352,7 +371,7 @@ class zip_archive extends file_archive { } /** - * Add empty directory into archive + * Add empty directory into archive. * * @param string $localname name of file in archive * @return bool success @@ -365,7 +384,7 @@ class zip_archive extends file_archive { $localname = $this->mangle_pathname($localname); if ($localname === '/') { - //sorry - conversion failed badly + // Sorry - conversion failed badly. return false; } @@ -379,7 +398,7 @@ class zip_archive extends file_archive { } /** - * Returns current file info + * Returns current file info. * * @return stdClass */ @@ -392,7 +411,7 @@ class zip_archive extends file_archive { } /** - * Returns the index of current file + * Returns the index of current file. * * @return int current file index */ @@ -401,14 +420,14 @@ class zip_archive extends file_archive { } /** - * Moves forward to next file + * Moves forward to next file. */ public function next() { $this->pos++; } /** - * Rewinds back to the first file + * Rewinds back to the first file. */ public function rewind() { $this->pos = 0; @@ -696,7 +715,7 @@ class zip_archive extends file_archive { } /** - * Parse file header + * Parse file header. * @internal * @param string $data * @param array $centralend diff --git a/lib/filestorage/zip_packer.php b/lib/filestorage/zip_packer.php index c30b4791225..2f5bfc3f3e7 100644 --- a/lib/filestorage/zip_packer.php +++ b/lib/filestorage/zip_packer.php @@ -14,7 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . - /** * Implementation of zip packer. * @@ -39,7 +38,7 @@ require_once("$CFG->libdir/filestorage/zip_archive.php"); class zip_packer extends file_packer { /** - * Zip files and store the result in file storage + * Zip files and store the result in file storage. * * @param array $files array with full zip paths (including directory information) * as keys (archivepath=>ospathname or archivepath/subdir=>stored_file or archivepath=>array('content_as_string')) @@ -50,9 +49,10 @@ class zip_packer extends file_packer { * @param string $filepath file path * @param string $filename file name * @param int $userid user ID - * @return stored_file|bool false if error stored file instance if ok + * @param bool $ignoreinvalidfiles true means ignore missing or invalid files, false means abort on any error + * @return stored_file|bool false if error stored_file instance if ok */ - public function archive_to_storage($files, $contextid, $component, $filearea, $itemid, $filepath, $filename, $userid = NULL) { + public function archive_to_storage(array $files, $contextid, $component, $filearea, $itemid, $filepath, $filename, $userid = NULL, $ignoreinvalidfiles=true) { global $CFG; $fs = get_file_storage(); @@ -60,7 +60,7 @@ class zip_packer extends file_packer { check_dir_exists($CFG->tempdir.'/zip'); $tmpfile = tempnam($CFG->tempdir.'/zip', 'zipstor'); - if ($result = $this->archive_to_pathname($files, $tmpfile)) { + if ($result = $this->archive_to_pathname($files, $tmpfile, $ignoreinvalidfiles)) { if ($file = $fs->get_file($contextid, $component, $filearea, $itemid, $filepath, $filename)) { if (!$file->delete()) { @unlink($tmpfile); @@ -84,70 +84,78 @@ class zip_packer extends file_packer { } /** - * Zip files and store the result in os file + * Zip files and store the result in os file. * * @param array $files array with zip paths as keys (archivepath=>ospathname or archivepath=>stored_file or archivepath=>array('content_as_string')) * @param string $archivefile path to target zip file - * @return bool success + * @param bool $ignoreinvalidfiles true means ignore missing or invalid files, false means abort on any error + * @return bool true if file created, false if not */ - public function archive_to_pathname($files, $archivefile) { - if (!is_array($files)) { - return false; - } - + public function archive_to_pathname(array $files, $archivefile, $ignoreinvalidfiles=true) { $ziparch = new zip_archive(); if (!$ziparch->open($archivefile, file_archive::OVERWRITE)) { return false; } - $result = false; // One processed file or dir means success here. - + $abort = false; foreach ($files as $archivepath => $file) { $archivepath = trim($archivepath, '/'); if (is_null($file)) { - // empty directories have null as content - if ($ziparch->add_directory($archivepath.'/')) { - $result = true; - } else { + // Directories have null as content. + if (!$ziparch->add_directory($archivepath.'/')) { debugging("Can not zip '$archivepath' directory", DEBUG_DEVELOPER); + if (!$ignoreinvalidfiles) { + $abort = true; + break; + } } } else if (is_string($file)) { - if ($this->archive_pathname($ziparch, $archivepath, $file)) { - $result = true; - } else { + if (!$this->archive_pathname($ziparch, $archivepath, $file)) { debugging("Can not zip '$archivepath' file", DEBUG_DEVELOPER); + if (!$ignoreinvalidfiles) { + $abort = true; + break; + } } } else if (is_array($file)) { $content = reset($file); - if ($ziparch->add_file_from_string($archivepath, $content)) { - $result = true; - } else { + if (!$ziparch->add_file_from_string($archivepath, $content)) { debugging("Can not zip '$archivepath' file", DEBUG_DEVELOPER); + if (!$ignoreinvalidfiles) { + $abort = true; + break; + } } } else { - if ($this->archive_stored($ziparch, $archivepath, $file)) { - $result = true; - } else { + if (!$this->archive_stored($ziparch, $archivepath, $file)) { debugging("Can not zip '$archivepath' file", DEBUG_DEVELOPER); + if (!$ignoreinvalidfiles) { + $abort = true; + break; + } } } } - // We can consider that there was an error if the file generated does not contain anything. - if ($ziparch->count() == 0) { - $result = false; - debugging("Nothing was added to the zip file", DEBUG_DEVELOPER); + if (!$ziparch->close()) { + @unlink($archivefile); + return false; } - return ($ziparch->close() && $result); + if ($abort) { + @unlink($archivefile); + return false; + } + + return true; } /** - * Perform archiving file from stored file + * Perform archiving file from stored file. * * @param zip_archive $ziparch zip archive instance * @param string $archivepath file path to archive @@ -183,7 +191,7 @@ class zip_packer extends file_packer { } /** - * Perform archiving file from file path + * Perform archiving file from file path. * * @param zip_archive $ziparch zip archive instance * @param string $archivepath file path to archive @@ -213,13 +221,13 @@ class zip_packer extends file_packer { $newpath = $archivepath.'/'.$file->getFilename(); $this->archive_pathname($ziparch, $newpath, $file->getPathname()); } - unset($files); //release file handles + unset($files); // Release file handles. return true; } } /** - * Unzip file to given file path (real OS filesystem), existing files are overwrited + * Unzip file to given file path (real OS filesystem), existing files are overwritten. * * @todo MDL-31048 localise messages * @param string|stored_file $archivefile full pathname of zip file or stored_file instance @@ -319,7 +327,7 @@ class zip_packer extends file_packer { } /** - * Unzip file to given file path (real OS filesystem), existing files are overwrited + * Unzip file to given file path (real OS filesystem), existing files are overwritten. * * @todo MDL-31048 localise messages * @param string|stored_file $archivefile full pathname of zip file or stored_file instance @@ -375,7 +383,7 @@ class zip_packer extends file_packer { } if ($size < 2097151) { - // small file + // Small file. if (!$fz = $ziparch->get_stream($info->index)) { $processed[$name] = 'Can not read file from zip archive'; // TODO: localise continue; @@ -469,7 +477,7 @@ class zip_packer extends file_packer { } /** - * Returns array of info about all files in archive + * Returns array of info about all files in archive. * * @param string|file_archive $archivefile * @return array of file infos diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 468691fad16..be688260e76 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -7,6 +7,8 @@ information provided here is intended especially for developers. appropriate renderers: print_section_add_menus() See functions' phpdocs in lib/deprecatedlib.php * Function get_print_section_cm_text() is deprecated, replaced with methods in cm_info +* zip_packer may create empty zip archives, there is a new option to ignore + problematic files when creating archive === 2.4 ===