From 6b7e523b5d3d8ea5d35cdbd70f7863d5b8ecfaf8 Mon Sep 17 00:00:00 2001
From: Frederic Massart <fred@moodle.com>
Date: Fri, 18 Nov 2016 14:17:59 +0800
Subject: [PATCH] MDL-56182 mod_lti: Include the query string when comparing
 type URLs

---
 mod/lti/locallib.php            |  12 +++-
 mod/lti/tests/locallib_test.php | 114 +++++++++++++++++++++++++++++++-
 2 files changed, 124 insertions(+), 2 deletions(-)

diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php
index 7012c17134f..a2e07ecfa3d 100644
--- a/mod/lti/locallib.php
+++ b/mod/lti/locallib.php
@@ -1547,6 +1547,10 @@ function lti_get_url_thumbprint($url) {
         $urlparts['path'] = '';
     }
 
+    if (!isset($urlparts['query'])) {
+        $urlparts['query'] = '';
+    }
+
     if (!isset($urlparts['host'])) {
         $urlparts['host'] = '';
     }
@@ -1555,7 +1559,13 @@ function lti_get_url_thumbprint($url) {
         $urlparts['host'] = substr($urlparts['host'], 4);
     }
 
-    return $urllower = $urlparts['host'] . '/' . $urlparts['path'];
+    $urllower = $urlparts['host'] . '/' . $urlparts['path'];
+
+    if ($urlparts['query'] != '') {
+        $urllower .= '?' . $urlparts['query'];
+    }
+
+    return $urllower;
 }
 
 function lti_get_best_tool_by_url($url, $tools, $courseid = null) {
diff --git a/mod/lti/tests/locallib_test.php b/mod/lti/tests/locallib_test.php
index 07592fa804e..5a471c01d47 100644
--- a/mod/lti/tests/locallib_test.php
+++ b/mod/lti/tests/locallib_test.php
@@ -185,7 +185,8 @@ class mod_lti_locallib_testcase extends advanced_testcase {
         $this->assertEquals('moodle.org//this/is/moodle', lti_get_url_thumbprint('http://moodle.org/this/is/moodle'));
         $this->assertEquals('moodle.org//this/is/moodle', lti_get_url_thumbprint('https://moodle.org/this/is/moodle'));
         $this->assertEquals('moodle.org//this/is/moodle', lti_get_url_thumbprint('moodle.org/this/is/moodle'));
-        $this->assertEquals('moodle.org//this/is/moodle', lti_get_url_thumbprint('moodle.org/this/is/moodle?foo=bar'));
+        $this->assertEquals('moodle.org//this/is/moodle', lti_get_url_thumbprint('moodle.org/this/is/moodle?'));
+        $this->assertEquals('moodle.org//this/is/moodle?foo=bar', lti_get_url_thumbprint('moodle.org/this/is/moodle?foo=bar'));
     }
 
     /*
@@ -482,4 +483,115 @@ class mod_lti_locallib_testcase extends advanced_testcase {
         $this->expectException('coding_exception');
         lti_build_content_item_selection_request($typeid, $course, $returnurl, '', '', [], $targets);
     }
+
+    /**
+     * Provider for test_lti_get_best_tool_by_url.
+     *
+     * @return array of [urlToTest, expectedTool, allTools]
+     */
+    public function lti_get_best_tool_by_url_provider() {
+        $tools = [
+            (object) [
+                'name' => 'Here',
+                'baseurl' => 'https://example.com/i/am/?where=here',
+                'tooldomain' => 'example.com',
+                'state' => LTI_TOOL_STATE_CONFIGURED,
+                'course' => SITEID
+            ],
+            (object) [
+                'name' => 'There',
+                'baseurl' => 'https://example.com/i/am/?where=there',
+                'tooldomain' => 'example.com',
+                'state' => LTI_TOOL_STATE_CONFIGURED,
+                'course' => SITEID
+            ],
+            (object) [
+                'name' => 'Not here',
+                'baseurl' => 'https://example.com/i/am/?where=not/here',
+                'tooldomain' => 'example.com',
+                'state' => LTI_TOOL_STATE_CONFIGURED,
+                'course' => SITEID
+            ],
+            (object) [
+                'name' => 'Here',
+                'baseurl' => 'https://example.com/i/am/',
+                'tooldomain' => 'example.com',
+                'state' => LTI_TOOL_STATE_CONFIGURED,
+                'course' => SITEID
+            ],
+            (object) [
+                'name' => 'Here',
+                'baseurl' => 'https://example.com/i/was',
+                'tooldomain' => 'example.com',
+                'state' => LTI_TOOL_STATE_CONFIGURED,
+                'course' => SITEID
+            ],
+            (object) [
+                'name' => 'Here',
+                'baseurl' => 'https://badexample.com/i/am/?where=here',
+                'tooldomain' => 'badexample.com',
+                'state' => LTI_TOOL_STATE_CONFIGURED,
+                'course' => SITEID
+            ],
+        ];
+
+        $data = [
+            [
+                'url' => $tools[0]->baseurl,
+                'expected' => $tools[0],
+            ],
+            [
+                'url' => $tools[1]->baseurl,
+                'expected' => $tools[1],
+            ],
+            [
+                'url' => $tools[2]->baseurl,
+                'expected' => $tools[2],
+            ],
+            [
+                'url' => $tools[3]->baseurl,
+                'expected' => $tools[3],
+            ],
+            [
+                'url' => $tools[4]->baseurl,
+                'expected' => $tools[4],
+            ],
+            [
+                'url' => $tools[5]->baseurl,
+                'expected' => $tools[5],
+            ],
+            [
+                'url' => 'https://nomatch.com/i/am/',
+                'expected' => null
+            ],
+            [
+                'url' => 'https://example.com',
+                'expected' => null
+            ],
+            [
+                'url' => 'https://example.com/i/am/?where=unknown',
+                'expected' => $tools[3]
+            ]
+        ];
+
+        // Construct the final array as required by the provider API. Each row
+        // of the array contains the URL to test, the expected tool, and
+        // the complete list of tools.
+        return array_map(function($data) use ($tools) {
+            return [$data['url'], $data['expected'], $tools];
+        }, $data);
+    }
+
+    /**
+     * Test lti_get_best_tool_by_url.
+     *
+     * @dataProvider lti_get_best_tool_by_url_provider
+     * @param string $url The URL to test.
+     * @param object $expected The expected tool matching the URL.
+     * @param array $tools The pool of tools to match the URL with.
+     */
+    public function test_lti_get_best_tool_by_url($url, $expected, $tools) {
+        $actual = lti_get_best_tool_by_url($url, $tools, null);
+        $this->assertSame($expected, $actual);
+    }
 }