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

LTI can cause libxml to break

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide
      1. Run unit tests.
      2. Turn cache js off (most developers will have this off already) Site administration ▶︎ Appearance ▶︎ AJAX and Javascript
      3. Apply this diff (easy to type manually if you prefer)

        diff --git a/mod/lti/amd/src/cartridge_registration_form.js b/mod/lti/amd/src/cartridge_registration_form.js
        index 7af41a6..acf2729 100644
        --- a/mod/lti/amd/src/cartridge_registration_form.js
        +++ b/mod/lti/amd/src/cartridge_registration_form.js
        @@ -160,7 +160,8 @@ define(['jquery', 'core/ajax', 'core/notification', 'mod_lti/tool_type', 'mod_lt
                             message: s
                         });
                     }).fail(notification.exception);
        -        }).fail(function() {
        +        }).fail(function(error) {
        +            console.log(error);
                     str.get_string('failedtocreatetooltype', 'mod_lti').done(function(s) {
                         $(document).trigger(ltiEvents.NEW_TOOL_TYPE);
                         $(document).trigger(ltiEvents.STOP_CARTRIDGE_REGISTRATION);
        

      4. Go to Site administration ▶︎ Plugins ▶︎ Activity modules ▶︎ External tool ▶︎ Manage tools
      5. Add the url to the attached broken.xml
      6. Make sure you see the errors in the console output. (Should be in debuginfo)

      Regression test, check out master branch again (so the changes are no longer there)

      1. Go to Site administration ▶︎ Plugins ▶︎ Activity modules ▶︎ External tool ▶︎ Manage tools
      2. Add a cartridge known to work (e.g. one from https://www.eduappcenter.com/ )
      3. Ensure it adds the tool type successfully
      4. Add a xml file that shouldn't work (e.g. http://www.w3schools.com/xml/note.xml )
      5. Ensure you receive an error and the tool type isn't added.
      Show
      Run unit tests. Turn cache js off (most developers will have this off already) Site administration ▶︎ Appearance ▶︎ AJAX and Javascript Apply this diff (easy to type manually if you prefer) diff --git a/mod/lti/amd/src/cartridge_registration_form.js b/mod/lti/amd/src/cartridge_registration_form.js index 7af41a6..acf2729 100644 --- a/mod/lti/amd/src/cartridge_registration_form.js +++ b/mod/lti/amd/src/cartridge_registration_form.js @@ -160,7 +160,8 @@ define(['jquery', 'core/ajax', 'core/notification', 'mod_lti/tool_type', 'mod_lt message: s }); }).fail(notification.exception); - }).fail(function() { + }).fail(function(error) { + console.log(error); str.get_string('failedtocreatetooltype', 'mod_lti').done(function(s) { $(document).trigger(ltiEvents.NEW_TOOL_TYPE); $(document).trigger(ltiEvents.STOP_CARTRIDGE_REGISTRATION); Go to Site administration ▶︎ Plugins ▶︎ Activity modules ▶︎ External tool ▶︎ Manage tools Add the url to the attached broken.xml Make sure you see the errors in the console output. (Should be in debuginfo) Regression test , check out master branch again (so the changes are no longer there) Go to Site administration ▶︎ Plugins ▶︎ Activity modules ▶︎ External tool ▶︎ Manage tools Add a cartridge known to work (e.g. one from https://www.eduappcenter.com/ ) Ensure it adds the tool type successfully Add a xml file that shouldn't work (e.g. http://www.w3schools.com/xml/note.xml ) Ensure you receive an error and the tool type isn't added.
    • Affected Branches:
      MOODLE_31_STABLE, MOODLE_32_STABLE
    • Fixed Branches:
      MOODLE_31_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-55314-master

      Description

      The method lti_load_cartridge doesn't undo it's changes to libxml_disable_entity_loader if there a problem with the response from the curl.

      Also found problems with the exception in lti_load_cartridge (should be a coding_exception) and due to this method being called, several curls are being called unnecessarily during unit testing.

      Example of how lti_load_cartridge can cause problems: First call lti_load_cartridge where it throw an error (EG: downloads invalid XML or some HTML). This will mean that the change to libxml_disable_entity_loader was not reverted. This then causes code like this to fail (it will say that the XML file cannot be opened):

      $reader = new XMLReader();
      $reader->open(__DIR__.'/path/to/file.xml');
      

      In addition, the libxml_disable_entity_loader is not thread safe, so any active threads on the PHP server will also have this change, causing them to inconsistently fail to read XML.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Sep/16