Last Comment Bug 393196 - apply_rfc2047_encoding() erroneously inserts whitespace for addr-spec-only addresses
: apply_rfc2047_encoding() erroneously inserts whitespace for addr-spec-only ad...
Status: VERIFIED FIXED
: fixed1.8.1.8
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9alpha8
Assigned To: Kaspar Brand
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-22 07:48 PDT by Kaspar Brand
Modified: 2008-07-31 04:30 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (1.25 KB, patch)
2007-08-22 07:48 PDT, Kaspar Brand
mozilla: review+
mozilla: superreview+
mscott: approval1.8.1.8+
Details | Diff | Review

Description Kaspar Brand 2007-08-22 07:48:12 PDT
Created attachment 277718 [details] [diff] [review]
Proposed patch

When encountering addresses with an addr-spec part only (such as "user@example.com"), libmime's apply_rfc2047_encoding() erroneously inserts a space (0x20) character - i.e. when composing a message with Thunderbird or Seamonkey, the final message will have headers like these:

To:  user1@example.com
CC:  user2@example.net,  user3@example.org,  user4@example.com

(same for From headers, though these usually aren't addr-spec-only)

This can lead to problems with signing mechanisms such as DKIM (RFC 4871), if the header is first signed with the extra white space included, but later on in the SMTP chain removed by another MTA (it will break the original signature). Sendmail is one (prominent) example of such an MTA - it will strip unnecessary whitespace from To header fields automatically (can't even be disabled by means of configuration settings).

To prevent this problem, libmime shouldn't add extra whitespace in the first place. The proposed patch fixes this by checking if the preceding displayname is non-empty (and only adds a space character if that's the case).

Phil, I hope you're (one of) the right person(s) to ask for a review - if not, please let me know whom this should go to otherwise. Thanks!
Comment 1 Phil Ringnalda (:philor) 2007-08-23 08:24:15 PDT
Comment on attachment 277718 [details] [diff] [review]
Proposed patch

I could review it, but it wouldn't do you any good since my r+ doesn't count outside of mail/. If David isn't the right person, he'll certainly know who is.
Comment 2 David :Bienvenu 2007-08-23 08:35:36 PDT
Comment on attachment 277718 [details] [diff] [review]
Proposed patch

that looks good, thx, Kaspar.
Comment 3 Phil Ringnalda (:philor) 2007-08-23 20:16:39 PDT
mailnews/mime/src/comi18n.cpp 1.129
Comment 4 Kaspar Brand 2007-08-24 09:31:30 PDT
Thanks for the fast review and checkin, David and Phil!

Is there any chance of getting this into Tb 2? If so, who is supposed/allowed to set the approval1.8.1.7 flag?

(If these are questions with trivial answers, then I'm sorry for asking and obviously didn't look at the right place [Wiki? Tinderbox?] - thanks for pointing me to the right direction in this case.)
Comment 5 Kaspar Brand 2007-09-29 03:35:44 PDT
Comment on attachment 277718 [details] [diff] [review]
Proposed patch

Is it possible to get this into 2.0.0.8?
Comment 6 Scott MacGregor 2007-09-30 22:11:28 PDT
Comment on attachment 277718 [details] [diff] [review]
Proposed patch

I think this has baked enough.
Comment 7 Scott MacGregor 2007-09-30 22:18:41 PDT
I've landed this on the 1.8.1.8 branch.
Comment 8 Kaspar Brand 2007-09-30 23:08:10 PDT
(In reply to comment #7)
> I've landed this on the 1.8.1.8 branch.

Thanks, Scott!
Comment 9 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-10-04 14:46:33 PDT
I don't know how to verify this bug for the 1.8 branch.
This sounds more like a mail issue than a browser issue, right?
Is there any way to verify that this is fixed on the 1.8 branch?
Comment 10 Phil Ringnalda (:philor) 2007-10-04 20:30:24 PDT
Martijn: So much more a mail issue than a browser issue that I wonder whether you meant to make that comment on some other bug.

To verify it, send an email with two or more cc's from a pre-20070930 branch Tbird, check that you receive it with two spaces between the addresses in the Cc: header in the message source (to be sure you aren't getting them normalized along the way, as I do with two out of three recipient addresses), then send another in a post-20070930 build. Which I just did on the trunk, so, verified fixed there.
Comment 11 Kaspar Brand 2007-10-04 21:45:41 PDT
(In reply to comment #10)
> (to be sure you aren't getting them normalized
> along the way, as I do with two out of three recipient addresses)

Just as a side note - saving the message as a draft will also do the trick here.

Note You need to log in before you can comment on or make changes to this bug.