diff --git a/admin/tool/uploadcourse/classes/course.php b/admin/tool/uploadcourse/classes/course.php index 782e1e843b6..4691ead4b96 100644 --- a/admin/tool/uploadcourse/classes/course.php +++ b/admin/tool/uploadcourse/classes/course.php @@ -347,7 +347,8 @@ class tool_uploadcourse_course { /** * Get the directory of the object to restore. * - * @return string|false subdirectory in $CFG->tempdir/backup/... + * @return string|false|null subdirectory in $CFG->tempdir/backup/..., false when an error occured + * and null when there is simply nothing. */ protected function get_restore_content_dir() { $backupfile = null; @@ -366,6 +367,9 @@ class tool_uploadcourse_course { $this->error($key, $message); } return false; + } else if ($dir === false) { + // We want to return null when nothing was wrong, but nothing was found. + $dir = null; } if (empty($dir) && !empty($this->importoptions['restoredir'])) { @@ -656,8 +660,12 @@ class tool_uploadcourse_course { $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata); // Restore data. - // TODO log warnings. + // TODO Speed up things by not really extracting the backup just yet, but checking that + // the backup file or shortname passed are valid. Extraction should happen in proceed(). $this->restoredata = $this->get_restore_content_dir(); + if ($this->restoredata === false) { + return false; + } // We can only reset courses when allowed and we are updating the course. if ($this->importoptions['reset'] || $this->options['reset']) { diff --git a/admin/tool/uploadcourse/tests/course_test.php b/admin/tool/uploadcourse/tests/course_test.php index 3459c4a3cf9..8f862125e1b 100644 --- a/admin/tool/uploadcourse/tests/course_test.php +++ b/admin/tool/uploadcourse/tests/course_test.php @@ -708,6 +708,46 @@ class tool_uploadcourse_course_testcase extends advanced_testcase { set_time_limit(0); } + public function test_restore_invalid_file() { + $this->resetAfterTest(); + + // Restore from a non-existing file should not be allowed. + $mode = tool_uploadcourse_processor::MODE_CREATE_NEW; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'A1', 'backupfile' => '/lead/no/where', + 'category' => 1, 'fullname' => 'A1'); + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertFalse($co->prepare()); + $this->assertArrayHasKey('cannotreadbackupfile', $co->get_errors()); + + // Restore from an invalid file should not be allowed. + $mode = tool_uploadcourse_processor::MODE_CREATE_NEW; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'A1', 'backupfile' => __FILE__, + 'category' => 1, 'fullname' => 'A1'); + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + + $this->assertFalse($co->prepare()); + $this->assertArrayHasKey('invalidbackupfile', $co->get_errors()); + + // Zip packer throws a debugging message, this assertion is only here to prevent + // the message from being displayed. + $this->assertDebuggingCalled(); + } + + public function test_restore_invalid_course() { + $this->resetAfterTest(); + + // Restore from an invalid file should not be allowed. + $mode = tool_uploadcourse_processor::MODE_CREATE_NEW; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'A1', 'templatecourse' => 'iamnotavalidcourse', + 'category' => 1, 'fullname' => 'A1'); + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertFalse($co->prepare()); + $this->assertArrayHasKey('coursetorestorefromdoesnotexist', $co->get_errors()); + } + /** * Testing the reset on groups, group members and enrolments. */