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

              Assignee:
              bushido Mark Nielsen
              Reporter:
              bushido Mark Nielsen
              Peer reviewer:
              John Okely
              Integrator:
              Eloy Lafuente (stronk7)
              Tester:
              Jake Dallimore
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

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