384160 - Cannot send attachments with cli and file with accents

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[team]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.3) Gecko/20070208 Mandriva/2.0.0.3-2mdv2007.1 (2007.1) Firefox/2.0.0.3
Build Identifier: 

When you send a file from the desktop (gnome or kde), it triggers a command line which ressembles to :
thunderbird -compose "attachment='file:///home/mloiseleur/Desktop/aéa.txt'"

That's at least what I do. The compose windows then popup, but the mail cannot be sent. It says that the file is not present, but the file is here, and the attachment is viewable !

The problem here seems to be the encoding of the uri.


Reproducible: Always

Steps to Reproduce:
1. 
2.
3.
Actual Results:  
The problem is that the mail cannot be send. There is a message saying the file is not here. 
After further investigation, we have seen that we need an encoded url in order to work. In my example, the problem go away with a call like this : 
thunderbird -compose "attachment='file://$path_with_accents'"
any file with accents is subject to this issue.




Expected Results:  
The mail should be sent normally, since the attachment is viewable in the GUI.
It's really strange to user and developpers who try to user thunderbird in CLI.

I have done a small patch which fix this problem. It reencode properly the uri part. I have tried to make it as convenient as I can think of.

Comment 1

[team]

Attachment: Patch which fix this isse

This patch tries to be convenient with the caller of this feature. It accepts or not the prefix "file://" but will alway encode properly the uri part.

Comment 2

[team]

Adding me to CC.
 
-- 
Michel Loiseleur

Comment 3

[Magnus Melin [:mkmelin]]

I tried (with thunderbird2.0) 
thunderbird -compose "attachment='file:///home/magnus/tmp/aéa.txt'"

No problem for me, maybe it got to do with your environment settings, LC_ALL perhaps? Mine is an utf8 one...

Comment 4

[team]

Did the email sent correctly ? The compose window is fine, it's just the sending which is wrong. 

I was able to reproduce this problem with thunderbird 2.0 on kubuntu and on mandriva. They were both in UTF-8 encoding.

Comment 5

[Magnus Melin [:mkmelin]]

Ah, didn't try to send. Get the "Please verify that your Mail & Newsgroups account settings are correct and try again."

Xref bug 243504, bug 288424.

Comment 6

[team]

Well, if you have an account set correctly, it will say that it cannot find the attachment, despite the fact that this attachment is viewable and correctly displayed in the compose window.

Comment 7

[Magnus Melin [:mkmelin]]

Yeah, sorry, the error message is "Sending of message failed. Unable to open file:///......."  The other one was for some other thing i was testing.

Comment 8

[team]

Comment on attachment 268097 [details] [diff] [review]
Patch which fix this isse

>--- mcontent/messenger/messengercompose/MsgComposeCommands.js	2007-03-05 23:11:44.000000000 +0100
>+++ content/messenger/messengercompose/MsgComposeCommands.js	2007-06-12 18:40:26.000000000 +0200
>@@ -1256,7 +1256,12 @@
>         for (var i = 0; i < attachmentList.length; i ++)
>         {
>           attachment = Components.classes["@mozilla.org/messengercompose/attachment;1"].createInstance(Components.interfaces.nsIMsgAttachment);
>-          attachment.url = attachmentList[i];
>+	  // we need to reencode url, but without the file:// prefix
>+	  var url = attachmentList[i];
>+	  url = url.replace('file://','');
>+	     
>+          attachment.url = "file://" + encodeURIComponent(url);
>           composeFields.addAttachment(attachment);
>         }
>       }

Comment 9

[David :Bienvenu]

I tried applying this patch, by hand, on windows, for both the trunk, and 2.0 branch, and it completely breaks attaching a file to a compose window from the command line - it fails creating the compose window.  So unless I'm missing something, this patch doesn't seem to work on Windows. I'll try to debug it a little - it fails in the composeFields.addAttachment call.

Comment 10

[David :Bienvenu]

Comment on attachment 268097 [details] [diff] [review]
Patch which fix this isse

this doesn't work for windows - I'm not sure how to fix this.  I think one possibility is to utf8 encode the file name

Comment 11

[David :Bienvenu]

Attachment: alternate fix

I'm not sure if this is the right thing to do, but this is my proposal:

If some program is launching Thunderbird with a file: url on the command line, then that program needs to generate a correctly escaped file: url.  Thunderbird can't just blindly escape that url, because that would break programs that correctly escape the url. So I think Beagle needs to do the right escaping. In this case, I think that would look something like this:

file:///tmp/a%C3%A9a.txt

But, if the launching program isn't using a file: url, then we can do the escaping ourselves, e.g., if they're just specifying the attachment like this:

-compose "attachment='c:\tmp\aéa.txt'"

I suppose we could look at the file: url and see if it contains any characters that need escaping, other than '%' of course, because an escaped url would already have '%'. But even in that case, we'd need to figure out how to escape the url, because the previous patch doesn't work on Windows...it's especially tricky because our different escaping capabilities aren't exposed so well to js. In particular, we want to escape the path with the esc_Directory flag.

Comment 12

[Michel Loiseleur]

This patch seems better. I'll give it a try tomorrow, 

Thanks,

Comment 13

[Scott MacGregor]

Comment on attachment 271385 [details] [diff] [review]
alternate fix

I'll bite.

Although this:

if (attachmentStr.substr(0, 7) == "file://")

can probably just be:

if (/^file:///i.test(attachmentStr))

might need to prefix the forward slashes with a back slash actually...

Comment 14

[Michel Loiseleur]

Well, just a note to say it works well for me. I've replaced my patch with the one of Scott MacGregor without any hitch.

  Any chance to see it into 2.x or 3.x ?

Regards,

Comment 15

[Magnus Melin [:mkmelin]]

Actually, that's David's patch. Just waiting to be checked in, with the nit from comment 13.

Comment 16

[Magnus Melin [:mkmelin]]

bienvenu, you seem to have a patch waiting to get checked in here

Comment 19

[Noèl Köthe]

Hello,

any feedback from (super)review of this patch?

thx.

Comment 20

[David :Bienvenu]

I'll check if this has landed.

Comment 21

[David :Bienvenu]

fix checked into TB trunk with nit addressed changeset:   356:35e89cc8d796

SM might be interested in a similar fix...

Comment 25

[[e-mail]]

This is marked as fixed, and I'm using the nightly TB version, but I still can't send files from Open Office if they have special characters. Not sure which application is doing the wrong thing, but if it's TB, I hope that it can be fixed before TB3 final is released.

Comment 26

[Glenn Sommer]

I've sent at bugreport (enhanchment) to Gentoo and Mandriva.

Please see http://bugs.gentoo.org/show_bug.cgi?id=265299
I've backported the fixes from TB3 to TB 2.0.0.11 (Guess it should run on any 2.0.x.x?).
Maybe my backported patch should go upstream instead?

Comment 27

[Michel Loiseleur]

I encountered the same problem. I have backported upstream patch for this issue into Thunderbird and it was not fixed for some use case.

Here's my current fix :

diff -Nur thunderbird-2.0.0.0/build-tree/mozilla/mail/components/compose/content/MsgComposeCommands.js thunderbird-2.0.0.0.new/build-tree/mozilla/mail/components/compose/content/MsgComposeCommands.js
--- mozilla/mail/components/compose/content/MsgComposeCommands.js 4 May 2007 15:43:53 -0000 1.113
+++ mozilla/mail/components/compose/content/MsgComposeCommands.js 7 Jul 2007 22:35:40 -0000
@@ -1255,20 +1255,33 @@ function ComposeStartup(recycled, aParam
       if (args.newsgroups)
         composeFields.newsgroups = args.newsgroups;
       if (args.subject)
         composeFields.subject = args.subject;
       if (args.attachment)
       {
         var attachmentList = args.attachment.split(",");
         var attachment;
+        var localFile = Components.classes["@mozilla.org/file/local;1"].createInstance(Components.interfaces.nsILocalFile);
+        var ioService = Components.classes["@mozilla.org/network/io-service;1"];
+        ioService = ioService.getService(Components.interfaces.nsIIOService);
+        var fileHandler = ioService.getProtocolHandler("file").QueryInterface(Components.interfaces.nsIFileProtocolHandler);
         for (var i = 0; i < attachmentList.length; i ++)
         {
+          var attachmentStr = attachmentList[i];
           attachment = Components.classes["@mozilla.org/messengercompose/attachment;1"].createInstance(Components.interfaces.nsIMsgAttachment);
-          attachment.url = attachmentList[i];
+          if (attachmentStr.substr(0, 7) == "file://")
+          {
+            attachment.url = encodeURI(attachmentStr);
+          }
+          else
+          {
+            localFile.initWithPath(attachmentList[i]);
+            attachment.url = fileHandler.getURLSpecFromFile(localFile);;
+          }
           composeFields.addAttachment(attachment);
         }
       }
       if (args.newshost)
         composeFields.newshost = args.newshost;
       if (args.body)
          composeFields.body = args.body;
     }

Comment 28

[Magnus Melin [:mkmelin]]

Michel: can you please attach a patch (against hg tip) for the case that isn't fixed. Create a new bug for it, and cc me.

Comment 29

[Michel Loiseleur]

Magnus: it's done on Bug 524428.