From 4a74ab0200d472f600d238787ebbc5e782b2135d Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 3 Jun 2018 21:06:45 +0800 Subject: [PATCH 1/6] MDL-36754 core_files: Add a token version of pluginfile --- files/classes/privacy/provider.php | 95 +++++++++++++++++++++++++++++- lang/en/files.php | 1 + lib/moodlelib.php | 7 ++- pluginfile.php | 8 ++- tokenpluginfile.php | 44 ++++++++++++++ 5 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 tokenpluginfile.php diff --git a/files/classes/privacy/provider.php b/files/classes/privacy/provider.php index 2bf2dbbe460..865d75fbe25 100644 --- a/files/classes/privacy/provider.php +++ b/files/classes/privacy/provider.php @@ -27,6 +27,8 @@ namespace core_files\privacy; defined('MOODLE_INTERNAL') || die(); use core_privacy\local\metadata\collection; +use core_privacy\local\request\contextlist; +use core_privacy\local\request\approved_contextlist; /** * Data provider class. @@ -41,7 +43,10 @@ use core_privacy\local\metadata\collection; */ class provider implements \core_privacy\local\metadata\provider, - \core_privacy\local\request\subsystem\plugin_provider { + \core_privacy\local\request\subsystem\plugin_provider, + + // We store a userkey for token-based file access. + \core_privacy\local\request\subsystem\provider { /** * Returns metadata. @@ -65,7 +70,95 @@ class provider implements 'timemodified' => 'privacy:metadata:files:timemodified', ], 'privacy:metadata:files'); + $collection->add_subsystem_link('core_userkey', [], 'privacy:metadata:core_userkey'); + return $collection; } + /** + * Get the list of contexts that contain user information for the specified user. + * + * This is currently just the user context. + * + * @param int $userid The user to search. + * @return contextlist $contextlist The contextlist containing the list of contexts used in this plugin. + */ + public static function get_contexts_for_userid(int $userid) : contextlist { + $sql = "SELECT ctx.id + FROM {user_private_key} k + JOIN {user} u ON k.userid = u.id + JOIN {context} ctx ON ctx.instanceid = u.id AND ctx.contextlevel = :contextlevel + WHERE k.userid = :userid AND k.script = :script"; + $params = [ + 'userid' => $userid, + 'contextlevel' => CONTEXT_USER, + 'script' => 'core_files', + ]; + $contextlist = new contextlist(); + $contextlist->add_from_sql($sql, $params); + + return $contextlist; + } + + /** + * Export all user data for the specified user, in the specified contexts. + * + * @param approved_contextlist $contextlist The approved contexts to export information for. + */ + public static function export_user_data(approved_contextlist $contextlist) { + // If the user has data, then only the CONTEXT_USER should be present so get the first context. + $contexts = $contextlist->get_contexts(); + if (count($contexts) == 0) { + return; + } + + // Sanity check that context is at the user context level, then get the userid. + $context = reset($contexts); + if ($context->contextlevel !== CONTEXT_USER) { + return; + } + + // Export associated userkeys. + $subcontext = [ + get_string('files'), + ]; + \core_userkey\privacy\provider::export_userkeys($context, $subcontext, 'core_files'); + } + + /** + * Delete all use data which matches the specified deletion_criteria. + * + * @param context $context A user context. + */ + public static function delete_data_for_all_users_in_context(\context $context) { + // Sanity check that context is at the user context level, then get the userid. + if ($context->contextlevel !== CONTEXT_USER) { + return; + } + + // Delete all the userkeys. + \core_userkey\privacy\provider::delete_userkeys('core_files', $context->instanceid); + } + + /** + * Delete all user data for the specified user, in the specified contexts. + * + * @param approved_contextlist $contextlist The approved contexts and user information to delete information for. + */ + public static function delete_data_for_user(approved_contextlist $contextlist) { + // If the user has data, then only the user context should be present so get the first context. + $contexts = $contextlist->get_contexts(); + if (count($contexts) == 0) { + return; + } + + // Sanity check that context is at the user context level, then get the userid. + $context = reset($contexts); + if ($context->contextlevel !== CONTEXT_USER) { + return; + } + + // Delete all the userkeys for core_files.. + \core_userkey\privacy\provider::delete_userkeys('core_files', $context->instanceid); + } } diff --git a/lang/en/files.php b/lang/en/files.php index 6cb55ac4e0d..b2e305b483e 100644 --- a/lang/en/files.php +++ b/lang/en/files.php @@ -37,3 +37,4 @@ $string['privacy:metadata:files:source'] = 'The source of the file'; $string['privacy:metadata:files:timecreated'] = 'The time when the file was created'; $string['privacy:metadata:files:timemodified'] = 'The time when the file was last modified'; $string['privacy:metadata:files:userid'] = 'The user who created the file'; +$string['privacy:metadata:core_userkey'] = 'A private token is generated and stored. This token can be used to access Moodle files without requiring you to log in.'; diff --git a/lib/moodlelib.php b/lib/moodlelib.php index e26ef637ae5..4962aa46499 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -3112,9 +3112,10 @@ function validate_user_key($keyvalue, $script, $instance) { * @uses PARAM_ALPHANUM * @param string $script unique script identifier * @param int $instance optional instance id + * @param string $keyvalue The key. If not supplied, this will be fetched from the current session. * @return int Instance ID */ -function require_user_key_login($script, $instance=null) { +function require_user_key_login($script, $instance = null, $keyvalue = null) { global $DB; if (!NO_MOODLE_COOKIES) { @@ -3124,7 +3125,9 @@ function require_user_key_login($script, $instance=null) { // Extra safety. \core\session\manager::write_close(); - $keyvalue = required_param('key', PARAM_ALPHANUM); + if (null === $keyvalue) { + $keyvalue = required_param('key', PARAM_ALPHANUM); + } $key = validate_user_key($keyvalue, $script, $instance); diff --git a/pluginfile.php b/pluginfile.php index 3d6d542496e..24e6cda10e9 100644 --- a/pluginfile.php +++ b/pluginfile.php @@ -25,12 +25,16 @@ */ // Disable moodle specific debug messages and any errors in output. -define('NO_DEBUG_DISPLAY', true); +if (!defined('NO_DEBUG_DISPLAY')) { + define('NO_DEBUG_DISPLAY', true); +} require_once('config.php'); require_once('lib/filelib.php'); -$relativepath = get_file_argument(); +if (empty($relativepath)) { + $relativepath = get_file_argument(); +} $forcedownload = optional_param('forcedownload', 0, PARAM_BOOL); $preview = optional_param('preview', null, PARAM_ALPHANUM); // Offline means download the file from the repository and serve it, even if it was an external link. diff --git a/tokenpluginfile.php b/tokenpluginfile.php new file mode 100644 index 00000000000..156d4126f80 --- /dev/null +++ b/tokenpluginfile.php @@ -0,0 +1,44 @@ +. + +/** + * Entry point for token-based access to pluginfile.php. + * + * @package core + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +// Disable the use of sessions/cookies - we recreate $USER for every call. +define('NO_MOODLE_COOKIES', true); + +// Disable debugging for this script. +// It is typically used to display images. +define('NO_DEBUG_DISPLAY', true); + +require_once('config.php'); + +$relativepath = get_file_argument(); +$token = optional_param('token', '', PARAM_ALPHANUM); +if (0 == strpos($relativepath, '/token/')) { + $relativepath = ltrim($relativepath, '/'); + $pathparts = explode('/', $relativepath, 2); + $token = $pathparts[0]; + $relativepath = "/{$pathparts[1]}"; +} + +require_user_key_login('core_files', null, $token); +require_once('pluginfile.php'); From 91f1f282331aa0e3d575ec80845cd4ce763e81e7 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 3 Jun 2018 21:14:40 +0800 Subject: [PATCH 2/6] MDL-36754 core_files: Support tokens when rewriting text Support use of a token for access to pluginfile.php when rewriting text content. This is primarily intended for notifications where the user is unlikely to be logged into Moodle when receiving the notification. --- lib/filelib.php | 39 ++++++++++++-------- lib/tests/filelib_test.php | 73 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/lib/filelib.php b/lib/filelib.php index a80f3a8ecb7..01b360c8430 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -455,30 +455,41 @@ function file_prepare_draft_area(&$draftitemid, $contextid, $component, $fileare * Passing a new option reverse = true in the $options var will make the function to convert actual URLs in $text to encoded URLs * in the @@PLUGINFILE@@ form. * - * @category files - * @global stdClass $CFG - * @param string $text The content that may contain ULRs in need of rewriting. - * @param string $file The script that should be used to serve these files. pluginfile.php, draftfile.php, etc. - * @param int $contextid This parameter and the next two identify the file area to use. - * @param string $component - * @param string $filearea helps identify the file area. - * @param int $itemid helps identify the file area. - * @param array $options text and file options ('forcehttps'=>false), use reverse = true to reverse the behaviour of the function. - * @return string the processed text. + * @param string $text The content that may contain ULRs in need of rewriting. + * @param string $file The script that should be used to serve these files. pluginfile.php, draftfile.php, etc. + * @param int $contextid This parameter and the next two identify the file area to use. + * @param string $component + * @param string $filearea helps identify the file area. + * @param int $itemid helps identify the file area. + * @param array $options + * bool $options.forcehttps Force the user of https + * bool $options.reverse Reverse the behaviour of the function + * bool $options.includetoken Use a token for authentication + * string The processed text. */ function file_rewrite_pluginfile_urls($text, $file, $contextid, $component, $filearea, $itemid, array $options=null) { - global $CFG; + global $CFG, $USER; $options = (array)$options; if (!isset($options['forcehttps'])) { $options['forcehttps'] = false; } - if (!$CFG->slasharguments) { - $file = $file . '?file='; + $baseurl = "{$CFG->wwwroot}/{$file}"; + if (!empty($options['includetoken'])) { + $token = get_user_key('core_files', $USER->id); + $finalfile = basename($file); + $tokenfile = "token{$finalfile}"; + $file = substr($file, 0, strlen($file) - strlen($finalfile)) . $tokenfile; + + if (!$CFG->slasharguments) { + $baseurl .= "?token={$token}&file="; + } else { + $baseurl .= "/{$token}"; + } } - $baseurl = "$CFG->wwwroot/$file/$contextid/$component/$filearea/"; + $baseurl .= "/{$contextid}/{$component}/{$filearea}/"; if ($itemid !== null) { $baseurl .= "$itemid/"; diff --git a/lib/tests/filelib_test.php b/lib/tests/filelib_test.php index 6931a597b29..d417ad382f6 100644 --- a/lib/tests/filelib_test.php +++ b/lib/tests/filelib_test.php @@ -1052,6 +1052,79 @@ EOF; $this->assertEquals($originaltext, $finaltext); } + /** + * Test file_rewrite_pluginfile_urls with includetoken. + */ + public function test_file_rewrite_pluginfile_urls_includetoken() { + global $USER, $CFG; + + $CFG->slasharguments = true; + + $this->resetAfterTest(); + + $syscontext = context_system::instance(); + $originaltext = 'Fake test with an image '; + $options = ['includetoken' => true]; + + // Rewrite the content. This will generate a new token. + $finaltext = file_rewrite_pluginfile_urls( + $originaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); + + $token = get_user_key('core_files', $USER->id); + $expectedurl = new \moodle_url("/tokenpluginfile.php/{$token}/{$syscontext->id}/user/private/0/image.png"); + $expectedtext = "Fake test with an image "; + $this->assertEquals($expectedtext, $finaltext); + + // Do it again - the second time will use an existing token. + $finaltext = file_rewrite_pluginfile_urls( + $originaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); + $this->assertEquals($expectedtext, $finaltext); + + // Now undo. + $options['reverse'] = true; + $finaltext = file_rewrite_pluginfile_urls($finaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); + + // Compare the final text is the same that the original. + $this->assertEquals($originaltext, $finaltext); + } + + /** + * Test file_rewrite_pluginfile_urls with includetoken with slasharguments disabled.. + */ + public function test_file_rewrite_pluginfile_urls_includetoken_no_slashargs() { + global $USER, $CFG; + + $CFG->slasharguments = false; + + $this->resetAfterTest(); + + $syscontext = context_system::instance(); + $originaltext = 'Fake test with an image '; + $options = ['includetoken' => true]; + + // Rewrite the content. This will generate a new token. + $finaltext = file_rewrite_pluginfile_urls( + $originaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); + + $token = get_user_key('core_files', $USER->id); + $expectedurl = new \moodle_url("/tokenpluginfile.php"); + $expectedurl .= "?token={$token}&file=/{$syscontext->id}/user/private/0/image.png"; + $expectedtext = "Fake test with an image "; + $this->assertEquals($expectedtext, $finaltext); + + // Do it again - the second time will use an existing token. + $finaltext = file_rewrite_pluginfile_urls( + $originaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); + $this->assertEquals($expectedtext, $finaltext); + + // Now undo. + $options['reverse'] = true; + $finaltext = file_rewrite_pluginfile_urls($finaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); + + // Compare the final text is the same that the original. + $this->assertEquals($originaltext, $finaltext); + } + /** * Helpter function to create draft files * From 8af75f08e8084a8b9197d4a15966f0457851c3d1 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 5 Jun 2018 10:45:06 +0800 Subject: [PATCH 3/6] MDL-36754 tests: Move moodle_url tests to own file --- lib/tests/moodle_url_test.php | 265 ++++++++++++++++++++++++++++++++++ lib/tests/weblib_test.php | 232 ----------------------------- 2 files changed, 265 insertions(+), 232 deletions(-) create mode 100644 lib/tests/moodle_url_test.php diff --git a/lib/tests/moodle_url_test.php b/lib/tests/moodle_url_test.php new file mode 100644 index 00000000000..10a878474be --- /dev/null +++ b/lib/tests/moodle_url_test.php @@ -0,0 +1,265 @@ +. + +/** + * Tests for moodle_url. + * + * @package core + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Tests for moodle_url. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_moodle_url_testcase extends advanced_testcase { + /** + * Test basic moodle_url construction. + */ + public function test_moodle_url_constructor() { + global $CFG; + + $url = new moodle_url('/index.php'); + $this->assertSame($CFG->wwwroot.'/index.php', $url->out()); + + $url = new moodle_url('/index.php', array()); + $this->assertSame($CFG->wwwroot.'/index.php', $url->out()); + + $url = new moodle_url('/index.php', array('id' => 2)); + $this->assertSame($CFG->wwwroot.'/index.php?id=2', $url->out()); + + $url = new moodle_url('/index.php', array('id' => 'two')); + $this->assertSame($CFG->wwwroot.'/index.php?id=two', $url->out()); + + $url = new moodle_url('/index.php', array('id' => 1, 'cid' => '2')); + $this->assertSame($CFG->wwwroot.'/index.php?id=1&cid=2', $url->out()); + $this->assertSame($CFG->wwwroot.'/index.php?id=1&cid=2', $url->out(false)); + + $url = new moodle_url('/index.php', null, 'test'); + $this->assertSame($CFG->wwwroot.'/index.php#test', $url->out()); + + $url = new moodle_url('/index.php', array('id' => 2), 'test'); + $this->assertSame($CFG->wwwroot.'/index.php?id=2#test', $url->out()); + } + + /** + * Tests moodle_url::get_path(). + */ + public function test_moodle_url_get_path() { + $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); + $this->assertSame('/my/file/is/here.txt', $url->get_path()); + + $url = new moodle_url('http://www.example.org/'); + $this->assertSame('/', $url->get_path()); + + $url = new moodle_url('http://www.example.org/pluginfile.php/slash/arguments'); + $this->assertSame('/pluginfile.php/slash/arguments', $url->get_path()); + $this->assertSame('/pluginfile.php', $url->get_path(false)); + } + + public function test_moodle_url_round_trip() { + $strurl = 'http://moodle.org/course/view.php?id=5'; + $url = new moodle_url($strurl); + $this->assertSame($strurl, $url->out(false)); + + $strurl = 'http://moodle.org/user/index.php?contextid=53&sifirst=M&silast=D'; + $url = new moodle_url($strurl); + $this->assertSame($strurl, $url->out(false)); + } + + /** + * Test Moodle URL objects created with a param with empty value. + */ + public function test_moodle_url_empty_param_values() { + $strurl = 'http://moodle.org/course/view.php?id=0'; + $url = new moodle_url($strurl, array('id' => 0)); + $this->assertSame($strurl, $url->out(false)); + + $strurl = 'http://moodle.org/course/view.php?id'; + $url = new moodle_url($strurl, array('id' => false)); + $this->assertSame($strurl, $url->out(false)); + + $strurl = 'http://moodle.org/course/view.php?id'; + $url = new moodle_url($strurl, array('id' => null)); + $this->assertSame($strurl, $url->out(false)); + + $strurl = 'http://moodle.org/course/view.php?id'; + $url = new moodle_url($strurl, array('id' => '')); + $this->assertSame($strurl, $url->out(false)); + + $strurl = 'http://moodle.org/course/view.php?id'; + $url = new moodle_url($strurl); + $this->assertSame($strurl, $url->out(false)); + } + + /** + * Test set good scheme on Moodle URL objects. + */ + public function test_moodle_url_set_good_scheme() { + $url = new moodle_url('http://moodle.org/foo/bar'); + $url->set_scheme('myscheme'); + $this->assertSame('myscheme://moodle.org/foo/bar', $url->out()); + } + + /** + * Test set bad scheme on Moodle URL objects. + * + * @expectedException coding_exception + */ + public function test_moodle_url_set_bad_scheme() { + $url = new moodle_url('http://moodle.org/foo/bar'); + $url->set_scheme('not a valid $ scheme'); + } + + public function test_moodle_url_round_trip_array_params() { + $strurl = 'http://example.com/?a%5B1%5D=1&a%5B2%5D=2'; + $url = new moodle_url($strurl); + $this->assertSame($strurl, $url->out(false)); + + $url = new moodle_url('http://example.com/?a[1]=1&a[2]=2'); + $this->assertSame($strurl, $url->out(false)); + + // For un-keyed array params, we expect 0..n keys to be returned. + $strurl = 'http://example.com/?a%5B0%5D=0&a%5B1%5D=1'; + $url = new moodle_url('http://example.com/?a[]=0&a[]=1'); + $this->assertSame($strurl, $url->out(false)); + } + + public function test_compare_url() { + $url1 = new moodle_url('index.php', array('var1' => 1, 'var2' => 2)); + $url2 = new moodle_url('index2.php', array('var1' => 1, 'var2' => 2, 'var3' => 3)); + + $this->assertFalse($url1->compare($url2, URL_MATCH_BASE)); + $this->assertFalse($url1->compare($url2, URL_MATCH_PARAMS)); + $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); + + $url2 = new moodle_url('index.php', array('var1' => 1, 'var3' => 3)); + + $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); + $this->assertFalse($url1->compare($url2, URL_MATCH_PARAMS)); + $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); + + $url2 = new moodle_url('index.php', array('var1' => 1, 'var2' => 2, 'var3' => 3)); + + $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); + $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); + $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); + + $url2 = new moodle_url('index.php', array('var2' => 2, 'var1' => 1)); + + $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); + $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); + $this->assertTrue($url1->compare($url2, URL_MATCH_EXACT)); + + $url1->set_anchor('test'); + $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); + $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); + $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); + + $url2->set_anchor('test'); + $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); + $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); + $this->assertTrue($url1->compare($url2, URL_MATCH_EXACT)); + } + + public function test_out_as_local_url() { + global $CFG; + // Test http url. + $url1 = new moodle_url('/lib/tests/weblib_test.php'); + $this->assertSame('/lib/tests/weblib_test.php', $url1->out_as_local_url()); + + // Test https url. + $httpswwwroot = str_replace("http://", "https://", $CFG->wwwroot); + $url2 = new moodle_url($httpswwwroot.'/login/profile.php'); + $this->assertSame('/login/profile.php', $url2->out_as_local_url()); + + // Test http url matching wwwroot. + $url3 = new moodle_url($CFG->wwwroot); + $this->assertSame('', $url3->out_as_local_url()); + + // Test http url matching wwwroot ending with slash (/). + $url3 = new moodle_url($CFG->wwwroot.'/'); + $this->assertSame('/', $url3->out_as_local_url()); + } + + /** + * @expectedException coding_exception + * @return void + */ + public function test_out_as_local_url_error() { + $url2 = new moodle_url('http://www.google.com/lib/tests/weblib_test.php'); + $url2->out_as_local_url(); + } + + /** + * You should get error with modified url + * + * @expectedException coding_exception + * @return void + */ + public function test_modified_url_out_as_local_url_error() { + global $CFG; + + $modifiedurl = $CFG->wwwroot.'1'; + $url3 = new moodle_url($modifiedurl.'/login/profile.php'); + $url3->out_as_local_url(); + } + + /** + * Try get local url from external https url and you should get error + * + * @expectedException coding_exception + */ + public function test_https_out_as_local_url_error() { + $url4 = new moodle_url('https://www.google.com/lib/tests/weblib_test.php'); + $url4->out_as_local_url(); + } + + public function test_moodle_url_get_scheme() { + // Should return the scheme only. + $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); + $this->assertSame('http', $url->get_scheme()); + + // Should work for secure URLs. + $url = new moodle_url('https://www.example.org:447/my/file/is/here.txt?really=1'); + $this->assertSame('https', $url->get_scheme()); + + // Should return an empty string if no scheme is specified. + $url = new moodle_url('www.example.org:447/my/file/is/here.txt?really=1'); + $this->assertSame('', $url->get_scheme()); + } + + public function test_moodle_url_get_host() { + // Should return the host part only. + $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); + $this->assertSame('www.example.org', $url->get_host()); + } + + public function test_moodle_url_get_port() { + // Should return the port if one provided. + $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); + $this->assertSame(447, $url->get_port()); + + // Should return an empty string if port not specified. + $url = new moodle_url('http://www.example.org/some/path/here.php'); + $this->assertSame('', $url->get_port()); + } +} diff --git a/lib/tests/weblib_test.php b/lib/tests/weblib_test.php index 2bf88cb25b0..444667280ac 100644 --- a/lib/tests/weblib_test.php +++ b/lib/tests/weblib_test.php @@ -244,238 +244,6 @@ class core_weblib_testcase extends advanced_testcase { $this->assertSame('this is a link [ http://someaddress.com/query ]', wikify_links('this is a link')); } - /** - * Test basic moodle_url construction. - */ - public function test_moodle_url_constructor() { - global $CFG; - - $url = new moodle_url('/index.php'); - $this->assertSame($CFG->wwwroot.'/index.php', $url->out()); - - $url = new moodle_url('/index.php', array()); - $this->assertSame($CFG->wwwroot.'/index.php', $url->out()); - - $url = new moodle_url('/index.php', array('id' => 2)); - $this->assertSame($CFG->wwwroot.'/index.php?id=2', $url->out()); - - $url = new moodle_url('/index.php', array('id' => 'two')); - $this->assertSame($CFG->wwwroot.'/index.php?id=two', $url->out()); - - $url = new moodle_url('/index.php', array('id' => 1, 'cid' => '2')); - $this->assertSame($CFG->wwwroot.'/index.php?id=1&cid=2', $url->out()); - $this->assertSame($CFG->wwwroot.'/index.php?id=1&cid=2', $url->out(false)); - - $url = new moodle_url('/index.php', null, 'test'); - $this->assertSame($CFG->wwwroot.'/index.php#test', $url->out()); - - $url = new moodle_url('/index.php', array('id' => 2), 'test'); - $this->assertSame($CFG->wwwroot.'/index.php?id=2#test', $url->out()); - } - - /** - * Tests moodle_url::get_path(). - */ - public function test_moodle_url_get_path() { - $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); - $this->assertSame('/my/file/is/here.txt', $url->get_path()); - - $url = new moodle_url('http://www.example.org/'); - $this->assertSame('/', $url->get_path()); - - $url = new moodle_url('http://www.example.org/pluginfile.php/slash/arguments'); - $this->assertSame('/pluginfile.php/slash/arguments', $url->get_path()); - $this->assertSame('/pluginfile.php', $url->get_path(false)); - } - - public function test_moodle_url_round_trip() { - $strurl = 'http://moodle.org/course/view.php?id=5'; - $url = new moodle_url($strurl); - $this->assertSame($strurl, $url->out(false)); - - $strurl = 'http://moodle.org/user/index.php?contextid=53&sifirst=M&silast=D'; - $url = new moodle_url($strurl); - $this->assertSame($strurl, $url->out(false)); - } - - /** - * Test Moodle URL objects created with a param with empty value. - */ - public function test_moodle_url_empty_param_values() { - $strurl = 'http://moodle.org/course/view.php?id=0'; - $url = new moodle_url($strurl, array('id' => 0)); - $this->assertSame($strurl, $url->out(false)); - - $strurl = 'http://moodle.org/course/view.php?id'; - $url = new moodle_url($strurl, array('id' => false)); - $this->assertSame($strurl, $url->out(false)); - - $strurl = 'http://moodle.org/course/view.php?id'; - $url = new moodle_url($strurl, array('id' => null)); - $this->assertSame($strurl, $url->out(false)); - - $strurl = 'http://moodle.org/course/view.php?id'; - $url = new moodle_url($strurl, array('id' => '')); - $this->assertSame($strurl, $url->out(false)); - - $strurl = 'http://moodle.org/course/view.php?id'; - $url = new moodle_url($strurl); - $this->assertSame($strurl, $url->out(false)); - } - - /** - * Test set good scheme on Moodle URL objects. - */ - public function test_moodle_url_set_good_scheme() { - $url = new moodle_url('http://moodle.org/foo/bar'); - $url->set_scheme('myscheme'); - $this->assertSame('myscheme://moodle.org/foo/bar', $url->out()); - } - - /** - * Test set bad scheme on Moodle URL objects. - * - * @expectedException coding_exception - */ - public function test_moodle_url_set_bad_scheme() { - $url = new moodle_url('http://moodle.org/foo/bar'); - $url->set_scheme('not a valid $ scheme'); - } - - public function test_moodle_url_round_trip_array_params() { - $strurl = 'http://example.com/?a%5B1%5D=1&a%5B2%5D=2'; - $url = new moodle_url($strurl); - $this->assertSame($strurl, $url->out(false)); - - $url = new moodle_url('http://example.com/?a[1]=1&a[2]=2'); - $this->assertSame($strurl, $url->out(false)); - - // For un-keyed array params, we expect 0..n keys to be returned. - $strurl = 'http://example.com/?a%5B0%5D=0&a%5B1%5D=1'; - $url = new moodle_url('http://example.com/?a[]=0&a[]=1'); - $this->assertSame($strurl, $url->out(false)); - } - - public function test_compare_url() { - $url1 = new moodle_url('index.php', array('var1' => 1, 'var2' => 2)); - $url2 = new moodle_url('index2.php', array('var1' => 1, 'var2' => 2, 'var3' => 3)); - - $this->assertFalse($url1->compare($url2, URL_MATCH_BASE)); - $this->assertFalse($url1->compare($url2, URL_MATCH_PARAMS)); - $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); - - $url2 = new moodle_url('index.php', array('var1' => 1, 'var3' => 3)); - - $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); - $this->assertFalse($url1->compare($url2, URL_MATCH_PARAMS)); - $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); - - $url2 = new moodle_url('index.php', array('var1' => 1, 'var2' => 2, 'var3' => 3)); - - $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); - $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); - $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); - - $url2 = new moodle_url('index.php', array('var2' => 2, 'var1' => 1)); - - $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); - $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); - $this->assertTrue($url1->compare($url2, URL_MATCH_EXACT)); - - $url1->set_anchor('test'); - $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); - $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); - $this->assertFalse($url1->compare($url2, URL_MATCH_EXACT)); - - $url2->set_anchor('test'); - $this->assertTrue($url1->compare($url2, URL_MATCH_BASE)); - $this->assertTrue($url1->compare($url2, URL_MATCH_PARAMS)); - $this->assertTrue($url1->compare($url2, URL_MATCH_EXACT)); - } - - public function test_out_as_local_url() { - global $CFG; - // Test http url. - $url1 = new moodle_url('/lib/tests/weblib_test.php'); - $this->assertSame('/lib/tests/weblib_test.php', $url1->out_as_local_url()); - - // Test https url. - $httpswwwroot = str_replace("http://", "https://", $CFG->wwwroot); - $url2 = new moodle_url($httpswwwroot.'/login/profile.php'); - $this->assertSame('/login/profile.php', $url2->out_as_local_url()); - - // Test http url matching wwwroot. - $url3 = new moodle_url($CFG->wwwroot); - $this->assertSame('', $url3->out_as_local_url()); - - // Test http url matching wwwroot ending with slash (/). - $url3 = new moodle_url($CFG->wwwroot.'/'); - $this->assertSame('/', $url3->out_as_local_url()); - } - - /** - * @expectedException coding_exception - * @return void - */ - public function test_out_as_local_url_error() { - $url2 = new moodle_url('http://www.google.com/lib/tests/weblib_test.php'); - $url2->out_as_local_url(); - } - - /** - * You should get error with modified url - * - * @expectedException coding_exception - * @return void - */ - public function test_modified_url_out_as_local_url_error() { - global $CFG; - - $modifiedurl = $CFG->wwwroot.'1'; - $url3 = new moodle_url($modifiedurl.'/login/profile.php'); - $url3->out_as_local_url(); - } - - /** - * Try get local url from external https url and you should get error - * - * @expectedException coding_exception - */ - public function test_https_out_as_local_url_error() { - $url4 = new moodle_url('https://www.google.com/lib/tests/weblib_test.php'); - $url4->out_as_local_url(); - } - - public function test_moodle_url_get_scheme() { - // Should return the scheme only. - $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); - $this->assertSame('http', $url->get_scheme()); - - // Should work for secure URLs. - $url = new moodle_url('https://www.example.org:447/my/file/is/here.txt?really=1'); - $this->assertSame('https', $url->get_scheme()); - - // Should return an empty string if no scheme is specified. - $url = new moodle_url('www.example.org:447/my/file/is/here.txt?really=1'); - $this->assertSame('', $url->get_scheme()); - } - - public function test_moodle_url_get_host() { - // Should return the host part only. - $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); - $this->assertSame('www.example.org', $url->get_host()); - } - - public function test_moodle_url_get_port() { - // Should return the port if one provided. - $url = new moodle_url('http://www.example.org:447/my/file/is/here.txt?really=1'); - $this->assertSame(447, $url->get_port()); - - // Should return an empty string if port not specified. - $url = new moodle_url('http://www.example.org/some/path/here.php'); - $this->assertSame('', $url->get_port()); - } - public function test_clean_text() { $text = "lala xx"; $this->assertSame($text, clean_text($text, FORMAT_PLAIN)); From 883f65555265f1bf435067c46db70148ea8cf6eb Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 3 Jun 2018 21:15:35 +0800 Subject: [PATCH 4/6] MDL-36754 output: Support token pluginfiles in group pic --- lib/tests/moodle_url_test.php | 82 +++++++++++++++++++++++++++++++++++ lib/upgrade.txt | 6 +++ lib/weblib.php | 53 +++++++++++++++++----- 3 files changed, 130 insertions(+), 11 deletions(-) diff --git a/lib/tests/moodle_url_test.php b/lib/tests/moodle_url_test.php index 10a878474be..c5a9be4d0d7 100644 --- a/lib/tests/moodle_url_test.php +++ b/lib/tests/moodle_url_test.php @@ -262,4 +262,86 @@ class core_moodle_url_testcase extends advanced_testcase { $url = new moodle_url('http://www.example.org/some/path/here.php'); $this->assertSame('', $url->get_port()); } + + /** + * Test the make_pluginfile_url function. + * + * @dataProvider make_pluginfile_url_provider + * @param bool $slashargs + * @param array $args Args to be provided to make_pluginfile_url + * @param string $expected The expected result + */ + public function test_make_pluginfile_url($slashargs, $args, $expected) { + global $CFG; + + $this->resetAfterTest(); + + $CFG->slasharguments = $slashargs; + $url = call_user_func_array('moodle_url::make_pluginfile_url', $args); + $this->assertRegexp($expected, $url->out(true)); + } + + /** + * Data provider for make_pluginfile_url tests. + * + * @return array[] + */ + public function make_pluginfile_url_provider() { + $baseurl = "https://www.example.com/moodle/pluginfile.php"; + $tokenbaseurl = "https://www.example.com/moodle/tokenpluginfile.php"; + return [ + 'Standard with slashargs' => [ + 'slashargs' => true, + 'args' => [ + 1, + 'mod_forum', + 'posts', + 422, + '/my/location/', + 'file.png', + ], + 'expected' => "@{$baseurl}/1/mod_forum/posts/422/my/location/file.png@", + ], + 'Standard without slashargs' => [ + 'slashargs' => false, + 'args' => [ + 1, + 'mod_forum', + 'posts', + 422, + '/my/location/', + 'file.png', + ], + 'expected' => "@{$baseurl}\?file=%2F1%2Fmod_forum%2Fposts%2F422%2Fmy%2Flocation%2Ffile.png@", + ], + 'Token included with slashargs' => [ + 'slashargs' => true, + 'args' => [ + 1, + 'mod_forum', + 'posts', + 422, + '/my/location/', + 'file.png', + false, + true, + ], + 'expected' => "@{$tokenbaseurl}/[^/]*/1/mod_forum/posts/422/my/location/file.png@", + ], + 'Token included without slashargs' => [ + 'slashargs' => false, + 'args' => [ + 1, + 'mod_forum', + 'posts', + 422, + '/my/location/', + 'file.png', + false, + true, + ], + 'expected' => "@{$tokenbaseurl}\?file=%2F1%2Fmod_forum%2Fposts%2F422%2Fmy%2Flocation%2Ffile.png&token=[a-z0-9]*@", + ], + ]; + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index b5bc0f5055d..a8aebdfc56b 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -3,6 +3,12 @@ information provided here is intended especially for developers. === 3.6 === +* A new token-based version of pluginfile.php has been added which can be used for out-of-session file serving by + setting the `$includetoken` parameter to true on the `moodle_url::make_pluginfile_url()`, and + `moodle_url::make_file_url()` functions. +* The following picture functions have been updated to support use of the new token-based file serving: + - print_group_picture + - get_group_picture_url * Custom AJAX handlers for the form autocomplete fields can now optionally return string in their processResults() callback. If a string is returned, it is displayed instead of the list of suggested items. This can be used, for example, to inform the user that there are too many items matching the current search criteria. diff --git a/lib/weblib.php b/lib/weblib.php index b253d3d10df..6b6c00a4be8 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -773,17 +773,41 @@ class moodle_url { * @param string $pathname * @param string $filename * @param bool $forcedownload + * @param boolean $includetoken Whether to use a user token when displaying this group image. + * If the group picture is included in an e-mail or some other location where the audience is a specific + * user who will not be logged in when viewing, then we use a token to authenticate the user. * @return moodle_url */ public static function make_pluginfile_url($contextid, $component, $area, $itemid, $pathname, $filename, - $forcedownload = false) { - global $CFG; - $urlbase = "$CFG->wwwroot/pluginfile.php"; - if ($itemid === null) { - return self::make_file_url($urlbase, "/$contextid/$component/$area".$pathname.$filename, $forcedownload); + $forcedownload = false, $includetoken = false) { + global $CFG, $USER; + + $path = []; + + if ($includetoken) { + $urlbase = "$CFG->wwwroot/tokenpluginfile.php"; + $token = get_user_key('core_files', $USER->id); + if ($CFG->slasharguments) { + $path[] = $token; + } } else { - return self::make_file_url($urlbase, "/$contextid/$component/$area/$itemid".$pathname.$filename, $forcedownload); + $urlbase = "$CFG->wwwroot/pluginfile.php"; } + $path[] = $contextid; + $path[] = $component; + $path[] = $area; + + if ($itemid !== null) { + $path[] = $itemid; + } + + $path = "/" . implode('/', $path) . "{$pathname}{$filename}"; + + $url = self::make_file_url($urlbase, $path, $forcedownload, $includetoken); + if ($includetoken && empty($CFG->slasharguments)) { + $url->param('token', $token); + } + return $url; } /** @@ -2468,15 +2492,18 @@ function print_collapsible_region_end($return = false) { * @param boolean $large Default small picture, or large. * @param boolean $return If false print picture, otherwise return the output as string * @param boolean $link Enclose image in a link to view specified course? + * @param boolean $includetoken Whether to use a user token when displaying this group image. + * If the group picture is included in an e-mail or some other location where the audience is a specific + * user who will not be logged in when viewing, then we use a token to authenticate the user. * @return string|void Depending on the setting of $return */ -function print_group_picture($group, $courseid, $large=false, $return=false, $link=true) { +function print_group_picture($group, $courseid, $large = false, $return = false, $link = true, $includetoken = false) { global $CFG; if (is_array($group)) { $output = ''; foreach ($group as $g) { - $output .= print_group_picture($g, $courseid, $large, true, $link); + $output .= print_group_picture($g, $courseid, $large, true, $link, $includetoken); } if ($return) { return $output; @@ -2486,7 +2513,7 @@ function print_group_picture($group, $courseid, $large=false, $return=false, $li } } - $pictureurl = get_group_picture_url($group, $courseid, $large); + $pictureurl = get_group_picture_url($group, $courseid, $large, $includetoken); // If there is no picture, do nothing. if (!isset($pictureurl)) { @@ -2519,9 +2546,12 @@ function print_group_picture($group, $courseid, $large=false, $return=false, $li * @param stdClass $group A group object. * @param int $courseid The course ID for the group. * @param bool $large A large or small group picture? Default is small. + * @param boolean $includetoken Whether to use a user token when displaying this group image. + * If the group picture is included in an e-mail or some other location where the audience is a specific + * user who will not be logged in when viewing, then we use a token to authenticate the user. * @return moodle_url Returns the url for the group picture. */ -function get_group_picture_url($group, $courseid, $large = false) { +function get_group_picture_url($group, $courseid, $large = false, $includetoken = false) { global $CFG; $context = context_course::instance($courseid); @@ -2542,7 +2572,8 @@ function get_group_picture_url($group, $courseid, $large = false) { $file = 'f2'; } - $grouppictureurl = moodle_url::make_pluginfile_url($context->id, 'group', 'icon', $group->id, '/', $file); + $grouppictureurl = moodle_url::make_pluginfile_url( + $context->id, 'group', 'icon', $group->id, '/', $file, false, $includetoken); $grouppictureurl->param('rev', $group->picture); return $grouppictureurl; } From 88d89512e4f968d16cfaf3999614c4fba7636e7a Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 3 Jun 2018 21:15:35 +0800 Subject: [PATCH 5/6] MDL-36754 output: Support token pluginfiles in userpic --- lib/outputcomponents.php | 8 +++++++- lib/outputrenderers.php | 1 + lib/upgrade.txt | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/outputcomponents.php b/lib/outputcomponents.php index db773a63e25..dc90aea8997 100644 --- a/lib/outputcomponents.php +++ b/lib/outputcomponents.php @@ -206,6 +206,11 @@ class user_picture implements renderable { */ public $includefullname = false; + /** + * @var bool Include user authentication token. + */ + public $includetoken = false; + /** * User picture constructor. * @@ -403,7 +408,8 @@ class user_picture implements renderable { $path .= $page->theme->name.'/'; } // Set the image URL to the URL for the uploaded file and return. - $url = moodle_url::make_pluginfile_url($contextid, 'user', 'icon', NULL, $path, $filename); + $url = moodle_url::make_pluginfile_url( + $contextid, 'user', 'icon', null, $path, $filename, false, $this->includetoken); $url->param('rev', $this->user->picture); return $url; } diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 600e4b3017c..10b589d366d 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -2503,6 +2503,7 @@ class core_renderer extends renderer_base { * - class = image class attribute (default 'userpicture') * - visibletoscreenreaders=true (whether to be visible to screen readers) * - includefullname=false (whether to include the user's full name together with the user picture) + * - includetoken = false * @return string HTML fragment */ public function user_picture(stdClass $user, array $options = null) { diff --git a/lib/upgrade.txt b/lib/upgrade.txt index a8aebdfc56b..e9a5c7e1efd 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -9,6 +9,8 @@ information provided here is intended especially for developers. * The following picture functions have been updated to support use of the new token-based file serving: - print_group_picture - get_group_picture_url +* The `user_picture` class has a new public `$includetoken` property which can be set to make use of the new token-based + file serving. * Custom AJAX handlers for the form autocomplete fields can now optionally return string in their processResults() callback. If a string is returned, it is displayed instead of the list of suggested items. This can be used, for example, to inform the user that there are too many items matching the current search criteria. From 7fa3089753ec9fb70342f6104562d41fc37ef768 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 3 Jun 2018 21:17:24 +0800 Subject: [PATCH 6/6] MDL-36754 mod_forum: Use token for emailed pictures --- mod/forum/classes/output/email/renderer.php | 11 ++++++-- .../emaildigestbasic/renderer_textemail.php | 14 +++++++--- .../emaildigestfull/renderer_textemail.php | 14 +++++++--- mod/forum/classes/output/forum_post.php | 26 +++++++++---------- mod/forum/tests/mail_test.php | 2 +- 5 files changed, 45 insertions(+), 22 deletions(-) diff --git a/mod/forum/classes/output/email/renderer.php b/mod/forum/classes/output/email/renderer.php index f38b76c46a2..fa5f0121cd5 100644 --- a/mod/forum/classes/output/email/renderer.php +++ b/mod/forum/classes/output/email/renderer.php @@ -56,9 +56,16 @@ class renderer extends \mod_forum_renderer { */ public function format_message_text($cm, $post) { $context = \context_module::instance($cm->id); - $message = file_rewrite_pluginfile_urls($post->message, 'pluginfile.php', + $message = file_rewrite_pluginfile_urls( + $post->message, + 'pluginfile.php', $context->id, - 'mod_forum', 'post', $post->id); + 'mod_forum', + 'post', + $post->id, + [ + 'includetoken' => true, + ]); $options = new \stdClass(); $options->para = true; $options->context = $context; diff --git a/mod/forum/classes/output/emaildigestbasic/renderer_textemail.php b/mod/forum/classes/output/emaildigestbasic/renderer_textemail.php index 17560317872..3f4adfcb583 100644 --- a/mod/forum/classes/output/emaildigestbasic/renderer_textemail.php +++ b/mod/forum/classes/output/emaildigestbasic/renderer_textemail.php @@ -53,9 +53,17 @@ class renderer_textemail extends \mod_forum\output\email\renderer_textemail { * @return string */ public function format_message_text($cm, $post) { - $message = file_rewrite_pluginfile_urls($post->message, 'pluginfile.php', - \context_module::instance($cm->id)->id, - 'mod_forum', 'post', $post->id); + $context = \context_module::instance($cm->id); + $message = file_rewrite_pluginfile_urls( + $post->message, + 'pluginfile.php', + $context->id, + 'mod_forum', + 'post', + $post->id, + [ + 'includetoken' => true, + ]); return format_text_email($message, $post->messageformat); } } diff --git a/mod/forum/classes/output/emaildigestfull/renderer_textemail.php b/mod/forum/classes/output/emaildigestfull/renderer_textemail.php index 119c4dcd6d3..cdf8579b844 100644 --- a/mod/forum/classes/output/emaildigestfull/renderer_textemail.php +++ b/mod/forum/classes/output/emaildigestfull/renderer_textemail.php @@ -53,9 +53,17 @@ class renderer_textemail extends \mod_forum\output\email\renderer_textemail { * @return string */ public function format_message_text($cm, $post) { - $message = file_rewrite_pluginfile_urls($post->message, 'pluginfile.php', - \context_module::instance($cm->id)->id, - 'mod_forum', 'post', $post->id); + $context = \context_module::instance($cm->id); + $message = file_rewrite_pluginfile_urls( + $post->message, + 'pluginfile.php', + $context->id, + 'mod_forum', + 'post', + $post->id, + [ + 'includetoken' => true, + ]); return format_text_email($message, $post->messageformat); } } diff --git a/mod/forum/classes/output/forum_post.php b/mod/forum/classes/output/forum_post.php index 14f9026a33d..a31d3eae187 100644 --- a/mod/forum/classes/output/forum_post.php +++ b/mod/forum/classes/output/forum_post.php @@ -135,7 +135,7 @@ class forum_post implements \renderable, \templatable { * * @param \mod_forum_renderer $renderer The render to be used for formatting the message and attachments * @param bool $plaintext Whethe the target is a plaintext target - * @return stdClass Data ready for use in a mustache template + * @return array Data ready for use in a mustache template */ public function export_for_template(\renderer_base $renderer, $plaintext = false) { if ($plaintext) { @@ -149,7 +149,7 @@ class forum_post implements \renderable, \templatable { * Export this data so it can be used as the context for a mustache template. * * @param \mod_forum_renderer $renderer The render to be used for formatting the message and attachments - * @return stdClass Data ready for use in a mustache template + * @return array Data ready for use in a mustache template */ protected function export_for_template_text(\mod_forum_renderer $renderer) { return array( @@ -180,9 +180,9 @@ class forum_post implements \renderable, \templatable { 'discussionlink' => $this->get_discussionlink(), 'authorlink' => $this->get_authorlink(), - 'authorpicture' => $this->get_author_picture(), + 'authorpicture' => $this->get_author_picture($renderer), - 'grouppicture' => $this->get_group_picture(), + 'grouppicture' => $this->get_group_picture($renderer), ); } @@ -190,7 +190,7 @@ class forum_post implements \renderable, \templatable { * Export this data so it can be used as the context for a mustache template. * * @param \mod_forum_renderer $renderer The render to be used for formatting the message and attachments - * @return stdClass Data ready for use in a mustache template + * @return array Data ready for use in a mustache template */ protected function export_for_template_html(\mod_forum_renderer $renderer) { return array( @@ -221,9 +221,9 @@ class forum_post implements \renderable, \templatable { 'discussionlink' => $this->get_discussionlink(), 'authorlink' => $this->get_authorlink(), - 'authorpicture' => $this->get_author_picture(), + 'authorpicture' => $this->get_author_picture($renderer), - 'grouppicture' => $this->get_group_picture(), + 'grouppicture' => $this->get_group_picture($renderer), ); } @@ -543,20 +543,20 @@ class forum_post implements \renderable, \templatable { /** * The HTML for the author's user picture. * + * @param \renderer_base $renderer * @return string */ - public function get_author_picture() { - global $OUTPUT; - - return $OUTPUT->user_picture($this->author, array('courseid' => $this->course->id)); + public function get_author_picture(\renderer_base $renderer) { + return $renderer->user_picture($this->author, array('courseid' => $this->course->id)); } /** * The HTML for a group picture. * + * @param \renderer_base $renderer * @return string */ - public function get_group_picture() { + public function get_group_picture(\renderer_base $renderer) { if (isset($this->userfrom->groups)) { $groups = $this->userfrom->groups[$this->forum->id]; } else { @@ -564,7 +564,7 @@ class forum_post implements \renderable, \templatable { } if ($this->get_is_firstpost()) { - return print_group_picture($groups, $this->course->id, false, true, true); + return print_group_picture($groups, $this->course->id, false, true, true, true); } } } diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php index e15ceaa7f97..e96ba94cc17 100644 --- a/mod/forum/tests/mail_test.php +++ b/mod/forum/tests/mail_test.php @@ -1042,7 +1042,7 @@ class mod_forum_mail_testcase extends advanced_testcase { '
( *\n *)?\n.*HTML text and image', '>Moodle Forum', '

Welcome to Moodle, ' - .'!

', '>Love Moodle', '>1\d1');