From 123cc912ecf8b6e848a6b02ad3f8afa3d0c3f8ca Mon Sep 17 00:00:00 2001
From: Mark Nielsen <mark@moodlerooms.com>
Date: Fri, 22 Jul 2016 11:10:10 -0700
Subject: [PATCH] MDL-55314 mod_lti: Several fixes to lti_load_cartridge

* Fixed XML loading when there is an error
* Fixed exception thrown by lti_load_cartridge
* By default, don't allow LTI mod generator to
  use a toolurl as that results in a cURL call.
---
 mod/lti/locallib.php               | 16 ++++++++++++----
 mod/lti/tests/externallib_test.php |  3 ++-
 mod/lti/tests/generator/lib.php    |  2 +-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php
index 092ac4c0ed5..ddec86456c4 100644
--- a/mod/lti/locallib.php
+++ b/mod/lti/locallib.php
@@ -2700,24 +2700,32 @@ function lti_load_tool_from_cartridge($url, $lti) {
 function lti_load_cartridge($url, $map, $propertiesmap = array()) {
     global $CFG;
     require_once($CFG->libdir. "/filelib.php");
-    // TODO MDL-46023 Replace this code with a call to the new library.
-    $origentity = libxml_disable_entity_loader(true);
 
     $curl = new curl();
     $response = $curl->get($url);
 
+    // TODO MDL-46023 Replace this code with a call to the new library.
+    $origerrors = libxml_use_internal_errors(true);
+    $origentity = libxml_disable_entity_loader(true);
+    libxml_clear_errors();
+
     $document = new DOMDocument();
     @$document->loadXML($response, LIBXML_DTDLOAD | LIBXML_DTDATTR);
 
     $cartridge = new DomXpath($document);
 
     $errors = libxml_get_errors();
+
+    libxml_clear_errors();
+    libxml_use_internal_errors($origerrors);
+    libxml_disable_entity_loader($origentity);
+
     if (count($errors) > 0) {
         $message = 'Failed to load cartridge.';
         foreach ($errors as $error) {
             $message .= "\n" . trim($error->message, "\n\r\t .") . " at line " . $error->line;
         }
-        throw new moodle_exception($message);
+        throw new moodle_exception('errorreadingfile', '', '', $url, $message);
     }
 
     $toolinfo = array();
@@ -2735,7 +2743,7 @@ function lti_load_cartridge($url, $map, $propertiesmap = array()) {
             }
         }
     }
-    libxml_disable_entity_loader($origentity);
+
     return $toolinfo;
 }
 
diff --git a/mod/lti/tests/externallib_test.php b/mod/lti/tests/externallib_test.php
index 07fc5b47ce4..700feb3f4a7 100644
--- a/mod/lti/tests/externallib_test.php
+++ b/mod/lti/tests/externallib_test.php
@@ -52,7 +52,8 @@ class mod_lti_external_testcase extends externallib_advanced_testcase {
 
         // Setup test data.
         $this->course = $this->getDataGenerator()->create_course();
-        $this->lti = $this->getDataGenerator()->create_module('lti', array('course' => $this->course->id));
+        $this->lti = $this->getDataGenerator()->create_module('lti',
+            array('course' => $this->course->id, 'toolurl' => 'http://localhost/not/real/tool.php'));
         $this->context = context_module::instance($this->lti->cmid);
         $this->cm = get_coursemodule_from_instance('lti', $this->lti->id);
 
diff --git a/mod/lti/tests/generator/lib.php b/mod/lti/tests/generator/lib.php
index 4703f91403c..a8963284c04 100644
--- a/mod/lti/tests/generator/lib.php
+++ b/mod/lti/tests/generator/lib.php
@@ -41,7 +41,7 @@ class mod_lti_generator extends testing_module_generator {
         $record  = (object) (array) $record;
 
         if (!isset($record->toolurl)) {
-            $record->toolurl = 'http://www.imsglobal.org/developers/LTI/test/v1p1/tool.php';
+            $record->toolurl = '';
         }
         if (!isset($record->resourcekey)) {
             $record->resourcekey = '12345';