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

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

          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