Issue Details (XML | Word | Printable)

Key: MDL-13792
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: Mathieu Petit-Clair
Reporter: Bill Burgos
Votes: 1
Watchers: 6
Operations

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

File uploads created with Japanese file names in Japanese Windows and IE cannot be downloaded.

Created: 05/Mar/08 08:20 AM   Updated: 28/May/08 01:19 PM
Return to search
Component/s: Unicode
Affects Version/s: 1.8.4
Fix Version/s: 1.8.6, 1.9.1

File Attachments: 1. Text File patch-jp-urlencode.1.txt (2 kB)
2. Text File patch-urlencode-skodak_18.txt (6 kB)
3. Text File urlencoding_skodak.patch (6 kB)

Environment:
User side: Japanese Windows, IE 6
Server LAMP

Database: MySQL
Participants: Bill Burgos, Dongsheng Cai, Larry M Elchuck, Ph.D., Martin Dougiamas, Mathieu Petit-Clair and Petr Skoda
Security Level: None
QA Assignee: Dongsheng Cai
Resolved date: 17/Apr/08
Affected Branches: MOODLE_18_STABLE
Fixed Branches: MOODLE_18_STABLE, MOODLE_19_STABLE


 Description  « Hide
1. Upload a file to, say, an assignment using a Japanese filename or upload to Folder.

2. Retrieve the file by clicking on the link.

3. Moodle reports an error that the file cannot be found.

This only seems to occur with Japanese Windows and IE. Using English Windows does not reproduce this problem. May be a coding issue.

There exists a proposed fix:

http://docs.moodle.org/ja/%E3%82%B5%E3%82%A4%E3%83%88%E3%83%95%E3%82%A1%E3%82%A4%E3%83%AB

Perhaps this can get into code?

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Martin Dougiamas added a comment - 05/Mar/08 09:07 AM
Mat, this needs some checking and thinking to make sure it doesn't affect other browsers / languages etc

Martin Dougiamas made changes - 05/Mar/08 09:07 AM
Field Original Value New Value
Fix Version/s 1.9.1 [ 10240 ]
Assignee Martin Dougiamas [ dougiamas ] Mathieu Petit-Clair [ scyrma ]
Bill Burgos added a comment - 05/Mar/08 09:09 AM
Mat, Martin,

I can test anything on our Japanese windows environment, if needed.


Martin Dougiamas made changes - 05/Mar/08 10:04 AM
Fix Version/s 1.8.5 [ 10252 ]
Mathieu Petit-Clair added a comment - 05/Mar/08 02:56 PM
This is actually a big problem if hosted on Windows with IIS: we can't take for granted that the windows filesystem will correctly store unicode characters. The proposed fix is based on a unicode-aware filesystem and database.

I'm testing this out now, under different systems/versions .. we'll see what comes out of it.


Bill Burgos added a comment - 05/Mar/08 03:23 PM
Just to clarify, the file can be downloaded directly through the Course Folder. However, when presented as a link via Assignment, it fails to download.

Mathieu Petit-Clair added a comment - 05/Mar/08 04:28 PM
What exactly could be different between "Japanese windows" and "English Windows" that would make a problem like this happen? What charset is being used?

When uploading a file with a name consisting only of japanese characters, Moodle displays a filename of `_' (only an underline character), but that's in Linux (Ubuntu). That's not perfect, but it's possible to download the file.


Bill Burgos added a comment - 05/Mar/08 07:06 PM
The underscore is there because Moodle has stripped them. However, you can enable unicode filename uploads by setting:

$CFG->unicodedb = true;
$CFG->unicodecleanfilename = true;

in config.php

This will allow you to upload multibyte unicode filenames.

The problem seems to occur with the formation of the link on IE:

http://192.168.0.1/moodle-18/file.php/2/QA??.xls

as compared with Firefox:

http://192.168.0.1/moodle-18/file.php/2/QA%E8%B3%AA%E5%95%8F.xls

These link formations are the same on the client site. The top link with the proper characters give the error message.

The bottom link does not have the problem on both browsers.

Testing with the same user (Windows JPN and IE 6), the files download O.K. On the client server, we can download directly from the file directory. When clicking on the link from within an Assignment module, we get the Moodle error.

One big difference is that on our local server, we are using MySQL 5.0.4, on the client server 4.1.22. Our local server does not have the problem. The client server does.


Bill Burgos added a comment - 06/Mar/08 08:19 AM
Quick update:

I tried to download an assignment file using MySQL 5.0.4x on Moodle version 1.8. I was able to get the file to start the download, but the filename text was garbled. At least, we are not getting the Moodle error.

Regarding the character set for Japanese Windows, I believe that it is SJIS or shift-JIS.


Bill Burgos added a comment - 07/Mar/08 10:29 AM - edited
Further developments.

This bug seems to now be isolated to to separate areas.

The Missing file problems only seems to occur with the multiple file upload Assignment. For a Single file upload, this is O.K.. The environment of which this happens is the same, Japanese Windows using IE 6.

Could this be that the formation of the link in multiple file upload in Assignment gets mangled with IE?

The other problem with directly downloading from the Course file area seems to be fixed when Windows has Arial unicode font installed.


Bill Burgos added a comment - 07/Mar/08 08:50 PM
More information. The following Apache logs shows the difference between the Japanese and English version of attempts to download the same file.

=== Link to a file ===

== Japanese Windows with IE6 == FAIL

61.213.107.16 - - [07/Mar/2008:20:38:32 +0900] "GET /mod/resource/view.php?id=368 HTTP/1.1" 303 302 "http://18demo.manabu3.com/course/view.php?id=56" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"
61.213.107.16 - - [07/Mar/2008:20:38:32 +0900] "GET /file.php/56/moddata/assignment/82/36/QA\xe7\xae\xa1\xe7\x90\x81Exls HTTP/1.1" 404 4184 "http://18demo.manabu3.com/course/view.php?id=56" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"

== Englsh Windows with IE6 ==

61.213.107.16 - - [07/Mar/2008:20:39:02 +0900] "GET /mod/resource/view.php?id=368 HTTP/1.1" 303 302 "http://18demo.manabu3.com/course/view.php?id=56" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; .NET CLR 2.0.50727)"
61.213.107.16 - - [07/Mar/2008:20:39:02 +0900] "GET /file.php/56/moddata/assignment/82/36/QA\xe7\xae\xa1\xe7\x90\x86.xls HTTP/1.1" 200 13824 "http://18demo.manabu3.com/course/view.php?id=56" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; .NET CLR 2.0.50727)"

=== Advanced file upload Assignment ===

== Japanese Windows with IE6 == FAIL

61.213.107.16 - - [07/Mar/2008:20:44:08 +0900] "GET /file.php?file=/56/moddata/assignment/82/36/QA\xe7\xae\xa1\xe7\x90\x81Exls HTTP/1.1" 404 4111 "http://18demo.manabu3.com/mod/assignment/view.php?id=365" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"

== Englsh Windows with IE6 ==

61.213.107.16 - - [07/Mar/2008:20:45:13 +0900] "GET /file.php?file=/56/moddata/assignment/82/36/QA\xe7\xae\xa1\xe7\x90\x86.xls HTTP/1.1" 200 13824 "http://18demo.manabu3.com/mod/assignment/view.php?id=365" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; .NET CLR 2.0.50727)"

=== Single File upload Assignment ===

== Japanese Windows with IE6 == SUCCESS

61.213.107.16 - - [07/Mar/2008:20:46:06 +0900] "GET /file.php/56/moddata/assignment/81/36/QA%E7%AE%A1%E7%90%86.xls HTTP/1.1" 200 13824 "http://18demo.manabu3.com/mod/assignment/view.php?id=364" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"

== Englsh Windows with IE6 ==

61.213.107.16 - - [07/Mar/2008:20:46:42 +0900] "GET /file.php/56/moddata/assignment/81/36/QA%E7%AE%A1%E7%90%86.xls HTTP/1.1" 200 13824 "http://18demo.manabu3.com/mod/assignment/view.php?id=364" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; .NET CLR 2.0.50727)"


Martin Dougiamas added a comment - 26/Mar/08 04:16 PM
Mat can you prioritise this? Let me know if we're stuck or need more info.

Martin Dougiamas made changes - 26/Mar/08 04:16 PM
Priority Major [ 3 ] Critical [ 2 ]
Mathieu Petit-Clair added a comment - 26/Mar/08 05:28 PM
Hi Bill,

Can you try the patch I just attached to this issue? (file: patch-jp-urlencode.1.txt) It's an improvement, and hopefullly it even fixes this bug.

I can also send the whole file if it's more convenient.


Mathieu Petit-Clair made changes - 26/Mar/08 05:28 PM
Attachment patch-jp-urlencode.1.txt [ 13445 ]
Bill Burgos added a comment - 26/Mar/08 06:46 PM
Hi Mat,

The patch failed. Perhaps you can send me the file. At least it would eliminate one point of error.


Mathieu Petit-Clair added a comment - 27/Mar/08 10:17 AM
I sent both files (1.9 and 1.8 versions) by email.

Bill Burgos added a comment - 27/Mar/08 10:25 AM
Hi Mat,

The patched file for version 1.8 seems to work for Advanced assignment (multiple file upload). I would assume that this can be applied to the other failed links?

Below is the Apache log for the successful transfer.

[27/Mar/2008:10:22:12 +0900] "GET /file.php?file=%2F56%2Fmoddata%2Fassignment%2F82%2F36%2FQA%E7%AE%A1%E7%90%86A.xls HTTP/1.1" 200 13824 "http://18demo.manabu3.com/mod/assignment/submissions.php?id=365" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"

Bill


Mathieu Petit-Clair committed 2 files to 'Moodle CVS' on branch 'MOODLE_18_STABLE' - 27/Mar/08 10:48 AM
MDL-13792 - Use urlencode for filename in upload and uploadsingle assignments to fix problems with multibytes filenames.
MODIFY mod/assignment/type/uploadsingle/assignment.class.php   Rev. 1.30.2.3    (+3 -3 lines)
MODIFY mod/assignment/type/upload/assignment.class.php   Rev. 1.29.2.5    (+4 -4 lines)
Mathieu Petit-Clair added a comment - 27/Mar/08 10:51 AM
I just committed changes to both files in the 1.8 branch. Can you test the new files? I'll commit the changes to 1.9 and head as soon as you confirm it's all good.

Bill Burgos added a comment - 27/Mar/08 11:06 AM
This seems to work for Assignment. However, for linked files, it still produces the error. Here is the Apache log for the link to a file:

[27/Mar/2008:11:00:37 +0900] "GET /file.php/56/\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e\xe3\x83\x86\xe3\x82\xad\xe3\x82\xb9\xe3\x83\x81Etxt HTTP/1.1" 404 4603 "http://18demo.manabu3.com/course/view.php?id=56" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"


Mathieu Petit-Clair added a comment - 27/Mar/08 12:51 PM
Ok, I'll commit the changes for assignments.

There seems to be an awful lot of places where the same changes need to be made. I'll see if there's a better way around this...


Martin Dougiamas added a comment - 31/Mar/08 04:36 PM
I think we need to have a function in 1.9 like (v rough!)

function file_url($path)

if $CFG->slasharguments then

return $CFG->wwroot ..../ url_encode($path)

else

return $CFG->wwroot .?file = / url_encode($path)


Larry M Elchuck, Ph.D. added a comment - 01/Apr/08 02:33 AM
I think this patch broke a functionality in the quick grading of assignments:

We are running moodle 1.8.4 (updated today via CVS).

The following behavior just started in the past few days.

When on the quick grade page for the assignments (eg. http://myserver.com/moodle/mod/assignment/submissions.php?id=4010), if a teacher clicks on some of the attachments under the "last modified (Student)" column, the assignments will not download.

Clicking on the Grade or Update word in the "Status" column will allow the documents to download.

The following error occurs in the former instance:
The requested URL /moodle/file.php/11/moddata/assignment/136/233/student_name_submission.doc was not found on this server.

In the url line in the browser itself, it is converting the "/"s that follow file.php to "%2F".
(eg. http://ccvs.ednet.ns.ca/moodle/file.php%2F11%2Fmoddata%2Fassignment%2F136%2F233%2Fstudent_name_submission.doc)

The assignment type is "upload a single file"

thanks
larry


Mathieu Petit-Clair committed 2 files to 'Moodle CVS' on branch 'MOODLE_18_STABLE' - 01/Apr/08 09:55 AM
Mathieu Petit-Clair added a comment - 01/Apr/08 10:00 AM - edited
I changed the line to prevent urlencoding of the '/', should fix this issue... Can you confirm? I'm starting work on a much bigger patch that will make this change all over the code, in 1.8, 1.9 and head. Before starting on that, I would like to be convinced this is not going to break more functions...

Larry M Elchuck, Ph.D. added a comment - 01/Apr/08 05:20 PM
I ran CVS and the following changes were made:
P mod/assignment/type/upload/assignment.class.php
P mod/assignment/type/uploadsingle/assignment.class.php

It made no difference .... I even restarted the server (which I think was not necessary) and went back into the course after a restart..

Error message:
The requested URL /moodle/file.php/11/moddata/assignment/136/229/echat223.doc was not found on this server.

In the browser:
http://myserver.com/moodle/file.php/11%2Fmoddata%2Fassignment%2F136%2F229/echat223.doc

thank for the promptness in lookin at this
larry


Larry M Elchuck, Ph.D. added a comment - 01/Apr/08 06:09 PM
One clarification requested, Mathieu

Should this fix work with existing assignment uploads or only new ones?
I did not test with new ones until some are submitted today.

If existing records were added to the database with the errant code, what table and field?

thanks
larry


Petr Skoda added a comment - 02/Apr/08 04:04 AM
Is there a reason why it was marked as merged in MOODLE_18_STABLE branch and no code was committed into 19 and HEAD?

Petr Skoda added a comment - 02/Apr/08 04:22 AM
reverting all previous commits - urlencode() is designed to be used on url parameters, not the urls - the "/" must not be converted if it is part of real url with slasharguments

Petr Skoda committed 2 files to 'Moodle CVS' on branch 'MOODLE_18_STABLE' - 02/Apr/08 04:24 AM
MDL-13792 reverting previous commits - the use of urlencode() is not correct
MODIFY mod/assignment/type/upload/assignment.class.php   Rev. 1.29.2.7    (+4 -4 lines)
MODIFY mod/assignment/type/uploadsingle/assignment.class.php   Rev. 1.30.2.5    (+3 -3 lines)
Larry M Elchuck, Ph.D. added a comment - 02/Apr/08 04:37 AM
Took a few minutes for the commit to show up after the notification from the tracker, but it's there now.

All is working as before the errant code.

Thanks Petr

larry


Petr Skoda added a comment - 02/Apr/08 04:54 AM
sending a patch which could work, I can not reproduce the problem in my vista installation

Petr Skoda made changes - 02/Apr/08 04:55 AM
Attachment urlencoding_skodak.patch [ 13508 ]
Bill Burgos added a comment - 02/Apr/08 09:30 AM
I patched the files and this is what I get from the Japanese OS:

== Advanced file upload, check by instructor == -> File downloaded successfully

[02/Apr/2008:10:26:01 +0900] "GET /file.php/56/moddata/assignment/82/36/QA%E7%AE%A1%E7%90%86A.xls HTTP/1.1" 200 13824 "http://18demo.manabu3.com/mod/assignment/submissions.php?id=365" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"

=== Link to file (Resource) == -> 'Sorry, the requested file could not be found'

[02/Apr/2008:10:28:30 +0900] "GET /file.php/56/moddata/assignment/82/36/QA\xe7\xae\xa1\xe7\x90\x81Exls HTTP/1.1" 404 5707 "http://18demo.manabu3.com/course/view.php?id=56" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"


Mathieu Petit-Clair added a comment - 04/Apr/08 05:09 PM
Larry: can you test the patch that Petr commited? We need to fix this bug, but I'd rather have somebody test it before committing it, than having everybody suffer after...

This is the first thing on schedule next week...


Larry M Elchuck, Ph.D. added a comment - 04/Apr/08 05:16 PM
Hi Mathieu

Can you send me or post the files that were changed? I use CVS to update (on a Mac OS X build) and am unfamiliar with using diff files per se. I'll check it out.

thanks
larry


Mathieu Petit-Clair added a comment - 07/Apr/08 12:17 PM
Larry - I sent a zip file with the patched file. I tested it out locally, it's working fine for me. If it's all good for you, I'll commit the backported patch to 1.8, and Petr's patch to 1.9 and head.

Mathieu Petit-Clair made changes - 07/Apr/08 12:18 PM
Attachment patch-urlencode-skodak_18.txt [ 13555 ]
Larry M Elchuck, Ph.D. added a comment - 07/Apr/08 08:33 PM
Hi Mathieu

I replaced the 4 files on one of our boxes. All worked as desired for 2 new added assignments (one single and 1 multiple). This box had not been updated with Petr's replacement of the errant code, so I was able to see a before/after comparison to the code that caused the introduction of ascii code in place of the / symbol in urls.

larry


Mathieu Petit-Clair committed 4 files to 'Moodle CVS' on branch 'MOODLE_18_STABLE' - 08/Apr/08 11:00 AM
MDL-13792 - Commiting Patch by Skodak, fixing problem with japanese characters under some conditions. This has been reported to work, if nobody else
comes foward with bugs related to this, it will need to be changed everywhere else where there is no urlencoding done at this time.
MODIFY mod/assignment/lib.php   Rev. 1.219.2.25    (+3 -7 lines)
MODIFY mod/assignment/type/uploadsingle/assignment.class.php   Rev. 1.30.2.6    (+4 -9 lines)
MODIFY mod/assignment/type/upload/assignment.class.php   Rev. 1.29.2.8    (+5 -8 lines)
MODIFY lib/filelib.php   Rev. 1.39.2.11    (+35 -1 lines)
Mathieu Petit-Clair committed 4 files to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 08/Apr/08 11:02 AM
MDL-13792 - Commiting Patch by Skodak, fixing problem with japanese characters under some conditions. This has been reported to work, if nobody else
comes foward with bugs related to this, it will need to be changed everywhere else where there is no urlencoding done at this time. (1.9 - original
patch)
MODIFY lib/filelib.php   Rev. 1.50.2.12    (+35 -1 lines)
MODIFY mod/assignment/type/uploadsingle/assignment.class.php   Rev. 1.33.2.5    (+3 -8 lines)
MODIFY mod/assignment/type/upload/assignment.class.php   Rev. 1.32.2.13    (+5 -7 lines)
MODIFY mod/assignment/lib.php   Rev. 1.277.2.41    (+2 -7 lines)
Mathieu Petit-Clair committed 4 files to 'Moodle CVS' - 08/Apr/08 11:04 AM
MDL-13792 - Commiting Patch by Skodak, fixing problem with japanese characters under some conditions. This has been reported to work, if nobody else
comes foward with bugs related to this, it will need to be changed everywhere else where there is no urlencoding done at this time. (merge from 1.8
and 1.9)
MODIFY mod/assignment/type/uploadsingle/assignment.class.php   Rev. 1.38    (+3 -8 lines)
MODIFY lib/filelib.php   Rev. 1.63    (+35 -1 lines)
MODIFY mod/assignment/type/upload/assignment.class.php   Rev. 1.46    (+5 -7 lines)
MODIFY mod/assignment/lib.php   Rev. 1.320    (+2 -7 lines)
Mathieu Petit-Clair added a comment - 08/Apr/08 11:09 AM
Thanks for the review!

I committed the patch to 1.8, 1.9 and head. Will wait until after Tuesday-code-review is done, and give everybody some time to report further bug (hopefully, nothing). We will then need to fix it in mod/glossary, mod/exercise and other modules missing proper urlencoding.


Eloy Lafuente (stronk7) made changes - 09/Apr/08 11:27 PM
Fix Version/s 1.8.5 [ 10252 ]
Fix Version/s 1.8.6 [ 10270 ]
Mathieu Petit-Clair added a comment - 14/Apr/08 04:09 PM
Bill, is the problem (for assignments) fixed for you in 1.8?

Bill Burgos added a comment - 15/Apr/08 09:35 AM
Hi Mat,

The Advanced upload of files for Assignment is working O.K. for me. Log below:

[15/Apr/2008:10:32:13 +0900] "GET /file.php/2/moddata/assignment/1/12/QA%E7%AE%A1%E7%90%86A.xls HTTP/1.1" 200 13824 "http://demo08.manabu3.com/mod/assignment/submissions.php?id=4" "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)"

Moodle 1.8.5+ (Build: 20080414)

Bill


Mathieu Petit-Clair added a comment - 17/Apr/08 04:38 PM
I close this bug, the complete fix will be in 1.9 and head branches, but it requires too much modifications to be in 1.8. There is a patch, though, for people who want it, in MDL-14279.

Mathieu Petit-Clair made changes - 17/Apr/08 04:38 PM
Resolution Fixed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
Dongsheng Cai added a comment - 14/May/08 12:56 PM
Thanks

Dongsheng Cai made changes - 14/May/08 12:56 PM
QA Assignee dongsheng
Status Resolved [ 5 ] Closed [ 6 ]
Mathieu Petit-Clair committed 1 file to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 28/May/08 01:15 PM
MDL-13792: fixing problem in the previous patch, where filename contain spaces
MODIFY lib/filelib.php   Rev. 1.50.2.14    (+3 -3 lines)
Mathieu Petit-Clair committed 1 file to 'Moodle CVS' - 28/May/08 01:17 PM
MDL-13792: fixing problem in the previous patch, where filename contain spaces (merge from 1.9)
MODIFY lib/filelib.php   Rev. 1.67    (+3 -3 lines)
Mathieu Petit-Clair added a comment - 28/May/08 01:19 PM
I just committed a small change, we are now using rawurlencode instead of urlencode, so that spaces are correctly encoded as %20 (and not as '+').