Issue Details (XML | Word | Printable)

Key: MDL-14279
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Mathieu Petit-Clair
Reporter: Mathieu Petit-Clair
Votes: 1
Watchers: 4
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

Use get_file_url instead of manual slasharguments check

Created: 09/Apr/08 04:45 PM   Updated: 11/Jul/08 10:51 AM
Return to search
Component/s: General, RSS
Affects Version/s: 1.8, 1.9
Fix Version/s: 1.9.2

File Attachments: 1. Text File patch-get_file_url-18.txt (30 kB)
2. Text File patch-get_file_url-19.txt (31 kB)
3. Text File patch-get_file_url-19_20080707.txt (43 kB)
Issue Links:
Dependency
 

Participants: Bill Burgos, Eloy Lafuente (stronk7), Martin Dougiamas, Mathieu Petit-Clair, Petr Skoda and Ryan Smith
Security Level: None
Resolved date: 11/Jul/08
Affected Branches: MOODLE_18_STABLE, MOODLE_19_STABLE
Fixed Branches: MOODLE_19_STABLE


 Description  « Hide
To fix MDL-13792, the function get_file_url was added. This bug aims to make use of it in every possible place. I will send in a big patch later (starting with 1.8) for review.

There is a problem with links to rss files, as most of them are to /rss/file.php, not /file.php. See /blog/rsslib.php, line 41, for one case of this. Possibilities :
 1) don't do anything, xml files shouldn't have non-ascii file names anyway
 2) modify get_file_url to make a link to /rss/file.php in these cases
 3) make a different function for rss links

I vote for #2. Petr? Eloy? What do you think?

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Mathieu Petit-Clair made changes - 09/Apr/08 04:49 PM
Field Original Value New Value
Description To fix MDL-13792, the function get_file_url was added. This bug aims to make use of it in every possible place. I will send in a big patch later (starting with 1.8) for review.

There is a problem with links to rss files, as most of them are to /rss/file.php, not /file.php. Possibilities :
 1) don't do anything, xml files shouldn't have non-ascii file names anyway
 2) modify get_file_url to make a link to /rss/file.php in these cases
 3) make a different function for rss links

I vote for #2. Petr? Eloy? What do you think? 		To fix MDL-13792, the function get_file_url was added. This bug aims to make use of it in every possible place. I will send in a big patch later (starting with 1.8) for review.

There is a problem with links to rss files, as most of them are to /rss/file.php, not /file.php. See /blog/rsslib.php, line 41, for one case of this. Possibilities :
 1) don't do anything, xml files shouldn't have non-ascii file names anyway
 2) modify get_file_url to make a link to /rss/file.php in these cases
 3) make a different function for rss links

I vote for #2. Petr? Eloy? What do you think?
[ Permalink | « Hide ]
Mathieu Petit-Clair added a comment - 09/Apr/08 05:35 PM
About rss stuff, I answered my own question. I'll modify the function as it should, it's actually kinda obvious.. (sorry about the noise)

[ Permalink | « Hide ]
Mathieu Petit-Clair added a comment - 09/Apr/08 05:45 PM
Please have a look at the file...

Mathieu Petit-Clair made changes - 09/Apr/08 05:45 PM
Attachment patch-get_file_url-18.txt [ 13581 ]
[ Permalink | « Hide ]
Petr Skoda added a comment - 09/Apr/08 07:15 PM
stuff like this should not go into 1.8 imo

[ Permalink | « Hide ]
Mathieu Petit-Clair added a comment - 10/Apr/08 11:41 AM
That's where the original bug was reported and that's what I was trying to fix (MDL-13792). Why shouldn't that go into 1.8?

[ Permalink | « Hide ]
Petr Skoda added a comment - 10/Apr/08 03:28 PM
  • utf-8 is not officially supported in file names and will not be until 2.0 (or later)
  • we did not backport many more serious bugs into 1.8.x

To be 100% correct we should also bump up the requires version in modules.
Could you please make a patch against 19?


[ Permalink | « Hide ]
Eloy Lafuente (stronk7) added a comment - 14/Apr/08 09:27 AM
I like the idea of those all $CFG->slahsarguments conditions centralised in the new function. And 100% agree wit Petr about 1.9 being the proper target. Not sure if we need requires bumps (AFAIK we bumped before release, isn't it?). Easy to check, anyway.

Ciao


Eloy Lafuente (stronk7) made changes - 14/Apr/08 09:28 AM
Assignee Martin Dougiamas [ dougiamas ] Mathieu Petit-Clair [ scyrma ]
Eloy Lafuente (stronk7) made changes - 14/Apr/08 09:28 AM
Fix Version/s 1.9.1 [ 10240 ]
Mathieu Petit-Clair made changes - 18/Apr/08 01:22 PM
Priority Minor [ 4 ] Major [ 3 ]
Mathieu Petit-Clair made changes - 02/May/08 03:28 PM
Link This issue will help resolve MDL-11342 [ MDL-11342 ]
[ Permalink | « Hide ]
Mathieu Petit-Clair added a comment - 02/May/08 05:29 PM
Here's a version of the patch against MOODLE_19_STABLE. I did this carefully, but I can't possibly test all cases, so .. this needs some more eyes.

Mathieu Petit-Clair made changes - 02/May/08 05:29 PM
Attachment patch-get_file_url-19.txt [ 13843 ]
Martin Dougiamas made changes - 15/May/08 03:02 PM
Fix Version/s 1.9.2 [ 10280 ]
Fix Version/s 1.9.1 [ 10240 ]
[ Permalink | « Hide ]
Bill Burgos added a comment - 22/May/08 03:49 PM
Mat,

We are still having problems with Link to File Resource. Log snippet below:

192.168.0.1 - - [22/May/2008:16:44:19 +0900] "GET /mod/resource/view.php?id=3 HTTP/1.1" 303 232 "http://example.com/course/view.php?id=2" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"
192.168.0.1 - - [22/May/2008:16:44:19 +0900] "GET /file.php/2/QA\xe7\xae\xa1\xe7\x90\x81E.xls HTTP/1.1" 404 6333 "http://example.com/course/view.php?id=2" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"
192.168.0.1 - - [22/May/2008:16:44:20 +0900] "GET /theme/standardwhite/styles.php HTTP/1.1" 200 663 "http://example.com/file.php/2/QA\xe7\xae\xa1\xe7\x90\x81E.xls" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"
192.168.0.1 - - [22/May/2008:16:44:20 +0900] "GET /theme/standard/styles.php HTTP/1.1" 200 121244 "http://example.com/file.php/2/QA\xe7\xae\xa1\xe7\x90\x81E.xls" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"


Petr Skoda made changes - 05/Jul/08 06:39 PM
Fix Version/s 1.9.3 [ 10290 ]
Fix Version/s 1.9.2 [ 10280 ]
[ Permalink | « Hide ]
Mathieu Petit-Clair added a comment - 07/Jul/08 05:27 PM
Here's a new version of the patch, for 1.9. This one corrects (afaics) all calls using $CFG->slasharguments, adding new "types" to get_file_url where necessary.

I did go over it a few times, but it needs more reviews. Petr, Eloy?


Mathieu Petit-Clair made changes - 07/Jul/08 05:27 PM
Attachment patch-get_file_url-19_20080707.txt [ 14457 ]
Mathieu Petit-Clair made changes - 07/Jul/08 05:31 PM
Attachment patch-get_file_url-19_20080707.txt [ 14457 ]
Mathieu Petit-Clair made changes - 07/Jul/08 05:31 PM
Attachment patch-get_file_url-19_20080707.txt [ 14458 ]
Mathieu Petit-Clair made changes - 07/Jul/08 05:32 PM
Attachment patch-get_file_url-19_20080707.txt [ 14458 ]
Mathieu Petit-Clair made changes - 07/Jul/08 05:33 PM
Attachment patch-get_file_url-19_20080707.txt [ 14459 ]
[ Permalink | « Hide ]
Martin Dougiamas added a comment - 10/Jul/08 12:29 PM
I had a read through the patch without spotting anything out of place ... it looks OK to me. I think we should put it in and test it wider today before the release tomorrow.

[ Permalink | « Hide ]
Martin Dougiamas added a comment - 10/Jul/08 12:30 PM
Reminder: update $versions in modules where necessary

Mathieu Petit-Clair committed 33 files to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 10/Jul/08 05:48 PM
MDL-14279: use get_file_url instead of checking $CFG->slashargument manually
MODIFY mod/data/Attic/preset_class.php   Rev. 1.2.2.4    (+3 -2 lines)
MODIFY mod/resource/type/ims/Attic/resource.class.php   Rev. 1.47.2.5    (+3 -6 lines)
MODIFY lib/weblib.php   Rev. 1.970.2.98    (+10 -19 lines)
MODIFY file.php   Rev. 1.46.2.4    (+5 -1 lines)
MODIFY mod/glossary/lib.php   Rev. 1.193.2.14    (+2 -6 lines)
MODIFY mod/data/field/picture/field.class.php   Rev. 1.24.2.2    (+8 -14 lines)
MODIFY question/format/qti2/Attic/qt_common.php   Rev. 1.4.2.1    (+2 -3 lines)
MODIFY mod/exercise/Attic/lib.php   Rev. 1.48.2.3    (+3 -6 lines)
MODIFY lib/questionlib.php   Rev. 1.119.2.13    (+6 -12 lines)
MODIFY mod/data/field/file/field.class.php   Rev. 1.17.2.5    (+3 -7 lines)
MODIFY blog/rsslib.php   Rev. 1.12.2.1    (+2 -5 lines)
MODIFY mod/scorm/loadSCO.php   Rev. 1.33.4.1    (+3 -6 lines)
MODIFY lib/wiki_to_markdown.php   Rev. 1.11.18.1    (+7 -15 lines)
MODIFY question/format/qti2/Attic/format.php   Rev. 1.12.4.2    (+5 -9 lines)
MODIFY mod/forum/lib.php   Rev. 1.609.2.63    (+2 -6 lines)
MODIFY mod/resource/type/file/Attic/resource.class.php   Rev. 1.71.2.12    (+4 -16 lines)
MODIFY question/format/hotpot/format.php   Rev. 1.12.4.5    (+3 -6 lines)
MODIFY mod/workshop/Attic/locallib.php   Rev. 1.43.2.2    (+4 -7 lines)
MODIFY mod/lesson/mediafile.php   Rev. 1.13.2.1    (+4 -10 lines)
MODIFY backup/restorelib.php   Rev. 1.283.2.48    (+3 -12 lines)
MODIFY mod/forum/rsslib.php   Rev. 1.25.2.1    (+4 -6 lines)
MODIFY userpix/index.php   Rev. 1.9.2.3    (+5 -8 lines)
MODIFY userpix/upgrade.php   Rev. 1.5.2.2    (+5 -8 lines)
MODIFY mod/workshop/Attic/submissions.php   Rev. 1.60.2.3    (+4 -7 lines)
MODIFY lib/filelib.php   Rev. 1.50.2.18    (+21 -6 lines)
MODIFY blog/lib.php   Rev. 1.80.2.16    (+2 -6 lines)
MODIFY lib/editor/htmlarea/Attic/coursefiles.php   Rev. 1.13.8.2    (+3 -7 lines)
MODIFY question/export.php   Rev. 1.43.2.3    (+2 -7 lines)
MODIFY mod/lesson/importppt.php   Rev. 1.22.2.2    (+4 -7 lines)
MODIFY mod/hotpot/lib.php   Rev. 1.79.2.13    (+3 -6 lines)
MODIFY lib/rsslib.php   Rev. 1.52.2.3    (+3 -7 lines)
MODIFY files/index.php   Rev. 1.121.2.6    (+3 -6 lines)
MODIFY theme/standardlogo/header.html   Rev. 1.26.2.2    (+4 -10 lines)
Mathieu Petit-Clair committed 30 files to 'Moodle CVS' - 10/Jul/08 05:55 PM
MDL-14279: use get_file_url instead of checking $CFG->slashargument manually (merge from 1.9)
MODIFY question/format/qti2/Attic/qt_common.php   Rev. 1.5    (+2 -3 lines)
MODIFY question/export.php   Rev. 1.49    (+2 -7 lines)
MODIFY mod/scorm/loadSCO.php   Rev. 1.37    (+3 -6 lines)
MODIFY lib/editor/htmlarea/Attic/coursefiles.php   Rev. 1.19    (+3 -7 lines)
MODIFY lib/wiki_to_markdown.php   Rev. 1.13    (+7 -15 lines)
MODIFY mod/lesson/importppt.php   Rev. 1.28    (+4 -7 lines)
MODIFY files/index.php   Rev. 1.132    (+3 -6 lines)
MODIFY userpix/upgrade.php   Rev. 1.10    (+5 -8 lines)
MODIFY lib/rsslib.php   Rev. 1.56    (+3 -7 lines)
MODIFY mod/forum/rsslib.php   Rev. 1.27    (+4 -6 lines)
MODIFY mod/resource/type/ims/Attic/resource.class.php   Rev. 1.52    (+3 -6 lines)
MODIFY backup/restorelib.php   Rev. 1.343    (+3 -12 lines)
MODIFY lib/filelib.php   Rev. 1.72    (+21 -6 lines)
MODIFY lib/questionlib.php   Rev. 1.146    (+6 -12 lines)
MODIFY mod/lesson/mediafile.php   Rev. 1.17    (+3 -9 lines)
MODIFY mod/hotpot/lib.php   Rev. 1.102    (+3 -6 lines)
MODIFY question/format/qti2/Attic/format.php   Rev. 1.17    (+5 -9 lines)
MODIFY blog/lib.php   Rev. 1.99    (+2 -6 lines)
MODIFY userpix/index.php   Rev. 1.15    (+5 -8 lines)
MODIFY blog/rsslib.php   Rev. 1.15    (+2 -5 lines)
MODIFY mod/resource/type/file/Attic/resource.class.php   Rev. 1.83    (+4 -16 lines)
MODIFY mod/forum/lib.php   Rev. 1.685    (+2 -6 lines)
MODIFY question/format/hotpot/format.php   Rev. 1.18    (+3 -6 lines)
MODIFY mod/data/Attic/preset_class.php   Rev. 1.8    (+3 -2 lines)
MODIFY mod/data/field/file/field.class.php   Rev. 1.26    (+3 -7 lines)
MODIFY file.php   Rev. 1.53    (+5 -1 lines)
MODIFY theme/standardlogo/header.html   Rev. 1.28    (+4 -10 lines)
MODIFY mod/data/field/picture/field.class.php   Rev. 1.28    (+8 -14 lines)
MODIFY mod/glossary/lib.php   Rev. 1.212    (+2 -6 lines)
MODIFY lib/weblib.php   Rev. 1.1101    (+10 -19 lines)
[ Permalink | « Hide ]
Ryan Smith added a comment - 10/Jul/08 11:01 PM
I'm not sure if this bug caused this, but I updated this morning and got the new htmlarea.php and coursefiles.php and I now see no directory structure in the insert image section of the editor.

[Thu Jul 10 09:24:27 2008] [error] [client 137.x.x.x] PHP Parse error: syntax error, unexpected T_VARIABLE in C:\\Apache2\\htdocs\\moodle\\lib\\editor\\htmlarea
coursefiles.php on line 758, referer: moodle/lib/editor/htmlarea/popups/insert_image.php?id=1
[Thu Jul 10 09:24:46 2008] [error] [client 137.x.x.x] PHP Parse error: syntax error, unexpected T_VARIABLE in C:\\Apache2\\htdocs\\moodle\\lib\\editor\\htmlarea
coursefiles.php on line 758, referer: moodle/lib/editor/htmlarea/popups/insert_image.php?id=1
[Thu Jul 10 09:24:50 2008] [error] [client 137.x.x.x] PHP Parse error: syntax error, unexpected T_VARIABLE in C:\\Apache2\\htdocs\\moodle\\lib\\editor\\htmlarea
coursefiles.php on line 758, referer: moodle/lib/editor/htmlarea/popups/insert_image.php?id=1

I apologize if this isn't the correct bug to report this. Thanks.


[ Permalink | « Hide ]
Ryan Smith added a comment - 11/Jul/08 02:21 AM
Something is wrong in moodle/files/index.php now as well, when I try to download a file from the course files area I now get:

You don't have permission to access /moodlehttp://moodle/file.php/286/backupdata/backup-ejhs-cb1-20080519-2322.zip on this server.


Mathieu Petit-Clair committed 3 files to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 11/Jul/08 10:28 AM
MDL-14279: Fixes for regressions created by the get_file_url massive change
MODIFY lib/editor/htmlarea/Attic/coursefiles.php   Rev. 1.13.8.3    (+2 -2 lines)
MODIFY files/index.php   Rev. 1.121.2.7    (+2 -2 lines)
MODIFY lib/filelib.php   Rev. 1.50.2.19    (+2 -1 lines)
Mathieu Petit-Clair committed 3 files to 'Moodle CVS' - 11/Jul/08 10:31 AM
MDL-14279: Fixes for regressions created by the get_file_url massive change (merge from 1.9)
MODIFY lib/editor/htmlarea/Attic/coursefiles.php   Rev. 1.20    (+2 -2 lines)
MODIFY files/index.php   Rev. 1.133    (+2 -2 lines)
MODIFY lib/filelib.php   Rev. 1.73    (+2 -1 lines)
[ Permalink | « Hide ]
Mathieu Petit-Clair added a comment - 11/Jul/08 10:32 AM
Thanks for the report, Ryan. I've just sent in fixes for these.

Mathieu Petit-Clair made changes - 11/Jul/08 10:46 AM
Fix Version/s 1.9.2 [ 10280 ]
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Fix Version/s 1.9.3 [ 10290 ]
[ Permalink | « Hide ]
Ryan Smith added a comment - 11/Jul/08 10:51 AM
Mathieu,

I just tested the updated files and they seem to work perfectly. Thanks for fixing it!


gbateson committed 1 file to 'Moodle CVS' - 13/Jul/08 06:59 AM
MDL-14279 fixed typo in get_baseurl: get_file_url($this->filedir.'/') should be get_file_url($this->filedir).'/'
MODIFY mod/hotpot/lib.php   Rev. 1.103    (+2 -2 lines)
tjhunt committed 1 file to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 23/Jul/08 06:32 PM
MDL-15792 - Images in question don't display. This is a regression from MDL-14279. Fix thanks to Yolanda Ord��ez Rufat.
MODIFY lib/questionlib.php   Rev. 1.119.2.15    (+2 -2 lines)
tjhunt committed 1 file to 'Moodle CVS' - 23/Jul/08 06:32 PM
MDL-15792 - Images in question don't display. This is a regression from MDL-14279. Fix thanks to Yolanda Ord��ez Rufat.
MODIFY lib/questionlib.php   Rev. 1.151    (+2 -2 lines)

Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.
Atlassian JIRA the Professional Issue Tracker. (Enterprise Edition, Version: 3.13.5-#360) - Bug/feature request - Atlassian news - Contact Administrators