diff --git a/mod/scorm/lang/en/scorm.php b/mod/scorm/lang/en/scorm.php index dafdd2649df..9d370e5161f 100644 --- a/mod/scorm/lang/en/scorm.php +++ b/mod/scorm/lang/en/scorm.php @@ -59,7 +59,8 @@ $string['autocontinue_help'] = 'If enabled, subsequent learning objects are laun $string['autocontinuedesc'] = 'If enabled, subsequent learning objects are launched automatically, otherwise the Continue button must be used.'; $string['averageattempt'] = 'Average attempts'; $string['badmanifest'] = 'Some manifest errors: see errors log'; -$string['badpackage'] = 'The specified package/manifest is not valid. Check it and try again.'; +$string['badimsmanifestlocation'] = 'An imsmanifest.xml file was found but it was not in the root of your zip file, please re-package your SCORM'; +$string['badarchive'] = 'You must provide a valid zip file'; $string['browse'] = 'Preview'; $string['browsed'] = 'Browsed'; $string['browsemode'] = 'Preview mode'; @@ -222,7 +223,7 @@ $string['noattemptsmade'] = 'Number of attempts you have made'; $string['no_attributes'] = 'Tag {$a->tag} must have attributes'; $string['no_children'] = 'Tag {$a->tag} must have children'; $string['nolimit'] = 'Unlimited attempts'; -$string['nomanifest'] = 'Manifest not found'; +$string['nomanifest'] = 'Incorrect file package - missing imsmanifest.xml or AICC structure'; $string['noprerequisites'] = 'Sorry but you don\'t have the required prerequisites to access this activity.'; $string['noreports'] = 'No report to display'; $string['normal'] = 'Normal'; diff --git a/mod/scorm/lib.php b/mod/scorm/lib.php index 94e81038bbe..02da6c9ee2b 100644 --- a/mod/scorm/lib.php +++ b/mod/scorm/lib.php @@ -1271,32 +1271,10 @@ function scorm_dndupload_handle($uploadinfo) { $file = reset($files); // Validate the file, make sure it's a valid SCORM package! - $packer = get_file_packer('application/zip'); - $filelist = $file->list_files($packer); - - if (!is_array($filelist)) { + $errors = scorm_validate_package($file); + if (!empty($errors)) { return false; - } else { - $manifestpresent = false; - $aiccfound = false; - - foreach ($filelist as $info) { - if ($info->pathname == 'imsmanifest.xml') { - $manifestpresent = true; - break; - } - - if (preg_match('/\.cst$/', $info->pathname)) { - $aiccfound = true; - break; - } - } - - if (!$manifestpresent && !$aiccfound) { - return false; - } } - // Create a default scorm object to pass to scorm_add_instance()! $scorm = get_config('scorm'); $scorm->course = $uploadinfo->course->id; @@ -1343,3 +1321,41 @@ function scorm_set_completion($scorm, $userid, $completionstate = COMPLETION_COM $completion->update_state($cm, $completionstate, $userid); } } + +/** + * Check that a Zip file contains a valid SCORM package + * + * @param $file stored_file a Zip file. + * @return array empty if no issue is found. Array of error message otherwise + */ +function scorm_validate_package($file) { + $packer = get_file_packer('application/zip'); + $errors = array(); + $filelist = $file->list_files($packer); + + if (!is_array($filelist)) { + $errors['packagefile'] = get_string('badarchive', 'scorm'); + } else { + $aiccfound = false; + $badmanifestpresent = false; + foreach ($filelist as $info) { + if ($info->pathname == 'imsmanifest.xml') { + return array(); + } else if (strpos($info->pathname, 'imsmanifest.xml') !== false) { + // This package has an imsmanifest file inside a folder of the package. + $badmanifestpresent = true; + } + if (preg_match('/\.cst$/', $info->pathname)) { + return array(); + } + } + if (!$aiccfound) { + if ($badmanifestpresent) { + $errors['packagefile'] = get_string('badimsmanifestlocation', 'scorm'); + } else { + $errors['packagefile'] = get_string('nomanifest', 'scorm'); + } + } + } + return $errors; +} \ No newline at end of file diff --git a/mod/scorm/mod_form.php b/mod/scorm/mod_form.php index 44255b9cdb8..8826c8682ab 100644 --- a/mod/scorm/mod_form.php +++ b/mod/scorm/mod_form.php @@ -342,33 +342,7 @@ class mod_scorm_mod_form extends moodleform_mod { return $errors; } $file = reset($files); - $filename = $CFG->tempdir.'/scormimport/scrom_'.time(); - make_temp_directory('scormimport'); - $file->copy_content_to($filename); - - $packer = get_file_packer('application/zip'); - - $filelist = $packer->list_files($filename); - if (!is_array($filelist)) { - $errors['packagefile'] = 'Incorrect file package - not an archive'; //TODO: localise - } else { - $manifestpresent = false; - $aiccfound = false; - foreach ($filelist as $info) { - if ($info->pathname == 'imsmanifest.xml') { - $manifestpresent = true; - break; - } - if (preg_match('/\.cst$/', $info->pathname)) { - $aiccfound = true; - break; - } - } - if (!$manifestpresent and !$aiccfound) { - $errors['packagefile'] = 'Incorrect file package - missing imsmanifest.xml or AICC structure'; //TODO: localise - } - } - unlink($filename); + $errors = array_merge($errors, scorm_validate_package($file)); } } else if ($type === SCORM_TYPE_EXTERNAL) { diff --git a/mod/scorm/tests/packages/badscorm.zip b/mod/scorm/tests/packages/badscorm.zip new file mode 100644 index 00000000000..b2c52b60439 Binary files /dev/null and b/mod/scorm/tests/packages/badscorm.zip differ diff --git a/mod/scorm/tests/packages/invalid.zip b/mod/scorm/tests/packages/invalid.zip new file mode 100644 index 00000000000..f7fc62c0819 Binary files /dev/null and b/mod/scorm/tests/packages/invalid.zip differ diff --git a/mod/scorm/tests/packages/validaicc.zip b/mod/scorm/tests/packages/validaicc.zip new file mode 100644 index 00000000000..9e2669715da Binary files /dev/null and b/mod/scorm/tests/packages/validaicc.zip differ diff --git a/mod/scorm/tests/packages/validscorm.zip b/mod/scorm/tests/packages/validscorm.zip new file mode 100644 index 00000000000..5d00afb0710 Binary files /dev/null and b/mod/scorm/tests/packages/validscorm.zip differ diff --git a/mod/scorm/tests/validatepackage_test.php b/mod/scorm/tests/validatepackage_test.php new file mode 100644 index 00000000000..2c24981e7a2 --- /dev/null +++ b/mod/scorm/tests/validatepackage_test.php @@ -0,0 +1,77 @@ +. + +/** + * Unit tests for the mod_quiz_display_options class. + * + * @package mod_scorm + * @category phpunit + * @copyright 2013 Dan Marsden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/scorm/locallib.php'); + + +/** + * Unit tests for {@link mod_scorm}. + * + * @copyright 2013 Dan Marsden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_scorm_validatepackage_testcase extends basic_testcase { + public function test_validate_package() { + global $CFG; + $filename = "validscorm.zip"; + $file = new zip_archive(); + $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN); + $errors = scorm_validate_package($file); + $this->assertEmpty($errors); + $file->close(); + + $filename = "validaicc.zip"; + $file = new zip_archive(); + $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN); + $errors = scorm_validate_package($file); + $this->assertEmpty($errors); + $file->close(); + + $filename = "invalid.zip"; + $file = new zip_archive(); + $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN); + $errors = scorm_validate_package($file); + $this->assertArrayHasKey('packagefile', $errors); + if (isset($errors['packagefile'])) { + $this->assertEquals(get_string('nomanifest', 'scorm'), $errors['packagefile']); + } + $file->close(); + + $filename = "badscorm.zip"; + $file = new zip_archive(); + $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN); + $errors = scorm_validate_package($file); + $this->assertArrayHasKey('packagefile', $errors); + if (isset($errors['packagefile'])) { + $this->assertEquals(get_string('badimsmanifestlocation', 'scorm'), $errors['packagefile']); + } + $file->close(); + } +} +