diff --git a/repository/url/lib.php b/repository/url/lib.php index 6b2fc0023e8..3de7982e7a4 100644 --- a/repository/url/lib.php +++ b/repository/url/lib.php @@ -46,7 +46,8 @@ class repository_url extends repository { public function __construct($repositoryid, $context = SYSCONTEXTID, $options = array()){ global $CFG; parent::__construct($repositoryid, $context, $options); - $this->file_url = optional_param('file', '', PARAM_URL); + $this->file_url = optional_param('file', '', PARAM_RAW); + $this->file_url = $this->escape_url($this->file_url); } public function check_login() { @@ -93,13 +94,17 @@ EOD; * @return array */ public function get_listing($path='', $page='') { - global $CFG, $OUTPUT; $ret = array(); $ret['list'] = array(); $ret['nosearch'] = true; $ret['norefresh'] = true; $ret['nologin'] = true; + $this->file_url = clean_param($this->file_url, PARAM_URL); + if (empty($this->file_url)) { + throw new repository_exception('validfiletype', 'repository_url'); + } + $this->parse_file(null, $this->file_url, $ret, true); return $ret; } @@ -217,6 +222,26 @@ EOD; } } + /** + * Escapes a url by replacing spaces with %20. + * + * Note: In general moodle does not automatically escape urls, but for the purposes of making this plugin more user friendly + * and make it consistent with some other areas in moodle (such as mod_url), urls will automatically be escaped. + * + * If moodle_url or PARAM_URL is changed to clean characters that need to be escaped, then this function can be removed + * + * @param string $url An unescaped url. + * @return string The escaped url + */ + protected function escape_url($url) { + $url = str_replace('"', '%22', $url); + $url = str_replace('\'', '%27', $url); + $url = str_replace(' ', '%20', $url); + $url = str_replace('<', '%3C', $url); + $url = str_replace('>', '%3E', $url); + return $url; + } + public function supported_returntypes() { return (FILE_INTERNAL | FILE_EXTERNAL); } diff --git a/repository/url/tests/lib_test.php b/repository/url/tests/lib_test.php new file mode 100644 index 00000000000..2589fb6603f --- /dev/null +++ b/repository/url/tests/lib_test.php @@ -0,0 +1,91 @@ +. + +/** + * Unit tests for the URL repository. + * + * @package repository_url + * @copyright 2014 John Okely + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die; + +global $CFG; +require_once($CFG->dirroot . '/repository/url/lib.php'); + + +/** + * URL repository test case. + * + * @copyright 2014 John Okely + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class repository_url_lib_testcase extends advanced_testcase { + + /** + * Check that the url escaper performs as expected + */ + public function test_escape_url() { + $this->resetAfterTest(); + + $repoid = $this->getDataGenerator()->create_repository('url')->id; + + $conversions = array( + 'http://example.com/test_file.png' => 'http://example.com/test_file.png', + 'http://example.com/test%20file.png' => 'http://example.com/test%20file.png', + 'http://example.com/test file.png' => 'http://example.com/test%20file.png', + 'http://example.com/test file.png?query=string+test&more=string+tests' => + 'http://example.com/test%20file.png?query=string+test&more=string+tests', + 'http://example.com/?tag=

' => 'http://example.com/?tag=%3Cp%3E', + 'http://example.com/"quoted".txt' => 'http://example.com/%22quoted%22.txt', + 'http://example.com/\'quoted\'.txt' => 'http://example.com/%27quoted%27.txt', + '' => '' + ); + + foreach ($conversions as $input => $expected) { + // The constructor uses a optional_param, so we need to hack $_GET. + $_GET['file'] = $input; + $repository = new repository_url($repoid); + $this->assertSame($expected, $repository->file_url); + } + + $exceptions = array( + '%' => true, + '!' => true, + '!https://download.moodle.org/unittest/test.jpg' => true, + 'https://download.moodle.org/unittest/test.jpg' => false + ); + + foreach ($exceptions as $input => $expected) { + $caughtexception = false; + try { + // The constructor uses a optional_param, so we need to hack $_GET. + $_GET['file'] = $input; + $repository = new repository_url($repoid); + $repository->get_listing(); + } catch (repository_exception $e) { + if ($e->errorcode == 'validfiletype') { + $caughtexception = true; + } + } + $this->assertSame($expected, $caughtexception); + } + + unset($_GET['file']); + } + +}