Merge branch 'MDL-55020-master-pluginzip' of git://github.com/mudrd8mz/moodle

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);
+            }
+        }
+    }
 }

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() {