diff --git a/lib/classes/update/code_manager.php b/lib/classes/update/code_manager.php index 12719cbf446..9eac43bf937 100644 --- a/lib/classes/update/code_manager.php +++ b/lib/classes/update/code_manager.php @@ -26,6 +26,7 @@ namespace core\update; use core_component; use coding_exception; +use moodle_exception; use SplFileInfo; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; @@ -159,15 +160,18 @@ class code_manager { */ public function unzip_plugin_file($zipfilepath, $targetdir, $rootdir = '') { + // Extract the package into a temporary location. $fp = get_file_packer('application/zip'); - $files = $fp->extract_to_pathname($zipfilepath, $targetdir); + $tempdir = make_request_directory(); + $files = $fp->extract_to_pathname($zipfilepath, $tempdir); if (!$files) { return array(); } + // If requested, rename the root directory of the plugin. if (!empty($rootdir)) { - $files = $this->rename_extracted_rootdir($targetdir, $rootdir, $files); + $files = $this->rename_extracted_rootdir($tempdir, $rootdir, $files); } // Sometimes zip may not contain all parent directories, add them to make it consistent. @@ -187,6 +191,9 @@ class code_manager { } } + // Move the extracted files into the target location. + $this->move_extracted_plugin_files($tempdir, $targetdir, $files); + // Set the permissions of extracted subdirs and files. $this->set_plugin_files_permissions($targetdir, $files); @@ -443,12 +450,10 @@ class code_manager { /** * Renames the root directory of the extracted ZIP package. * - * This method does not validate the presence of the single root directory - * (it is the validator's duty). It just searches for the first directory - * under the given location and renames it. - * - * The method will not rename the root if the requested location already - * exists. + * This internal helper method assumes that the plugin ZIP package has been + * extracted into a temporary empty directory so the plugin folder is the + * only folder there. The ZIP package is supposed to be validated so that + * it contains just a single root folder. * * @param string $dirname fullpath location of the extracted ZIP package * @param string $rootdir the requested name of the root directory @@ -473,8 +478,11 @@ class code_manager { continue; } if (is_dir($dirname.'/'.$item)) { + if ($found !== null and $found !== $item) { + // Multiple directories found. + throw new moodle_exception('unexpected_archive_structure', 'core_plugin'); + } $found = $item; - break; } } @@ -520,4 +528,34 @@ class code_manager { } } } + + /** + * Moves the extracted contents of the plugin ZIP into the target location. + * + * @param string $sourcedir full path to the directory the ZIP file was extracted to + * @param mixed $targetdir full path to the directory where the files should be moved to + * @param array $files list of extracted files + */ + protected function move_extracted_plugin_files($sourcedir, $targetdir, array $files) { + global $CFG; + + foreach ($files as $file => $status) { + if ($status !== true) { + throw new moodle_exception('corrupted_archive_structure', 'core_plugin', '', $file, $status); + } + + $source = $sourcedir.'/'.$file; + $target = $targetdir.'/'.$file; + + if (is_dir($source)) { + continue; + + } else { + if (!is_dir(dirname($target))) { + mkdir(dirname($target), $CFG->directorypermissions, true); + } + rename($source, $target); + } + } + } } diff --git a/lib/tests/fixtures/update_validator/zips/multidir.zip b/lib/tests/fixtures/update_validator/zips/multidir.zip new file mode 100644 index 00000000000..a350015b319 Binary files /dev/null and b/lib/tests/fixtures/update_validator/zips/multidir.zip differ diff --git a/lib/tests/update_code_manager_test.php b/lib/tests/update_code_manager_test.php index 6166152d3a5..cbf4717cdee 100644 --- a/lib/tests/update_code_manager_test.php +++ b/lib/tests/update_code_manager_test.php @@ -73,6 +73,7 @@ class core_update_code_manager_testcase extends advanced_testcase { $codeman = new \core\update\testable_code_manager(); $zipfilepath = __DIR__.'/fixtures/update_validator/zips/invalidroot.zip'; $targetdir = make_request_directory(); + mkdir($targetdir.'/aaa_another'); $files = $codeman->unzip_plugin_file($zipfilepath, $targetdir); @@ -110,6 +111,15 @@ class core_update_code_manager_testcase extends advanced_testcase { $files = $codeman->unzip_plugin_file($zipfilepath, $targetdir, 'bar'); } + public function test_unzip_plugin_file_multidir() { + $codeman = new \core\update\testable_code_manager(); + $zipfilepath = __DIR__.'/fixtures/update_validator/zips/multidir.zip'; + $targetdir = make_request_directory(); + // Attempting to rename the root folder if there are multiple ones should lead to exception. + $this->setExpectedException('moodle_exception'); + $files = $codeman->unzip_plugin_file($zipfilepath, $targetdir, 'foo'); + } + public function test_get_plugin_zip_root_dir() { $codeman = new \core\update\testable_code_manager(); @@ -118,6 +128,9 @@ class core_update_code_manager_testcase extends advanced_testcase { $zipfilepath = __DIR__.'/fixtures/update_validator/zips/bar.zip'; $this->assertEquals('bar', $codeman->get_plugin_zip_root_dir($zipfilepath)); + + $zipfilepath = __DIR__.'/fixtures/update_validator/zips/multidir.zip'; + $this->assertSame(false, $codeman->get_plugin_zip_root_dir($zipfilepath)); } public function test_list_plugin_folder_files() {