MDL-33116 Media filter broken with slasharguments off
authorsam marshall <s.marshall@open.ac.uk>
Mon, 21 May 2012 11:55:32 +0000 (12:55 +0100)
committersam marshall <s.marshall@open.ac.uk>
Mon, 21 May 2012 13:24:42 +0000 (14:24 +0100)
lib/medialib.php
lib/tests/medialib_test.php
lib/weblib.php

index 88775c6..57a9e90 100644 (file)
@@ -172,13 +172,20 @@ abstract class core_media {
      * @return string Filename only (not escaped)
      */
     public static function get_filename(moodle_url $url) {
      * @return string Filename only (not escaped)
      */
     public static function get_filename(moodle_url $url) {
-        $path = $url->get_path();
+        global $CFG;
+
+        // Use the 'file' parameter if provided (for links created when
+        // slasharguments was off). If not present, just use URL path.
+        $path = $url->get_param('file');
+        if (!$path) {
+            $path = $url->get_path();
+        }
+
         // Remove everything before last / if present. Does not use textlib as / is UTF8-safe.
         $slash = strrpos($path, '/');
         if ($slash !== false) {
             $path = substr($path, $slash + 1);
         }
         // Remove everything before last / if present. Does not use textlib as / is UTF8-safe.
         $slash = strrpos($path, '/');
         if ($slash !== false) {
             $path = substr($path, $slash + 1);
         }
-
         return $path;
     }
 
         return $path;
     }
 
index a4181d4..dd12e90 100644 (file)
@@ -42,7 +42,7 @@ class medialib_testcase extends advanced_testcase {
         global $CFG;
         parent::setUp();
 
         global $CFG;
         parent::setUp();
 
-        // Reset CFG.
+        // Reset $CFG and $SERVER.
         $this->resetAfterTest(true);
 
         // Consistent initial setup: all players disabled.
         $this->resetAfterTest(true);
 
         // Consistent initial setup: all players disabled.
@@ -81,18 +81,6 @@ class medialib_testcase extends advanced_testcase {
         $_SERVER['HTTP_USER_AGENT'] = 'Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0';
     }
 
         $_SERVER['HTTP_USER_AGENT'] = 'Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0';
     }
 
-    /**
-     * Post-test cleanup. Replaces old $CFG.
-     */
-    public function tearDown() {
-        // Replace original user agent.
-        if (isset($this->realserver)) {
-            $_SERVER = $this->realserver;
-        }
-
-        parent::tearDown();
-    }
-
     /**
      * Test for the core_media_player is_enabled.
      */
     /**
      * Test for the core_media_player is_enabled.
      */
@@ -108,6 +96,32 @@ class medialib_testcase extends advanced_testcase {
         $this->assertTrue($test->is_enabled());
     }
 
         $this->assertTrue($test->is_enabled());
     }
 
+    /**
+     * Test for core_media::get_filename.
+     */
+    public function test_get_filename() {
+        $this->assertEquals('frog.mp4', core_media::get_filename(new moodle_url(
+                '/pluginfile.php/312/mod_page/content/7/frog.mp4')));
+        // This should work even though slasharguments is true, because we want
+        // it to support 'legacy' links if somebody toggles the option later.
+        $this->assertEquals('frog.mp4', core_media::get_filename(new moodle_url(
+                '/pluginfile.php?file=/312/mod_page/content/7/frog.mp4')));
+    }
+
+    /**
+     * Test for core_media::get_extension.
+     */
+    public function test_get_extension() {
+        $this->assertEquals('mp4', core_media::get_extension(new moodle_url(
+                '/pluginfile.php/312/mod_page/content/7/frog.mp4')));
+        $this->assertEquals('', core_media::get_extension(new moodle_url(
+                '/pluginfile.php/312/mod_page/content/7/frog')));
+        $this->assertEquals('mp4', core_media::get_extension(new moodle_url(
+                '/pluginfile.php?file=/312/mod_page/content/7/frog.mp4')));
+        $this->assertEquals('', core_media::get_extension(new moodle_url(
+                '/pluginfile.php?file=/312/mod_page/content/7/frog')));
+    }
+
     /**
      * Test for the core_media_player list_supported_urls.
      */
     /**
      * Test for the core_media_player list_supported_urls.
      */
@@ -344,6 +358,26 @@ class medialib_testcase extends advanced_testcase {
         $this->assertTrue(self::str_contains($t, '</audio>'));
     }
 
         $this->assertTrue(self::str_contains($t, '</audio>'));
     }
 
+    /**
+     * Same as test_embed_url MP3 test, but for slash arguments.
+     */
+    public function test_slash_arguments() {
+        global $CFG, $PAGE;
+
+        // Again we do not turn slasharguments actually on, because it has to
+        // work regardless of the setting of that variable in case of handling
+        // links created using previous setting.
+
+        // Enable MP3 and get renderer.
+        $CFG->core_media_enable_mp3 = true;
+        $renderer = new core_media_renderer_test($PAGE, '');
+
+        // Format: mp3.
+        $url = new moodle_url('http://example.org/pluginfile.php?file=x/y/z/test.mp3');
+        $t = $renderer->embed_url($url);
+        $this->assertTrue(self::str_contains($t, 'core_media_mp3_'));
+    }
+
     /**
      * Test for core_media_renderer embed_url.
      * Checks the EMBED_OR_BLANK option.
     /**
      * Test for core_media_renderer embed_url.
      * Checks the EMBED_OR_BLANK option.
index 7d99ced..f68f3d5 100644 (file)
@@ -763,7 +763,9 @@ class moodle_url {
      * By default the path includes slash-arguments (for example,
      * '/myfile.php/extra/arguments') so it is what you would expect from a
      * URL path. If you don't want this behaviour, you can opt to exclude the
      * By default the path includes slash-arguments (for example,
      * '/myfile.php/extra/arguments') so it is what you would expect from a
      * URL path. If you don't want this behaviour, you can opt to exclude the
-     * slash arguments.
+     * slash arguments. (Be careful: if the $CFG variable slasharguments is
+     * disabled, these URLs will have a different format and you may need to
+     * look at the 'file' parameter too.)
      *
      * @param bool $includeslashargument If true, includes slash arguments
      * @return string Path of URL
      *
      * @param bool $includeslashargument If true, includes slash arguments
      * @return string Path of URL
@@ -771,6 +773,20 @@ class moodle_url {
     public function get_path($includeslashargument = true) {
         return $this->path . ($includeslashargument ? $this->slashargument : '');
     }
     public function get_path($includeslashargument = true) {
         return $this->path . ($includeslashargument ? $this->slashargument : '');
     }
+
+    /**
+     * Returns a given parameter value from the URL.
+     *
+     * @param string $name Name of parameter
+     * @return string Value of parameter or null if not set
+     */
+    public function get_param($name) {
+        if (array_key_exists($name, $this->params)) {
+            return $this->params[$name];
+        } else {
+            return null;
+        }
+    }
 }
 
 /**
 }
 
 /**