User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20100101 Firefox/13.0 Build ID: 20120608021316 Steps to reproduce: I was trying to send some attachments to a friend via filelink/yousendit. Actual results: It failed on several of them exactly when they contained non-ASCII letters in the filenames, such as german umlauts: The files were displayed as being uploaded and the links were included in the file, but when one tries to download them, it said that there were no files attached to this link.
Ikezoe-san knows root cause. He says both server and client have a bug.
Created attachment 633972 [details] [diff] [review] Possigble fix for Thunderbird Unfortunately YouSendIt does not support RFC2231 yet, I suppose this patch fixes the issue in Thunderbird if YouSentIt spports RFC2231.
Maxim, any thoughts on this patch or any other possible fix for this?
Maxim, see comment 3
Can someone give an exact file name that causes the issue so the YSI folks can reproduce the bug? thx!
A sample is: ∀.eml I guess any non-ASCII characters cause the issue.
Comment on attachment 641156 [details] [diff] [review] proposed fix Review of attachment 641156 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/components/cloudfile/nsYouSendIt.js @@ +1019,5 @@ > let fileContents = "--" + boundary + > "\r\nContent-Disposition: form-data; name=\"bid\"\r\n\r\n" + > this._urlInfo.fileId; > > + let fileName = /^[\040-\176]+$/.test(this.file.leafName) Why do we do this check, instead of just encoding the leafName with encodeURIComponent every time?
Dennis comments: If file name contains only US-ASCII with space character, then the encoded file name will have %20 in the file name, e.g. (my%20file.txt) which doesn't provide a good user experience.
(In reply to Maxim Baz from comment #10) > Dennis comments: > If file name contains only US-ASCII with space character, then the encoded > file name will have %20 in the file name, e.g. (my%20file.txt) which doesn't > provide a good user experience. Alright, that sounds reasonable. The fix looks OK, but I'm unable to test it currently (the YouSendIt servers seem to be timing out on profile retrieval requests again). Could you please add a test for this in test-cloudfile-backend-yousendit.js?
(In reply to Maxim Baz from comment #10) > Dennis comments: > If file name contains only US-ASCII with space character, then the encoded > file name will have %20 in the file name, e.g. (my%20file.txt) which doesn't > provide a good user experience. I dunno where that my%20file.txt name would be showing but obviously that place should be using decodeURIComponent or it wouldn't display non-ascii files reasonable. (If it's somewhere server-side, the server side is buggy.)
Created attachment 641554 [details] [diff] [review] proposed fix with tests Added test.
Comment on attachment 641554 [details] [diff] [review] proposed fix with tests Code looks good, tests pass, and I've confirmed that this fixes the issue. Thanks for the test case - highly appreciated.
Unfortunately this is too late to take in 14. We'll get it landed for 15 though.
https://hg.mozilla.org/comm-central/rev/42fdd61be0ba https://hg.mozilla.org/releases/comm-aurora/rev/f0adfc883ef2 Thanks for the patch, Maxim! One request, for future patches, please follow the directions at the link below. It makes life easier for those checking in on your behalf. Thanks again! https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in
And backed out due to failures of the new test on OSX. https://hg.mozilla.org/comm-central/rev/85c33a9e9f46 https://hg.mozilla.org/releases/comm-aurora/rev/6d7cc83a5dfc
Can you please also answer comment 12? Are non-ascii chars showing there ok?
(In reply to Magnus Melin from comment #18) > Can you please also answer comment 12? Are non-ascii chars showing there ok? When clicking on the link, it seems that the user is sent to a "download" page, which shows the filename of the file about to be downloaded. Unfortunately, it doesn't appear to URL decode the filename on the server side, so the name appears like: my%20file.txt. Maxim - you might want to file a bug internally at YouSendIt about that.
So the tests for this patch are failing on OSX on our Mozmill testing machines. This is one of those frustrating situations where I cannot seem to reproduce the failure locally, so I've been doing try builds. I've been able to determine that the failure is occurring because the ∀.eml apparently does not exist at runtime for the tests. I'm going to assume that this is a packaging problem, and will consult Mark Banner about this. In the meantime, I'll split the test out and we can land the fix and the test later.
Created attachment 642559 [details] [diff] [review] Fix patch [checked-in to comm-central] This is the fix, extracted.
Comment on attachment 642559 [details] [diff] [review] Fix patch [checked-in to comm-central] Attachment 642559 [details] [diff] checked in to comm-central: https://hg.mozilla.org/comm-central/rev/ff0d44ec5e82
Created attachment 642594 [details] [diff] [review] Mozmill test Whoops, forgot to include the ∀.eml file.
Committed to comm-aurora, assuming Standard8's approval (since the applied patch is a subset of the one that already had approval, and Standard8 is super busy). https://hg.mozilla.org/releases/comm-aurora/rev/4bc10d6ac415
Why is this still open ?
4 years ago
Indeed, it landed on comm-central too - http://hg.mozilla.org/comm-central/rev/ff0d44ec5e82 No need to keep it open to fix the server side bug (comment 19) - if that's still a problem.