Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-69415

H5P has namespace overlap with mod_hvp, causing unexpected behavior

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      Setup

      1. Download the mod_hvp plugin from the Moodle DB and install it.
      2. Edit (just for testing this patch) the mod/hvp/autoloader.php file and add the missing code (if ($xxx = 'H5peditor') { debugging....):

          if (isset($classmap[$class])) {
              if ($class == 'H5peditor') {
                  debugging('MOD HVP!!!!!');
              }
              require_once($CFG->dirroot . '/mod/hvp/' . $classmap[$class]);
          }
      

      Testing instructions

      1. Login as admin.
      2. Access to the Content bank.
      3. Upload the attached H5P file.
      4. Purge caches.
      5. Click the "Edit" button.
      6. Check the debugging error "MOD HVP!!!!!" is not displayed.
      7. Modify the H5P content title. 
      8. Purge caches.
      9. Save changes.
      10. Check the debugging error "MOD HVP!!!!!" is not displayed.
      Show
      Setup Download the mod_hvp plugin from the Moodle DB  and install it. Edit (just for testing this patch) the mod/hvp/autoloader.php file and add the missing code ( if ($xxx = 'H5peditor') { debugging.... ): if (isset($classmap[$class])) { if ($class == 'H5peditor') { debugging('MOD HVP!!!!!'); } require_once($CFG->dirroot . '/mod/hvp/' . $classmap[$class]); } Testing instructions Login as admin. Access to the Content bank. Upload the attached H5P file. Purge caches. Click the "Edit" button. Check the debugging error "MOD HVP!!!!!" is not displayed. Modify the H5P content title.  Purge caches. Save changes. Check the debugging error "MOD HVP!!!!!" is not displayed.
    • Affected Branches:
      MOODLE_39_STABLE
    • Fixed Branches:
      MOODLE_310_STABLE, MOODLE_39_STABLE
    • Pull from Repository:
    • Pull 3.9 Branch:
    • Pull 3.10 Branch:
      MDL-69415-310
    • Pull 3.11 Branch:
      MDL-69415-311
    • Pull Master Branch:
      MDL-69415-master
    • Sprint:
      Moppies Kanban

      Description

      We had an issue where some users were getting a "Invalid security token" error when trying to add media to H5P in the Content bank.

      After quite a bit of digging, we found that basically, both mod_hvp, and core_h5p include core H5P classes, like \H5PCore, \H5PContentValidator, etc.

      Each includes their own autoloader.

      The problem comes in on some pages, the mod_hvp autoloader takes precedence, meaning that core_h5p\core inherits from mod_hvp's H5PCore, instead one included with Moodle. Specifically, it seems to happen (sometimes) when loading the content bank editing page, but not when ajax calls are made (I assume because the ajax calls load much few files/libraries in general).

      In this particular case, H5PCore::hashToken() behaves differently between the two libraries, meaning that the token would be generated with one method, and validated with another.

      Now, we rolled out a quick fix of just copying the core's version of hashToken into mod_hvp, but that doesn't fix the underlying problem of the overlapping, and unpredictable, namespace.

      Another class name clash error reported in https://github.com/h5p/moodle-mod_hvp/issues/347 when running cron :

      Fatal error: Cannot declare interface H5PFileStorage, because the name is already in use in /var/www/html/moodle38/mod/hvp/library/h5p-file-storage.interface.php on line 10
      

        Attachments

          Activity

            People

            Assignee:
            sarjona Sara Arjona (@sarjona)
            Reporter:
            emerrill Eric Merrill
            Peer reviewer:
            Carlos Escobedo
            Integrator:
            Andrew Nicols
            Tester:
            Gladys Basiana
            Participants:
            Component watchers:
            Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Ilya Tregubov, Sara Arjona (@sarjona), Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Ilya Tregubov, Sara Arjona (@sarjona)
            Votes:
            11 Vote for this issue
            Watchers:
            22 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Fix Release Date:
              10/May/21

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 3 hours, 35 minutes
                3h 35m