Moodle

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

Details

  • Type: Bug Bug
  • Status: Closed 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

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.

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
Jerome 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 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 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 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 Mouneyrac added a comment - - edited

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

Show
Jerome 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
Jerome Mouneyrac added a comment -

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

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

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: