393196 - apply_rfc2047_encoding() erroneously inserts whitespace for addr-spec-only addresses

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Kaspar]

Attachment: Proposed patch

When encountering addresses with an addr-spec part only (such as "[e-mail]"), 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:  [e-mail]
CC:  [e-mail],  [e-mail],  [e-mail]

(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)]

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]

Comment on attachment 277718 [details] [diff] [review]
Proposed patch

that looks good, thx, Kaspar.

Comment 3

[Phil Ringnalda (:philor)]

mailnews/mime/src/comi18n.cpp 1.129

Comment 4

[Kaspar]

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]

Comment on attachment 277718 [details] [diff] [review]
Proposed patch

Is it possible to get this into 2.0.0.8?

Comment 6

[Scott MacGregor]

Comment on attachment 277718 [details] [diff] [review]
Proposed patch

I think this has baked enough.

Comment 7

[Scott MacGregor]

I've landed this on the 1.8.1.8 branch.

Comment 8

[Kaspar]

(In reply to comment #7)
> I've landed this on the 1.8.1.8 branch.

Thanks, Scott!

Comment 9

[Martijn Wargers (dead)]

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)]

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]

(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.