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 Bug
    • Status: Closed
    • Priority: Major 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
    • Rank:
      31167

      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.

      1. MDL-16272.patch
        0.8 kB
        Jérôme Mouneyrac

        Activity

        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Jérôme Mouneyrac added a comment -

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

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

        Tested and reviewed code. It works. Thanks.

        Show
        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: