685462 - blanks at the end of email-address in Settings prevents displaying of inline images

Status: VERIFIED FIXED

(Thunderbird :: General, defect)

Description

[intendentedelleacque]

User Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.9.2.21) Gecko/20110830 Firefox/3.6.21
Build ID: 20110830092825

Steps to reproduce:

- in Account settings, put a email-address with a blank at the end
- try to use a signature with images or insert an inline images


Actual results:

- the images are not displayed


Expected results:

- the images should be displayed

This bug is caused by a wrong "cid" in the images.

In the body of message the "cid" reports the final blank as "%20" or doesn't report it (I got both behaviour in different PC) while the attachment mantains the final blank in the Content-Id.
In both cases the images will not be displayed.

Probably the best way to fix it, is to strip all spaces in generating the "Content-Id".

Comment 1

[Joe Sabash [:JoeS1]]

Tested with Mozilla/5.0 (Windows NT 5.0; rv:9.0a1) Gecko/20110906 Thunderbird/9.0a1 ID:20110906030027
The worst aspect of this is that it fails silently, appears to send properly, but no viewable image in the message.

Comment 2

[intendentedelleacque]

In my opinion, user shouldn't be allowed to create or modify an account, setting an email address with blanks at the end, but this is a different issue.
Should I open a new bug for that?

Comment 3

[Joe Sabash [:JoeS1]]

Well, preventing the trailing blank would solve the problem here, before it could manifest itself. I could see that happening with an errant copy/paste that picked up a trailing space.
Let's get Mark's opinion.

Comment 4

[intendentedelleacque]

(In reply to Joe Sabash from comment #3)
> Well, preventing the trailing blank would solve the problem here

Just partially, because this will not fix this bug for users that have email-addresses with blanks at the end already and don't realize ti.

Comment 5

[David :Bienvenu]

Yeah, I know where this code is...

Comment 6

[David :Bienvenu]

Attachment: proposed fix

Comment 7

[[e-mail]]

Can we be sure that the email won't contain other "confusing" characters?

Comment 8

[David :Bienvenu]

(In reply to [e-mail] from comment #7)
> Can we be sure that the email won't contain other "confusing" characters?

Other potentially escapable chars are already in e-mail addresses and don't seem to cause a problem, e.g., @ and '.', so I don't know of other chars that would cause a problem.

Comment 9

[[e-mail]]

Comment on attachment 560068 [details] [diff] [review]
proposed fix

I see now, only the domain is used, which shouldn't be a problem.

>+      email.Trim("\b\t\r\n ");
Any reason not to use StripWhitespace*? r=me with that fixed.

*Not including the bug that the external string API appears to have whereby StripWhitespace doesn't strip \b. I guess I ought to file it in Bugzilla.

Comment 10

[Joe Sabash [:JoeS1]]

Please consider this for Aurora and Beta as well.
(although it's not a regression, seems like it could go as a tag-along for other requests I see for Beta approval)

Comment 11

[Joe Sabash [:JoeS1]]

Too late for TB Beta 3
But if there is uncertainty about using StripWhitespace (bug 686775) why not just use the originally proposed Trim and be done with this bug.

Comment 12

[David :Bienvenu]

fixed on trunk - http://hg.mozilla.org/comm-central/rev/fbb0e1b6ad17

Comment 13

[David :Bienvenu]

I used StripWhitespace per Neil - I believe that's the real world problem.

Comment 14

[Joe Sabash [:JoeS1]]

It looks like the blank on the end of the email address has no actual effect on sending. So I don't think an additional bug will be necessary.
Thanks David, for the fix. And thanks to Paolo Kaosmos for filing the bug and providing some great Tb extensions.
https://nic-nac-project.org/~kaosmos/index-en.html
To be honest, if were not for your EditHtml extension, I might have been gone from this arena years ago.
Verified fixed:
Mozilla/5.0 (Windows NT 5.0; rv:9.0a1) Gecko/20110920 Thunderbird/9.0a1 ID:20110920030153

Comment 15

[Mark Banner (:standard8)]

Not tracking going forward. This isn't a frequent scenario as far as we know and has an easy work around.