From c7e6fb6c8d5fd55a32e47b9c9ef7892ad3486acb Mon Sep 17 00:00:00 2001
From: Dan Marsden <dan@danmarsden.com>
Date: Fri, 5 Jul 2013 16:38:10 +1200
Subject: [PATCH] MDL-35380 SCORM: improve check for imsmanifest file and
consolidate into a single function.
---
mod/scorm/lang/en/scorm.php | 5 +-
mod/scorm/lib.php | 64 ++++++++++++-------
mod/scorm/mod_form.php | 28 +--------
mod/scorm/tests/packages/badscorm.zip | Bin 0 -> 279 bytes
mod/scorm/tests/packages/invalid.zip | Bin 0 -> 114 bytes
mod/scorm/tests/packages/validaicc.zip | Bin 0 -> 147 bytes
mod/scorm/tests/packages/validscorm.zip | Bin 0 -> 167 bytes
mod/scorm/tests/validatepackage_test.php | 77 +++++++++++++++++++++++
8 files changed, 121 insertions(+), 53 deletions(-)
create mode 100644 mod/scorm/tests/packages/badscorm.zip
create mode 100644 mod/scorm/tests/packages/invalid.zip
create mode 100644 mod/scorm/tests/packages/validaicc.zip
create mode 100644 mod/scorm/tests/packages/validscorm.zip
create 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 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 8263a6d90a4..c69ef6ffb88 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 0000000000000000000000000000000000000000..b2c52b60439d1d134365017a0b8f62d01e8fa0c4
GIT binary patch
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{
literal 0
HcmV?d00001
diff --git a/mod/scorm/tests/packages/invalid.zip b/mod/scorm/tests/packages/invalid.zip
new file mode 100644
index 0000000000000000000000000000000000000000..f7fc62c0819e73255d855615da6e0c98ede91aa2
GIT binary patch
literal 114
zcmWIWW@h1HW&i@e4UEnp8U{FkjFQyi61|L)+yHMzCVOVw>L7X)8n!foSO{ILY#=@(
M5SjsLH4ujZ01~tfcmMzZ
literal 0
HcmV?d00001
diff --git a/mod/scorm/tests/packages/validaicc.zip b/mod/scorm/tests/packages/validaicc.zip
new file mode 100644
index 0000000000000000000000000000000000000000..9e2669715da6df7da3fcd004e28f336c397d6942
GIT binary patch
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
literal 0
HcmV?d00001
diff --git a/mod/scorm/tests/packages/validscorm.zip b/mod/scorm/tests/packages/validscorm.zip
new file mode 100644
index 0000000000000000000000000000000000000000..5d00afb071036948acdb2d61dbcceeb37b203eb9
GIT binary patch
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
literal 0
HcmV?d00001
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 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * 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();
+ }
+}
+