From aee8151f7d1334d429866a26ad5c23f5d089c9d8 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Thu, 22 Aug 2013 14:11:18 +0800 Subject: [PATCH] Revert "MDL-35380 SCORM: improve check for imsmanifest file and consolidate into a single function." This reverts commit 492407e9f734989782e6380c381cfa14a09b1b26. --- mod/scorm/lang/en/scorm.php | 5 +- mod/scorm/lib.php | 26 +++++++- mod/scorm/locallib.php | 38 ----------- mod/scorm/mod_form.php | 24 ++++++- mod/scorm/tests/packages/badscorm.zip | Bin 279 -> 0 bytes mod/scorm/tests/packages/invalid.zip | Bin 114 -> 0 bytes mod/scorm/tests/packages/validaicc.zip | Bin 147 -> 0 bytes mod/scorm/tests/packages/validscorm.zip | Bin 167 -> 0 bytes mod/scorm/tests/validatepackage_test.php | 77 ----------------------- 9 files changed, 48 insertions(+), 122 deletions(-) delete mode 100644 mod/scorm/tests/packages/badscorm.zip delete mode 100644 mod/scorm/tests/packages/invalid.zip delete mode 100644 mod/scorm/tests/packages/validaicc.zip delete mode 100644 mod/scorm/tests/packages/validscorm.zip delete mode 100644 mod/scorm/tests/validatepackage_test.php diff --git a/mod/scorm/lang/en/scorm.php b/mod/scorm/lang/en/scorm.php index 9d370e5161f..dafdd2649df 100644 --- a/mod/scorm/lang/en/scorm.php +++ b/mod/scorm/lang/en/scorm.php @@ -59,8 +59,7 @@ $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['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['badpackage'] = 'The specified package/manifest is not valid. Check it and try again.'; $string['browse'] = 'Preview'; $string['browsed'] = 'Browsed'; $string['browsemode'] = 'Preview mode'; @@ -223,7 +222,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'] = 'Incorrect file package - missing imsmanifest.xml or AICC structure'; +$string['nomanifest'] = 'Manifest not found'; $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 28014750c88..94e81038bbe 100644 --- a/mod/scorm/lib.php +++ b/mod/scorm/lib.php @@ -1271,10 +1271,32 @@ function scorm_dndupload_handle($uploadinfo) { $file = reset($files); // Validate the file, make sure it's a valid SCORM package! - $errors = scorm_validate_package($file); - if (!empty($errors)) { + $packer = get_file_packer('application/zip'); + $filelist = $file->list_files($packer); + + if (!is_array($filelist)) { 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; diff --git a/mod/scorm/locallib.php b/mod/scorm/locallib.php index a70c00582c8..45715d4300e 100644 --- a/mod/scorm/locallib.php +++ b/mod/scorm/locallib.php @@ -1886,41 +1886,3 @@ function scorm_check_url($url) { return true; } - -/** - * 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 = $packer->list_files($file); - - 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 ae2bc07b8f1..8263a6d90a4 100644 --- a/mod/scorm/mod_form.php +++ b/mod/scorm/mod_form.php @@ -346,8 +346,28 @@ class mod_scorm_mod_form extends moodleform_mod { make_temp_directory('scormimport'); $file->copy_content_to($filename); - $errors = array_merge($errors, scorm_validate_package($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); } diff --git a/mod/scorm/tests/packages/badscorm.zip b/mod/scorm/tests/packages/badscorm.zip deleted file mode 100644 index b2c52b60439d1d134365017a0b8f62d01e8fa0c4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 279 zcmWIWW@Zs#W&nbu^^DFS8U{FljHJYr;^h3IT>Su`GA^*P5TG&_kLx$ofzlu>fvhYu zw>UR3FEcH*xJ0iaH^;^vNGO!078Pga=h-S5>KQ00q~;~(r)1`(+bV^IxanFb+1qgi zcr!BDGvjsz&;}q7X!z?0qLJJL(uUhrAjJ#}3JqHtLGFZWLvv$*H!B-R9TO1N0_heI GhXDX+G&wl{ diff --git a/mod/scorm/tests/packages/invalid.zip b/mod/scorm/tests/packages/invalid.zip deleted file mode 100644 index f7fc62c0819e73255d855615da6e0c98ede91aa2..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 114 zcmWIWW@h1HW&i@e4UEnp8U{FkjFQyi61|L)+yHMzCVOVw>L7X)8n!foSO{ILY#=@( M5SjsLH4ujZ01~tfcmMzZ diff --git a/mod/scorm/tests/packages/validaicc.zip b/mod/scorm/tests/packages/validaicc.zip deleted file mode 100644 index 9e2669715da6df7da3fcd004e28f336c397d6942..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 147 zcmWIWW@h1HW&nbx%xsoB{cf4d0@)zU1;n}e`6)T6ddbBlN=Z5S$=OOeO1Y`INvTCj zyj)5}`S~S40Y?L+0B=SnduH6m08Ih{g@!GSAR1w4fHx}}NQ4mxEr7Huh{FH?;6oe4 diff --git a/mod/scorm/tests/packages/validscorm.zip b/mod/scorm/tests/packages/validscorm.zip deleted file mode 100644 index 5d00afb071036948acdb2d61dbcceeb37b203eb9..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 167 zcmWIWW@h1HW&nba^^DFg9@lTE1KA+V55$?d#kq-jnQ5uTC3+RPIX3n{LZK|Rs5mn} z&sND$&p=5bH7_|oB{MJGRw*>ZP1i!n-i|B4n~}+$8MmoGgMmPyVM`;3MzbTpo0SbD O%m{?GKsp4(VE_OSdn9ZC diff --git a/mod/scorm/tests/validatepackage_test.php b/mod/scorm/tests/validatepackage_test.php deleted file mode 100644 index 2c24981e7a2..00000000000 --- a/mod/scorm/tests/validatepackage_test.php +++ /dev/null @@ -1,77 +0,0 @@ -. - -/** - * 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(); - } -} - -- 2.17.1