Skip to content

Commit

Permalink
MDL-55314 mod_lti: Several fixes to lti_load_cartridge
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
polothy committed Jul 27, 2016
1 parent 90a8bdb commit 123cc91
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
16 changes: 12 additions & 4 deletions mod/lti/locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -2735,7 +2743,7 @@ function lti_load_cartridge($url, $map, $propertiesmap = array()) {
}
}
}
libxml_disable_entity_loader($origentity);

return $toolinfo;
}

Expand Down
3 changes: 2 additions & 1 deletion mod/lti/tests/externallib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public function setUp() {

// 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);

Expand Down
2 changes: 1 addition & 1 deletion mod/lti/tests/generator/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function create_instance($record = null, array $options = null) {
$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';
Expand Down

0 comments on commit 123cc91

Please sign in to comment.