From 9cf3f4aa7f3e28197b3aa366fed3a3d0971a7848 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Tue, 22 Jan 2019 15:27:27 +0100 Subject: [PATCH 1/3] JI-942 Add restict save when a higher version is available --- ...ntent-outdated-library-exception.class.php | 3 + h5p-default-storage.class.php | 18 ++++++ h5p-file-storage.interface.php | 10 ++++ h5p.classes.php | 55 ++++++++++++++++--- js/h5p-version.js | 21 +++++-- 5 files changed, 94 insertions(+), 13 deletions(-) create mode 100644 exceptions/h5p-save-content-outdated-library-exception.class.php diff --git a/exceptions/h5p-save-content-outdated-library-exception.class.php b/exceptions/h5p-save-content-outdated-library-exception.class.php new file mode 100644 index 0000000..dab8ed2 --- /dev/null +++ b/exceptions/h5p-save-content-outdated-library-exception.class.php @@ -0,0 +1,3 @@ +path . $upgrades)) { + return $upgrades; + } + else { + return NULL; + } + } + /** * Recursive function for copying directories. * diff --git a/h5p-file-storage.interface.php b/h5p-file-storage.interface.php index 4dbdbc6..4bb1368 100644 --- a/h5p-file-storage.interface.php +++ b/h5p-file-storage.interface.php @@ -199,4 +199,14 @@ interface H5PFileStorage { * @return bool */ public function hasPresave($libraryName, $developmentPath = null); + + /** + * Check if upgrades script exist for library. + * + * @param string $machineName + * @param int $majorVersion + * @param int $minorVersion + * @return string Relative path + */ + public function getUpgradeScript($machineName, $majorVersion, $minorVersion); } diff --git a/h5p.classes.php b/h5p.classes.php index d23394e..d2a7d27 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -614,6 +614,14 @@ interface H5PFrameworkInterface { * containing the new content type cache that should replace the old one. */ public function replaceContentTypeCache($contentTypeCache); + + /** + * Checks if the given library has a higher version. + * + * @param array $library + * @return boolean + */ + public function libraryHasUpgrade($library); } /** @@ -919,11 +927,27 @@ class H5PValidator { } if (!empty($missingLibraries)) { - foreach ($missingLibraries as $libString => $library) { - $this->h5pF->setErrorMessage($this->h5pF->t('Missing required library @library', array('@library' => $libString)), 'missing-required-library'); + // We still have missing libraries, check if our main library has an upgrade (BUT only if we has content) + $mainDependency = NULL; + if (!$skipContent) { + foreach ($mainH5PData['preloadedDependencies'] as $dep) { + if ($dep['machineName'] === $mainH5PData['mainLibrary']) { + $mainDependency = $dep; + } + } } - if (!$this->h5pC->mayUpdateLibraries()) { - $this->h5pF->setInfoMessage($this->h5pF->t("Note that the libraries may exist in the file you uploaded, but you're not allowed to upload new libraries. Contact the site administrator about this.")); + + if ($skipContent || !$mainDependency || !$this->h5pF->libraryHasUpgrade(array( + 'machineName' => $mainDependency['mainLibrary'], + 'majorVersion' => $mainDependency['majorVersion'], + 'minorVersion' => $mainDependency['minorVersion'] + ))) { + foreach ($missingLibraries as $libString => $library) { + $this->h5pF->setErrorMessage($this->h5pF->t('Missing required library @library', array('@library' => $libString)), 'missing-required-library'); + } + if (!$this->h5pC->mayUpdateLibraries()) { + $this->h5pF->setInfoMessage($this->h5pF->t("Note that the libraries may exist in the file you uploaded, but you're not allowed to upload new libraries. Contact the site administrator about this.")); + } } } $valid = empty($missingLibraries) && $valid; @@ -1394,15 +1418,23 @@ class H5PStorage { if (isset($options['disable'])) { $content['disable'] = $options['disable']; } - $content['id'] = $this->h5pC->saveContent($content, $contentMainId); - $this->contentId = $content['id']; try { + // Store content in database + $content['id'] = $this->h5pC->saveContent($content, $contentMainId); + $this->contentId = $content['id']; + // Save content folder contents $this->h5pC->fs->saveContent($current_path, $content); } catch (Exception $e) { - $this->h5pF->setErrorMessage($e->getMessage(), 'save-content-failed'); + if ($e instanceof H5PSaveContentOutdatedLibraryException) { + $message = $this->h5pF->t("You're trying to upload content of an older version of H5P. Please upgrade the content on the server it originated from and try to upload again or turn on the H5P Hub to have this server upgrade it for your automaticall."); + } + else { + $message = $e->getMessage(); + } + $this->h5pF->setErrorMessage($message, 'save-content-failed'); } // Remove temp content folder @@ -1922,8 +1954,6 @@ class H5PCore { $this->relativePathRegExp = '/^((\.\.\/){1,2})(.*content\/)?(\d+|editor)\/(.+)$/'; } - - /** * Save content and clear cache. * @@ -1932,6 +1962,13 @@ class H5PCore { * @return int Content ID */ public function saveContent($content, $contentMainId = NULL) { + + // Check that this is the latest version of the content type we have + if ($this->h5pF->libraryHasUpgrade($content['library'])) { + // We do not allow storing old content due to security concerns + throw new \H5PSaveContentOutdatedLibraryException($this->h5pF->t('Something unexpected happened. We were unable to save this content.')); + } + if (isset($content['id'])) { $this->h5pF->updateContent($content, $contentMainId); } diff --git a/js/h5p-version.js b/js/h5p-version.js index 7089b5a..8457341 100644 --- a/js/h5p-version.js +++ b/js/h5p-version.js @@ -7,11 +7,24 @@ H5P.Version = (function () { * @param {String} version */ function Version(version) { - var versionSplit = version.split('.', 3); - // Public - this.major =+ versionSplit[0]; - this.minor =+ versionSplit[1]; + if (typeof version === 'string') { + // Name version string (used by content upgrade) + var versionSplit = version.split('.', 3); + this.major =+ versionSplit[0]; + this.minor =+ versionSplit[1]; + } + else { + // Library objects (used by editor) + if (version.localMajorVersion !== undefined) { + this.major =+ version.localMajorVersion; + this.minor =+ version.localMinorVersion; + } + else { + this.major =+ version.majorVersion; + this.minor =+ version.minorVersion; + } + } /** * Public. Custom string for this object. From c9e1ac934714b50167f1884d21e235fed8aa5e01 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 28 Jan 2019 16:01:08 +0100 Subject: [PATCH 2/3] JI-942 Add better error handling for Content Upgrade --- h5p.classes.php | 3 +- js/h5p-content-upgrade-process.js | 14 +++++++-- js/h5p-content-upgrade.js | 48 +++++++++++++++++++++---------- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index d2a7d27..8ea9e7a 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -537,9 +537,10 @@ interface H5PFrameworkInterface { * Get number of contents using library as main library. * * @param int $libraryId + * @param array $skip * @return int */ - public function getNumContent($libraryId); + public function getNumContent($libraryId, $skip = NULL); /** * Determines if content slug is used. diff --git a/js/h5p-content-upgrade-process.js b/js/h5p-content-upgrade-process.js index e683902..6954622 100644 --- a/js/h5p-content-upgrade-process.js +++ b/js/h5p-content-upgrade-process.js @@ -27,6 +27,7 @@ H5P.ContentUpgradeProcess = (function (Version) { self.loadLibrary = loadLibrary; self.upgrade(name, oldVersion, newVersion, params.params, params.metadata, function (err, upgradedParams, upgradedMetadata) { if (err) { + err.id = id; return done(err); } @@ -176,7 +177,11 @@ H5P.ContentUpgradeProcess = (function (Version) { var usedVer = new Version(usedLib[1]); var availableVer = new Version(availableLib[1]); if (usedVer.major > availableVer.major || (usedVer.major === availableVer.major && usedVer.minor >= availableVer.minor)) { - return done(); // Larger or same version that's available + return done({ + type: 'errorTooHighVersion', + used: usedLib[0] + ' ' + usedVer, + supported: availableLib[0] + ' ' + availableVer + }); // Larger or same version that's available } // A newer version is available, upgrade params @@ -192,7 +197,12 @@ H5P.ContentUpgradeProcess = (function (Version) { }); } } - done(); + + // Content type was not supporte by the higher version + done({ + type: 'errorNotSupported', + used: usedLib[0] + ' ' + usedVer + }); break; case 'group': diff --git a/js/h5p-content-upgrade.js b/js/h5p-content-upgrade.js index bb5244a..168c694 100644 --- a/js/h5p-content-upgrade.js +++ b/js/h5p-content-upgrade.js @@ -1,7 +1,7 @@ /* global H5PAdminIntegration H5PUtils */ (function ($, Version) { - var info, $container, librariesCache = {}, scriptsCache = {}; + var info, $log, $container, librariesCache = {}, scriptsCache = {}; // Initialize $(document).ready(function () { @@ -9,7 +9,9 @@ info = H5PAdminIntegration.libraryInfo; // Get and reset container - $container = $('#h5p-admin-container').html('

' + info.message + '

'); + const $wrapper = $('#h5p-admin-container').html(''); + $log = $('').appendTo($wrapper); + $container = $('

' + info.message + '

').appendTo($wrapper); // Make it possible to select version var $version = $(getVersionSelect(info.versions)).appendTo($container); @@ -120,9 +122,7 @@ }, error: function (error) { self.printError(error.err); - - // Stop everything - self.terminate(); + self.workDone(error.id, null, this); }, loadLibrary: function (details) { var worker = this; @@ -184,7 +184,7 @@ self.token = inData.token; // Start processing - self.processBatch(inData.params); + self.processBatch(inData.params, inData.skipped); }); }; @@ -202,11 +202,12 @@ * * @param {Object} parameters */ - ContentUpgrade.prototype.processBatch = function (parameters) { + ContentUpgrade.prototype.processBatch = function (parameters, skipped) { var self = this; // Track upgraded params self.upgraded = {}; + self.skipped = skipped; // Track current batch self.parameters = parameters; @@ -276,7 +277,7 @@ }, function done(err, result) { if (err) { self.printError(err); - return ; + result = null; } self.workDone(id, result); @@ -291,7 +292,12 @@ var self = this; self.working--; - self.upgraded[id] = result; + if (result === null) { + self.skipped.push(id); + } + else { + self.upgraded[id] = result; + } // Update progress message self.throbber.setProgress(Math.round((info.total - self.left + self.current) / (info.total / 100)) + ' %'); @@ -302,6 +308,7 @@ self.nextBatch({ libraryId: self.version.libraryId, token: self.token, + skipped: JSON.stringify(self.skipped), params: JSON.stringify(self.upgraded) }); } @@ -410,14 +417,25 @@ ContentUpgrade.prototype.printError = function (error) { var self = this; - if (error.type === 'errorParamsBroken') { - error = info.errorContent.replace('%id', error.id) + ' ' + info.errorParamsBroken; - } - else if (error.type === 'scriptMissing') { - error = info.errorScript.replace('%lib', error.library); + switch (error.type) { + case 'errorParamsBroken': + error = info.errorContent.replace('%id', error.id) + ' ' + info.errorParamsBroken; + break; + + case 'scriptMissing': + error = info.errorScript.replace('%lib', error.library); + break; + + case 'errorTooHighVersion': + error = info.errorContent.replace('%id', error.id) + ' ' + info.errorTooHighVersion.replace('%used', error.used).replace('%supported', error.supported); + break; + + case 'errorNotSupported': + error = info.errorContent.replace('%id', error.id) + ' ' + info.errorNotSupported.replace('%used', error.used); + break; } - self.setStatus('

' + info.error + '
' + error + '

'); + $('
  • ' + info.error + '
    ' + error + '
    • ').appendTo($log); }; })(H5P.jQuery, H5P.Version); From f96d04cc27d08ea7675251fc1055c979f99cedb1 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Fri, 1 Feb 2019 13:03:27 +0100 Subject: [PATCH 3/3] JI-942 Add missing library error message --- js/h5p-content-upgrade-process.js | 6 ++++++ js/h5p-content-upgrade.js | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/js/h5p-content-upgrade-process.js b/js/h5p-content-upgrade-process.js index 6954622..cab1dfd 100644 --- a/js/h5p-content-upgrade-process.js +++ b/js/h5p-content-upgrade-process.js @@ -54,6 +54,12 @@ H5P.ContentUpgradeProcess = (function (Version) { if (err) { return done(err); } + if (library.semantics === null) { + return done({ + type: 'libraryMissing', + library: library.name + ' ' + library.version.major + '.' + library.version.minor + }); + } // Run upgrade routines on params self.processParams(library, oldVersion, newVersion, params, metadata, function (err, params, metadata) { diff --git a/js/h5p-content-upgrade.js b/js/h5p-content-upgrade.js index 168c694..9dc066c 100644 --- a/js/h5p-content-upgrade.js +++ b/js/h5p-content-upgrade.js @@ -422,6 +422,10 @@ error = info.errorContent.replace('%id', error.id) + ' ' + info.errorParamsBroken; break; + case 'libraryMissing': + error = info.errorLibrary.replace('%lib', error.library); + break; + case 'scriptMissing': error = info.errorScript.replace('%lib', error.library); break;