diff --git a/lib/componentlib.class.php b/lib/componentlib.class.php index 0d07797c368..858038d7e2a 100644 --- a/lib/componentlib.class.php +++ b/lib/componentlib.class.php @@ -268,15 +268,13 @@ class component_installer { * compare md5 values, download, unzip, install and regenerate * local md5 file * - * @global object * @uses COMPONENT_ERROR * @uses COMPONENT_UPTODATE * @uses COMPONENT_ERROR * @uses COMPONENT_INSTALLED * @return int COMPONENT_(ERROR | UPTODATE | INSTALLED) */ - function install() { - + public function install() { global $CFG; /// Check requisites are passed @@ -330,25 +328,30 @@ class component_installer { $this->errorstring='downloadedfilecheckfailed'; return COMPONENT_ERROR; } - /// Move current revision to a safe place - $destinationdir = $CFG->dataroot.'/'.$this->destpath; - $destinationcomponent = $destinationdir.'/'.$this->componentname; - @remove_dir($destinationcomponent.'_old'); // Deleting a possible old version. + + // Move current revision to a safe place. + $destinationdir = $CFG->dataroot . '/' . $this->destpath; + $destinationcomponent = $destinationdir . '/' . $this->componentname; + $destinationcomponentold = $destinationcomponent . '_old'; + @remove_dir($destinationcomponentold); // Deleting a possible old version. // Moving to a safe place. - @rename($destinationcomponent, $destinationcomponent.'_old'); + @rename($destinationcomponent, $destinationcomponentold); - /// Unzip new version - if (!unzip_file($zipfile, $destinationdir, false)) { - /// Error so, go back to the older + // Unzip new version. + $packer = get_file_packer('application/zip'); + $unzipsuccess = $packer->extract_to_pathname($zipfile, $destinationdir, null, null, true); + if (!$unzipsuccess) { @remove_dir($destinationcomponent); - @rename ($destinationcomponent.'_old', $destinationcomponent); - $this->errorstring='cannotunzipfile'; + @rename($destinationcomponentold, $destinationcomponent); + $this->errorstring = 'cannotunzipfile'; return COMPONENT_ERROR; } - /// Delete old component version - @remove_dir($destinationcomponent.'_old'); - /// Create local md5 + + // Delete old component version. + @remove_dir($destinationcomponentold); + + // Create local md5. if ($file = fopen($destinationcomponent.'/'.$this->componentname.'.md5', 'w')) { if (!fwrite($file, $new_md5)) { fclose($file); diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index bda248c94a4..fcc6cfba0be 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -608,11 +608,13 @@ function detect_munged_arguments($string, $allowdots=1) { * @param string $zipfile The zip file to unzip * @param string $destination The location to unzip to * @param bool $showstatus_ignored Unused + * @deprecated since 2.0 MDL-15919 */ function unzip_file($zipfile, $destination = '', $showstatus_ignored = true) { - global $CFG; + debugging(__FUNCTION__ . '() is deprecated. ' + . 'Please use the application/zip file_packer implementation instead.', DEBUG_DEVELOPER); - //Extract everything from zipfile + // Extract everything from zipfile. $path_parts = pathinfo(cleardoubleslashes($zipfile)); $zippath = $path_parts["dirname"]; //The path of the zip file $zipfilename = $path_parts["basename"]; //The name of the zip file @@ -674,11 +676,14 @@ function unzip_file($zipfile, $destination = '', $showstatus_ignored = true) { * @param array $originalfiles Files to zip * @param string $destination The destination path * @return bool Outcome + * + * @deprecated since 2.0 MDL-15919 */ -function zip_files ($originalfiles, $destination) { - global $CFG; +function zip_files($originalfiles, $destination) { + debugging(__FUNCTION__ . '() is deprecated. ' + . 'Please use the application/zip file_packer implementation instead.', DEBUG_DEVELOPER); - //Extract everything from destination + // Extract everything from destination. $path_parts = pathinfo(cleardoubleslashes($destination)); $destpath = $path_parts["dirname"]; //The path of the zip file $destfilename = $path_parts["basename"]; //The name of the zip file diff --git a/lib/filestorage/file_packer.php b/lib/filestorage/file_packer.php index 5e6ec4f6386..468fed46a55 100644 --- a/lib/filestorage/file_packer.php +++ b/lib/filestorage/file_packer.php @@ -94,10 +94,12 @@ abstract class file_packer { * @param string $pathname target directory * @param array $onlyfiles only extract files present in the array * @param file_progress $progress Progress indicator callback or null if not required + * @param bool $returnbool Whether to return a basic true/false indicating error state, or full per-file error + * details. * @return array|bool list of processed files; false if error */ public abstract function extract_to_pathname($archivefile, $pathname, - array $onlyfiles = NULL, file_progress $progress = null); + array $onlyfiles = NULL, file_progress $progress = null, $returnbool = false); /** * Extract file to given file path (real OS filesystem), existing files are overwritten. diff --git a/lib/filestorage/mbz_packer.php b/lib/filestorage/mbz_packer.php index 749c622b12a..3fde394c0f0 100644 --- a/lib/filestorage/mbz_packer.php +++ b/lib/filestorage/mbz_packer.php @@ -95,13 +95,15 @@ class mbz_packer extends file_packer { * @param string $pathname target directory * @param array $onlyfiles only extract files present in the array * @param file_progress $progress Progress indicator callback or null if not required + * @param bool $returnbool Whether to return a basic true/false indicating error state, or full per-file error + * details. * @return array list of processed files (name=>true) * @throws moodle_exception If error */ public function extract_to_pathname($archivefile, $pathname, - array $onlyfiles = null, file_progress $progress = null) { + array $onlyfiles = null, file_progress $progress = null, $returnbool = false) { return $this->get_packer_for_read_operation($archivefile)->extract_to_pathname( - $archivefile, $pathname, $onlyfiles, $progress); + $archivefile, $pathname, $onlyfiles, $progress, $returnbool); } /** diff --git a/lib/filestorage/tests/mbz_packer_test.php b/lib/filestorage/tests/mbz_packer_test.php index 5882218379a..b64c6939f03 100644 --- a/lib/filestorage/tests/mbz_packer_test.php +++ b/lib/filestorage/tests/mbz_packer_test.php @@ -87,4 +87,55 @@ class core_files_mbz_packer_testcase extends advanced_testcase { $this->assertNotEmpty($out); $this->assertEquals('frog', $out->get_content()); } + + public function usezipbackups_provider() { + return [ + 'Use zips' => [true], + 'Use tgz' => [false], + ]; + } + + /** + * @dataProvider usezipbackups_provider + */ + public function test_extract_to_pathname_returnvalue_successful($usezipbackups) { + global $CFG; + $this->resetAfterTest(); + + $packer = get_file_packer('application/vnd.moodle.backup'); + + // Set up basic archive contents. + $files = array('1.txt' => array('frog')); + + // Create 2 archives (each with one file in) in zip mode. + $CFG->usezipbackups = $usezipbackups; + + $mbzfile = make_request_directory() . '/file.mbz'; + $packer->archive_to_pathname($files, $mbzfile); + + $target = make_request_directory(); + $result = $packer->extract_to_pathname($mbzfile, $target, null, null, true); + $this->assertTrue($result); + } + + /** + * @dataProvider usezipbackups_provider + */ + public function test_extract_to_pathname_returnvalue_failure($usezipbackups) { + global $CFG; + $this->resetAfterTest(); + + $packer = get_file_packer('application/vnd.moodle.backup'); + + // Create 2 archives (each with one file in) in zip mode. + $CFG->usezipbackups = $usezipbackups; + + $mbzfile = make_request_directory() . '/file.mbz'; + file_put_contents($mbzfile, 'Content'); + + $target = make_request_directory(); + $result = $packer->extract_to_pathname($mbzfile, $target, null, null, true); + $this->assertDebuggingCalledCount(1); + $this->assertFalse($result); + } } diff --git a/lib/filestorage/tests/tgz_packer_test.php b/lib/filestorage/tests/tgz_packer_test.php index d61d264d2d2..74dba64f91d 100644 --- a/lib/filestorage/tests/tgz_packer_test.php +++ b/lib/filestorage/tests/tgz_packer_test.php @@ -247,6 +247,42 @@ class core_files_tgz_packer_testcase extends advanced_testcase implements file_p $this->assertTrue(is_dir($outdir . '/out6')); } + /** + * Tests extracting files returning only a boolean state with success. + */ + public function test_extract_to_pathname_returnvalue_successful() { + $packer = get_file_packer('application/x-gzip'); + + // Prepare files. + $files = $this->prepare_file_list(); + $archivefile = make_request_directory() . DIRECTORY_SEPARATOR . 'test.tgz'; + $packer->archive_to_pathname($files, $archivefile); + + // Extract same files. + $outdir = make_request_directory(); + $result = $packer->extract_to_pathname($archivefile, $outdir, null, null, true); + + $this->assertTrue($result); + } + + /** + * Tests extracting files returning only a boolean state with failure. + */ + public function test_extract_to_pathname_returnvalue_failure() { + $packer = get_file_packer('application/x-gzip'); + + // Create sample files. + $archivefile = make_request_directory() . DIRECTORY_SEPARATOR . 'test.tgz'; + file_put_contents($archivefile, ''); + + // Extract same files. + $outdir = make_request_directory(); + + $result = $packer->extract_to_pathname($archivefile, $outdir, null, null, true); + + $this->assertFalse($result); + } + /** * Tests the progress reporting. */ diff --git a/lib/filestorage/tests/zip_packer_test.php b/lib/filestorage/tests/zip_packer_test.php index 8da89dd3d80..d7f25004866 100644 --- a/lib/filestorage/tests/zip_packer_test.php +++ b/lib/filestorage/tests/zip_packer_test.php @@ -294,6 +294,41 @@ class core_files_zip_packer_testcase extends advanced_testcase implements file_p } + /** + * @depends test_archive_to_storage + */ + public function test_extract_to_pathname_returnvalue_successful() { + global $CFG; + + $this->resetAfterTest(false); + + $packer = get_file_packer('application/zip'); + + $target = make_request_directory(); + + $archive = "$CFG->tempdir/archive.zip"; + $this->assertFileExists($archive); + $result = $packer->extract_to_pathname($archive, $target, null, null, true); + $this->assertTrue($result); + } + + /** + * @depends test_archive_to_storage + */ + public function test_extract_to_pathname_returnvalue_failure() { + global $CFG; + + $this->resetAfterTest(false); + + $packer = get_file_packer('application/zip'); + + $target = make_request_directory(); + + $archive = "$CFG->tempdir/noarchive.zip"; + $result = $packer->extract_to_pathname($archive, $target, null, null, true); + $this->assertFalse($result); + } + /** * @depends test_archive_to_storage */ diff --git a/lib/filestorage/tgz_packer.php b/lib/filestorage/tgz_packer.php index 5aea5844ed3..23a8cd3c64e 100644 --- a/lib/filestorage/tgz_packer.php +++ b/lib/filestorage/tgz_packer.php @@ -635,14 +635,37 @@ class tgz_packer extends file_packer { * @param string $pathname target directory * @param array $onlyfiles only extract files present in the array * @param file_progress $progress Progress indicator callback or null if not required + * @param bool $returnbool Whether to return a basic true/false indicating error state, or full per-file error + * details. * @return array list of processed files (name=>true) * @throws moodle_exception If error */ public function extract_to_pathname($archivefile, $pathname, - array $onlyfiles = null, file_progress $progress = null) { + array $onlyfiles = null, file_progress $progress = null, $returnbool = false) { $extractor = new tgz_extractor($archivefile); - return $extractor->extract( - new tgz_packer_extract_to_pathname($pathname, $onlyfiles), $progress); + try { + $result = $extractor->extract( + new tgz_packer_extract_to_pathname($pathname, $onlyfiles), $progress); + if ($returnbool) { + if (!is_array($result)) { + return false; + } + foreach ($result as $status) { + if ($status !== true) { + return false; + } + } + return true; + } else { + return $result; + } + } catch (moodle_exception $e) { + if ($returnbool) { + return false; + } else { + throw $e; + } + } } /** diff --git a/lib/filestorage/zip_packer.php b/lib/filestorage/zip_packer.php index 175a55217a1..47395f73b24 100644 --- a/lib/filestorage/zip_packer.php +++ b/lib/filestorage/zip_packer.php @@ -258,10 +258,12 @@ class zip_packer extends file_packer { * @param array $onlyfiles only extract files present in the array. The path to files MUST NOT * start with a /. Example: array('myfile.txt', 'directory/anotherfile.txt') * @param file_progress $progress Progress indicator callback or null if not required + * @param bool $returnbool Whether to return a basic true/false indicating error state, or full per-file error + * details. * @return bool|array list of processed files; false if error */ public function extract_to_pathname($archivefile, $pathname, - array $onlyfiles = null, file_progress $progress = null) { + array $onlyfiles = null, file_progress $progress = null, $returnbool = false) { global $CFG; if (!is_string($archivefile)) { @@ -269,6 +271,7 @@ class zip_packer extends file_packer { } $processed = array(); + $success = true; $pathname = rtrim($pathname, '/'); if (!is_readable($archivefile)) { @@ -308,6 +311,7 @@ class zip_packer extends file_packer { // directory if (is_file($newdir) and !unlink($newdir)) { $processed[$name] = 'Can not create directory, file already exists'; // TODO: localise + $success = false; continue; } if (is_dir($newdir)) { @@ -318,6 +322,7 @@ class zip_packer extends file_packer { $processed[$name] = true; } else { $processed[$name] = 'Can not create directory'; // TODO: localise + $success = false; } } continue; @@ -330,6 +335,7 @@ class zip_packer extends file_packer { if (!is_dir($newdir)) { if (!mkdir($newdir, $CFG->directorypermissions, true)) { $processed[$name] = 'Can not create directory'; // TODO: localise + $success = false; continue; } } @@ -337,10 +343,12 @@ class zip_packer extends file_packer { $newfile = "$newdir/$filename"; if (!$fp = fopen($newfile, 'wb')) { $processed[$name] = 'Can not write target file'; // TODO: localise + $success = false; continue; } if (!$fz = $ziparch->get_stream($info->index)) { $processed[$name] = 'Can not read file from zip archive'; // TODO: localise + $success = false; fclose($fp); continue; } @@ -353,6 +361,7 @@ class zip_packer extends file_packer { fclose($fp); if (filesize($newfile) !== $size) { $processed[$name] = 'Unknown error during zip extraction'; // TODO: localise + $success = false; // something went wrong :-( @unlink($newfile); continue; @@ -360,7 +369,12 @@ class zip_packer extends file_packer { $processed[$name] = true; } $ziparch->close(); - return $processed; + + if ($returnbool) { + return $success; + } else { + return $processed; + } } /** diff --git a/lib/odslib.class.php b/lib/odslib.class.php index c5d5afd3017..7ac88b31a5e 100644 --- a/lib/odslib.class.php +++ b/lib/odslib.class.php @@ -72,6 +72,9 @@ class MoodleODSWorkbook { * Close the Moodle Workbook. */ public function close() { + global $CFG; + require_once($CFG->libdir . '/filelib.php'); + $writer = new MoodleODSWriter($this->worksheets); $contents = $writer->get_file_content(); @@ -825,53 +828,30 @@ class MoodleODSWriter { $this->worksheets = $worksheets; } + /** + * Fetch the file ocntnet for the ODS. + * + * @return string + */ public function get_file_content() { - global $CFG; + $dir = make_request_directory(); + $filename = $dir . '/result.ods'; - require_once($CFG->libdir.'/filelib.php'); + $files = [ + 'mimetype' => [$this->get_ods_mimetype()], + 'content.xml' => [$this->get_ods_content($this->worksheets)], + 'meta.xml' => [$this->get_ods_meta()], + 'styles.xml' => [$this->get_ods_styles()], + 'settings.xml' => [$this->get_ods_settings()], + 'META-INF/manifest.xml' => [$this->get_ods_manifest()], + ]; - do { - $dir = 'ods/'.time().'_'.rand(0, 10000); - } while (file_exists($CFG->tempdir.'/'.$dir)); + $packer = get_file_packer('application/zip'); + $packer->archive_to_pathname($files, $filename); - make_temp_directory($dir); - make_temp_directory($dir.'/META-INF'); - $dir = "$CFG->tempdir/$dir"; - $files = array(); - - $handle = fopen("$dir/mimetype", 'w'); - fwrite($handle, $this->get_ods_mimetype()); - $files[] = "$dir/mimetype"; - - $handle = fopen("$dir/content.xml", 'w'); - fwrite($handle, $this->get_ods_content($this->worksheets)); - $files[] = "$dir/content.xml"; - - $handle = fopen("$dir/meta.xml", 'w'); - fwrite($handle, $this->get_ods_meta()); - $files[] = "$dir/meta.xml"; - - $handle = fopen("$dir/styles.xml", 'w'); - fwrite($handle, $this->get_ods_styles()); - $files[] = "$dir/styles.xml"; - - $handle = fopen("$dir/settings.xml", 'w'); - fwrite($handle, $this->get_ods_settings()); - $files[] = "$dir/settings.xml"; - - $handle = fopen("$dir/META-INF/manifest.xml", 'w'); - fwrite($handle, $this->get_ods_manifest()); - $files[] = "$dir/META-INF"; - - $filename = "$dir/result.ods"; - zip_files($files, $filename); - - $handle = fopen($filename, 'rb'); - $contents = fread($handle, filesize($filename)); - fclose($handle); - - remove_dir($dir); // Cleanup the temp directory. + $contents = file_get_contents($filename); + remove_dir($dir); return $contents; } @@ -1286,7 +1266,7 @@ class MoodleODSWriter { office:version="1.2"> Moodle '.$CFG->release.' - '.fullname($USER, true).' + ' . htmlspecialchars(fullname($USER, true), ENT_QUOTES, 'utf-8') . ' '.strftime('%Y-%m-%dT%H:%M:%S').' diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 7d080673270..926ad3d0d59 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -25,6 +25,8 @@ information provided here is intended especially for developers. * The following functions have been deprecated and are not used any more: - get_records_csv() Please use csv_import_reader::load_csv_content() instead. - put_records_csv() Please use download_as_dataformat (lib/dataformatlib.php) instead. + - zip_files() - See MDL-24343 for more information. + - unzip_file() - See MDL-24343 for more information. * The password_compat library was removed as it is no longer required. * Phpunit has been upgraded to 5.4.x and following has been deprecated and is not used any more: - setExpectedException(), use @expectedException or $this->expectException() and $this->expectExceptionMessage() @@ -47,6 +49,8 @@ information provided here is intended especially for developers. * The alfresco library has been removed from core. It was an old version of the library which was not compatible with newer versions of Alfresco. * Added down arrow: $OUTPUT->darrow. +* All file_packer implementations now accept an additional parameter to allow a simple boolean return value instead of + an array of individual file statuses. === 3.1 === diff --git a/question/format/webct/format.php b/question/format/webct/format.php index 83c800e1268..9d1c8155535 100644 --- a/question/format/webct/format.php +++ b/question/format/webct/format.php @@ -255,7 +255,7 @@ class qformat_webct extends qformat_default { * @return bool success */ public function importpostprocess() { - if ($this->tempdir != '') { + if (!empty($this->tempdir)) { fulldelete($this->tempdir); } return true; @@ -280,15 +280,15 @@ class qformat_webct extends qformat_default { } // We are importing a zip file. // Create name for temporary directory. - $uniquecode = time(); - $this->tempdir = make_temp_directory('webct_import/' . $uniquecode); + $this->tempdir = make_request_directory(); if (is_readable($filename)) { if (!copy($filename, $this->tempdir . '/webct.zip')) { $this->error(get_string('cannotcopybackup', 'question')); fulldelete($this->tempdir); return false; } - if (unzip_file($this->tempdir . '/webct.zip', '', false)) { + $packer = get_file_packer('application/zip'); + if ($packer->extract_to_pathname($this->tempdir . '/webct.zip', $this->tempdir, null, null, true)) { $dir = $this->tempdir; if ((($handle = opendir($dir))) == false) { // The directory could not be opened.