Merge branch 'MDL-51571_ltiErrorHandler' of https://github.com/moodlerooms/moodle

mod/lti/classes/service_exception_handler.php

@@ -0,0 +1,122 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Exception handler for LTI services
+ *
+ * @package   mod_lti
+ * @copyright Copyright (c) 2015 Moodlerooms Inc. (http://www.moodlerooms.com)
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace mod_lti;
+
+defined('MOODLE_INTERNAL') || die();
+
+require_once(__DIR__.'/../locallib.php');
+require_once(__DIR__.'/../servicelib.php');
+
+/**
+ * Handles exceptions when handling incoming LTI messages.
+ *
+ * Ensures that LTI always returns a XML message that can be consumed by the caller.
+ *
+ * @package   mod_lti
+ * @copyright Copyright (c) 2015 Moodlerooms Inc. (http://www.moodlerooms.com)
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class service_exception_handler {
+    /**
+     * Enable error response logging.
+     *
+     * @var bool
+     */
+    protected $log = false;
+
+    /**
+     * The LTI service message ID, if known.
+     *
+     * @var string
+     */
+    protected $id = '';
+
+    /**
+     * The LTI service message type, if known.
+     *
+     * @var string
+     */
+    protected $type = 'unknownRequest';
+
+    /**
+     * Constructor.
+     *
+     * @param boolean $log Enable error response logging.
+     */
+    public function __construct($log) {
+        $this->log = $log;
+    }
+
+    /**
+     * Set the LTI message ID being handled.
+     *
+     * @param string $id
+     */
+    public function set_message_id($id) {
+        if (!empty($id)) {
+            $this->id = $id;
+        }
+    }
+
+    /**
+     * Set the LTI message type being handled.
+     *
+     * @param string $type
+     */
+    public function set_message_type($type) {
+        if (!empty($type)) {
+            $this->type = $type;
+        }
+    }
+
+    /**
+     * Echo an exception message encapsulated in XML
+     *
+     * @param \Exception $exception The exception that was thrown
+     */
+    public function handle(\Exception $exception) {
+        $message = $exception->getMessage();
+
+        // Add the exception backtrace for developers.
+        if (debugging('', DEBUG_DEVELOPER)) {
+            $message .= "\n".format_backtrace(get_exception_info($exception)->backtrace, true);
+        }
+
+        // Switch to response.
+        $type = str_replace('Request', 'Response', $this->type);
+
+        // Build the appropriate xml.
+        $response = lti_get_response_xml('failure', $message, $this->id, $type);
+
+        $xml = $response->asXML();
+
+        // Log the request if necessary.
+        if ($this->log) {
+            lti_log_response($xml, $exception);
+        }
+
+        echo $xml;
+    }
+}

mod/lti/locallib.php

@@ -1913,7 +1913,43 @@ function lti_should_log_request($rawbody) {
 function lti_log_request($rawbody) {
     if ($tempdir = make_temp_directory('mod_lti', false)) {
         if ($tempfile = tempnam($tempdir, 'mod_lti_request'.date('YmdHis'))) {
-            file_put_contents($tempfile, $rawbody);
+            $content  = "Request Headers:\n";
+            foreach (moodle\mod\lti\OAuthUtil::get_headers() as $header => $value) {
+                $content .= "$header: $value\n";
+            }
+            $content .= "Request Body:\n";
+            $content .= $rawbody;
+
+            file_put_contents($tempfile, $content);
+            chmod($tempfile, 0644);
+        }
+    }
+}
+
+/**
+ * Log an LTI response
+ *
+ * @param string $responsexml The response XML
+ * @param Exception $e If there was an exception, pass that too
+ */
+function lti_log_response($responsexml, $e = null) {
+    if ($tempdir = make_temp_directory('mod_lti', false)) {
+        if ($tempfile = tempnam($tempdir, 'mod_lti_response'.date('YmdHis'))) {
+            $content = '';
+            if ($e instanceof Exception) {
+                $info = get_exception_info($e);
+
+                $content .= "Exception:\n";
+                $content .= "Message: $info->message\n";
+                $content .= "Debug info: $info->debuginfo\n";
+                $content .= "Backtrace:\n";
+                $content .= format_backtrace($info->backtrace, true);
+                $content .= "\n";
+            }
+            $content .= "Response XML:\n";
+            $content .= $responsexml;
+
+            file_put_contents($tempfile, $content);
             chmod($tempfile, 0644);
         }
     }

mod/lti/service.php

@@ -31,11 +31,18 @@ require_once($CFG->dirroot.'/mod/lti/locallib.php');
 require_once($CFG->dirroot.'/mod/lti/servicelib.php');
 
 // TODO: Switch to core oauthlib once implemented - MDL-30149.
+use mod_lti\service_exception_handler;
 use moodle\mod\lti as lti;
 
 $rawbody = file_get_contents("php://input");
 
-if (lti_should_log_request($rawbody)) {
+$logrequests  = lti_should_log_request($rawbody);
+$errorhandler = new service_exception_handler($logrequests);
+
+// Register our own error handler so we can always send valid XML response.
+set_exception_handler(array($errorhandler, 'handle'));
+
+if ($logrequests) {
     lti_log_request($rawbody);
 }
 
@@ -73,20 +80,13 @@ foreach ($body->children() as $child) {
     $messagetype = $child->getName();
 }
 
+// We know more about the message, update error handler to send better errors.
+$errorhandler->set_message_id(lti_parse_message_id($xml));
+$errorhandler->set_message_type($messagetype);
+
 switch ($messagetype) {
     case 'replaceResultRequest':
-        try {
-            $parsed = lti_parse_grade_replace_message($xml);
-        } catch (Exception $e) {
-            $responsexml = lti_get_response_xml(
-                'failure',
-                $e->getMessage(),
-                uniqid(),
-                'replaceResultResponse');
-
-            echo $responsexml->asXML();
-            break;
-        }
+        $parsed = lti_parse_grade_replace_message($xml);
 
         $ltiinstance = $DB->get_record('lti', array('id' => $parsed->instanceid));
 
@@ -99,8 +99,12 @@ switch ($messagetype) {
 
         $gradestatus = lti_update_grade($ltiinstance, $parsed->userid, $parsed->launchid, $parsed->gradeval);
 
+        if (!$gradestatus) {
+            throw new Exception('Grade replace response');
+        }
+
         $responsexml = lti_get_response_xml(
-                $gradestatus ? 'success' : 'failure',
+                'success',
                 'Grade replace response',
                 $parsed->messageid,
                 'replaceResultResponse'
@@ -157,8 +161,12 @@ switch ($messagetype) {
 
         $gradestatus = lti_delete_grade($ltiinstance, $parsed->userid);
 
+        if (!$gradestatus) {
+            throw new Exception('Grade delete request');
+        }
+
         $responsexml = lti_get_response_xml(
-                $gradestatus ? 'success' : 'failure',
+                'success',
                 'Grade delete request',
                 $parsed->messageid,
                 'deleteResultResponse'

mod/lti/servicelib.php

@@ -57,6 +57,10 @@ function lti_get_response_xml($codemajor, $description, $messageref, $messagetyp
 }
 
 function lti_parse_message_id($xml) {
+    if (empty($xml->imsx_POXHeader)) {
+        return '';
+    }
+
     $node = $xml->imsx_POXHeader->imsx_POXRequestHeaderInfo->imsx_messageIdentifier;
     $messageid = (string)$node;
 
@@ -282,29 +286,14 @@ function lti_verify_sourcedid($ltiinstance, $parsed) {
 function lti_extend_lti_services($data) {
     $plugins = get_plugin_list_with_function('ltisource', $data->messagetype);
     if (!empty($plugins)) {
-        try {
-            // There can only be one.
-            if (count($plugins) > 1) {
-                throw new coding_exception('More than one ltisource plugin handler found');
-            }
-            $data->xml = new SimpleXMLElement($data->body);
-            $callback = current($plugins);
-            call_user_func($callback, $data);
-        } catch (moodle_exception $e) {
-            $error = $e->getMessage();
-            if (debugging('', DEBUG_DEVELOPER)) {
-                $error .= ' '.format_backtrace(get_exception_info($e)->backtrace);
-            }
-            $responsexml = lti_get_response_xml(
-                'failure',
-                $error,
-                $data->messageid,
-                $data->messagetype
-            );
-
-            header('HTTP/1.0 400 bad request');
-            echo $responsexml->asXML();
+        // There can only be one.
+        if (count($plugins) > 1) {
+            throw new coding_exception('More than one ltisource plugin handler found');
         }
+        $data->xml = new SimpleXMLElement($data->body);
+        $callback = current($plugins);
+        call_user_func($callback, $data);
+
         return true;
     }
     return false;

mod/lti/tests/service_exception_handler_test.php

@@ -0,0 +1,85 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Tests Exception handler for LTI services
+ *
+ * @package   mod_lti
+ * @copyright Copyright (c) 2015 Moodlerooms Inc. (http://www.moodlerooms.com)
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+use mod_lti\service_exception_handler;
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Tests Exception handler for LTI services
+ *
+ * @package   mod_lti
+ * @copyright Copyright (c) 2015 Moodlerooms Inc. (http://www.moodlerooms.com)
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class mod_lti_service_exception_handler_testcase extends advanced_testcase {
+    /**
+     * Testing service error handling.
+     */
+    public function test_handle() {
+        $handler = new service_exception_handler(false);
+        $handler->set_message_id('123');
+        $handler->set_message_type('testRequest');
+        $handler->handle(new Exception('Error happened'));
+
+        $this->expectOutputRegex('/imsx_codeMajor>failure/');
+        $this->expectOutputRegex('/imsx_description>Error happened/');
+        $this->expectOutputRegex('/imsx_messageRefIdentifier>123/');
+        $this->expectOutputRegex('/imsx_operationRefIdentifier>testRequest/');
+        $this->expectOutputRegex('/imsx_POXBody><testResponse/');
+    }
+
+    /**
+     * Testing service error handling when message ID and type are not known yet.
+     */
+    public function test_handle_early_error() {
+        $handler = new service_exception_handler(false);
+        $handler->handle(new Exception('Error happened'));
+
+        $this->expectOutputRegex('/imsx_codeMajor>failure/');
+        $this->expectOutputRegex('/imsx_description>Error happened/');
+        $this->expectOutputRegex('/imsx_messageRefIdentifier\/>/');
+        $this->expectOutputRegex('/imsx_operationRefIdentifier>unknownRequest/');
+        $this->expectOutputRegex('/imsx_POXBody><unknownResponse/');
+    }
+
+    /**
+     * Testing that a log file is generated when logging is turned on.
+     */
+    public function test_handle_log() {
+        global $CFG;
+
+        $this->resetAfterTest();
+
+        $handler = new service_exception_handler(true);
+
+        ob_start();
+        $handler->handle(new Exception('Error happened'));
+        ob_end_clean();
+
+        $this->assertTrue(is_dir($CFG->dataroot.'/temp/mod_lti'));
+        $files = glob($CFG->dataroot.'/temp/mod_lti/mod_lti_response*');
+        $this->assertEquals(1, count($files));
+    }
+}

mod/lti/tests/servicelib_test.php

@@ -0,0 +1,115 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Tests for servicelib.php
+ *
+ * @package   mod_lti
+ * @copyright Copyright (c) 2015 Moodlerooms Inc. (http://www.moodlerooms.com)
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+
+require_once($CFG->dirroot.'/mod/lti/servicelib.php');
+
+/**
+ * Tests for servicelib.php
+ *
+ * @package   mod_lti
+ * @copyright Copyright (c) 2015 Moodlerooms Inc. (http://www.moodlerooms.com)
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class mod_lti_servicelib_testcase extends basic_testcase {
+    /**
+     * Test that lti_parse_message_id never fails with good and bad XML.
+     *
+     * @dataProvider message_id_provider
+     * @param mixed $expected Expected message ID.
+     * @param string $xml XML to parse.
+     */
+    public function test_lti_parse_message_id($expected, $xml) {
+        $xml = simplexml_load_string($xml);
+        $this->assertEquals($expected, lti_parse_message_id($xml));
+    }
+
+    /**
+     * Test data provider for testing lti_parse_message_id
+     *
+     * @return array
+     */
+    public function message_id_provider() {
+        $valid = <<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<imsx_POXEnvelopeRequest xmlns="http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0">
+    <imsx_POXHeader>
+        <imsx_POXRequestHeaderInfo>
+            <imsx_version>V1.0</imsx_version>
+            <imsx_messageIdentifier>9999</imsx_messageIdentifier>
+        </imsx_POXRequestHeaderInfo>
+    </imsx_POXHeader>
+    <imsx_POXBody/>
+</imsx_POXEnvelopeRequest>
+XML;
+
+        $noheader = <<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<imsx_POXEnvelopeRequest xmlns="http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0">
+    <badXmlHere>
+        <imsx_POXRequestHeaderInfo>
+            <imsx_version>V1.0</imsx_version>
+            <imsx_messageIdentifier>9999</imsx_messageIdentifier>
+        </imsx_POXRequestHeaderInfo>
+    </badXmlHere>
+    <imsx_POXBody/>
+</imsx_POXEnvelopeRequest>
+XML;
+
+        $noinfo = <<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<imsx_POXEnvelopeRequest xmlns="http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0">
+    <imsx_POXHeader>
+        <badXmlHere>
+            <imsx_version>V1.0</imsx_version>
+            <imsx_messageIdentifier>9999</imsx_messageIdentifier>
+        </badXmlHere>
+    </imsx_POXHeader>
+    <imsx_POXBody/>
+</imsx_POXEnvelopeRequest>
+XML;
+
+        $noidentifier = <<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<imsx_POXEnvelopeRequest xmlns="http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0">
+    <imsx_POXHeader>
+        <imsx_POXRequestHeaderInfo>
+            <imsx_version>V1.0</imsx_version>
+        </imsx_POXRequestHeaderInfo>
+    </imsx_POXHeader>
+    <imsx_POXBody/>
+</imsx_POXEnvelopeRequest>
+XML;
+
+        return array(
+            array(9999, $valid),
+            array('', $noheader),
+            array('', $noinfo),
+            array('', $noidentifier),
+        );
+    }
+}