Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-16272

Link to File with Anchor (blank.html#anchor) Gives "Sorry, the requested file could not be found" Error

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.2
    • Fix Version/s: 1.9.5
    • Component/s: Resource
    • Labels:
      None
    • Environment:
      Linux/Apache/PHP4/MySQL
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE

      Description

      Upload an HTML file to a course. Now, add a resource linking to that uploaded file and include an anchor as part of the link (ex. blah.html#anchor). When you follow the resource link, you should receive the error "Sorry, the requested file could not be found".

      This problem does not occur when linking to external files.

      It looks like the URL to the file is in the form http://server.com/file.php/xx/index.html%23anchor instead of http://server.com/file.php/xx/index.html#anchor where # is being changed to %23.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I guess we are urlencoding too much.

            Jerome can you reproduce and if possible, avoid urlscaping colons, slashes and hashes? : / #

            And see if that fixes the problem. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I guess we are urlencoding too much. Jerome can you reproduce and if possible, avoid urlscaping colons, slashes and hashes? : / # And see if that fixes the problem. Ciao
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Hi,
            first look: I confirmed the bug and that the url gets rawurlencode()
            it's here:

            ------------------------------------------------------------
            mod/resource/type/file/resource.class.php
            ------------------------------------------------------------
            316: $fullurl = get_file_url($course->id.'/'.$resource->reference, $querys);

            ------------------------------------------------------------
            filelib.php
            ------------------------------------------------------------
            5 : function get_file_url(...)

            { 35 : $parts = array_map('rawurlencode', $parts); //we should not encode the anchor hash 51 : }
            Show
            jerome Jérôme Mouneyrac added a comment - Hi, first look: I confirmed the bug and that the url gets rawurlencode() it's here: ------------------------------------------------------------ mod/resource/type/file/resource.class.php ------------------------------------------------------------ 316: $fullurl = get_file_url($course->id.'/'.$resource->reference, $querys); ------------------------------------------------------------ filelib.php ------------------------------------------------------------ 5 : function get_file_url(...) { 35 : $parts = array_map('rawurlencode', $parts); //we should not encode the anchor hash 51 : }
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            Here a solution for managing anchor:
            ------------------------------------------
            Current code
            ------------------------------------------
            if ($CFG->slasharguments)

            { $parts = explode('/', $path); $parts = array_map('rawurlencode', $parts); $path = implode('/', $parts); $ffurl = $url.'/'.$path; $separator = '?'; }

            ------------------------------------------
            Patch
            ------------------------------------------
            if ($CFG->slasharguments) {
            $parts = explode('/', $path);
            foreach ($parts as $key => $part)

            { $subparts = explode('#', $part); $subparts = array_map('rawurlencode', $subparts); $parts[$key] = implode('#', $subparts); }

            $path = implode('/', $parts);
            $ffurl = $url.'/'.$path;
            $separator = '?';
            }

            Looking if we really need to manage colon (it would complicate the code for nothing)

            Show
            jerome Jérôme Mouneyrac added a comment - - edited Here a solution for managing anchor: ------------------------------------------ Current code ------------------------------------------ if ($CFG->slasharguments) { $parts = explode('/', $path); $parts = array_map('rawurlencode', $parts); $path = implode('/', $parts); $ffurl = $url.'/'.$path; $separator = '?'; } ------------------------------------------ Patch ------------------------------------------ if ($CFG->slasharguments) { $parts = explode('/', $path); foreach ($parts as $key => $part) { $subparts = explode('#', $part); $subparts = array_map('rawurlencode', $subparts); $parts[$key] = implode('#', $subparts); } $path = implode('/', $parts); $ffurl = $url.'/'.$path; $separator = '?'; } Looking if we really need to manage colon (it would complicate the code for nothing)
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            This patch needs to be reviewed before commit. (open an new issue for colon if needed)

            Show
            jerome Jérôme Mouneyrac added a comment - - edited This patch needs to be reviewed before commit. (open an new issue for colon if needed)
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Tested here with $path string seems to work perfectly with hashes and slashes being respected. So, I think we can ignore colons for now.

            Note: In HEAD, I've found TWO occurrences of "array_map('rawurlencode"), one in lib/filelib and another in lib/file/file_browser.php. Not sure if both need the hash-hack but ideally it should be present IMO.

            +1 Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Tested here with $path string seems to work perfectly with hashes and slashes being respected. So, I think we can ignore colons for now. Note: In HEAD, I've found TWO occurrences of "array_map('rawurlencode"), one in lib/filelib and another in lib/file/file_browser.php. Not sure if both need the hash-hack but ideally it should be present IMO. +1 Thanks!
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Thanks Jeff for the report.
            Thanks Eloy for the review.
            commited

            Show
            jerome Jérôme Mouneyrac added a comment - Thanks Jeff for the report. Thanks Eloy for the review. commited
            Hide
            timhunt Tim Hunt added a comment -

            Tested and reviewed code. It works. Thanks.

            Show
            timhunt Tim Hunt added a comment - Tested and reviewed code. It works. Thanks.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/May/09