custom headers omit \r

RESOLVED FIXED in mozilla1.4alpha

Status

--
critical
RESOLVED FIXED
16 years ago
10 years ago

People

(Reporter: teilo+bugzilla, Assigned: teilo+bugzilla)

Tracking

Trunk
mozilla1.4alpha
Bug Flags:
blocking1.3 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
the implementation I did for multiple custom headers introduced a bug as the
headers are terminated with \n and not \r\n

Sendmail & postfix seem to handle this gracefully (even though its wrong) but
qmail aparantly refuse to accept the mail (see URL)
(Assignee)

Comment 1

16 years ago
Created attachment 116256 [details] [diff] [review]
Patch to add \r\n to custom headers
(Assignee)

Comment 2

16 years ago
Need r  s/r Please

Nominating for 1.3 as is such a trivial fix and some ppl are unable to send mail
with custom headers without this
Flags: blocking1.3?
(Assignee)

Updated

16 years ago
Attachment #116256 - Flags: review?(ducarroz)
(Assignee)

Comment 3

16 years ago
mail headers should be separated by CRLF but in creating custom headers they are
separated by only LF

Test case: from news

setup custom headers in prefs.js
  user_pref("mail.compose.other.header", "X-James-Was-Here");
Compose a message and set the custom header
Send the mail though a QMail MTA

Actual results

"An error occured while sending mail. The mail server responded: See
http://pobox.com/~djb/docs/smtplf.html.

. Please check the message and try again."

This is a typical qmail complaint about a bare LF in the message.

(Works ok against postfix/EXIM/Sendmail)

Comment 4

16 years ago
Comment on attachment 116256 [details] [diff] [review]
Patch to add \r\n to custom headers

r=ssu for the trivial fix.
(Assignee)

Updated

16 years ago
Attachment #116256 - Flags: superreview?(sspitzer)
Attachment #116256 - Flags: review?(ducarroz)
Attachment #116256 - Flags: review+
(Assignee)

Comment 5

16 years ago
Risks: (for blocking)

Only tested on WinXP against a EXIM server (no access to anything else)
Worst case could break custom headers (see bug 16925) for mail/news.

Code is not entered unless custom headers are used.
Risks of not patching Mail with custom headers is in violation of RFCs :-(
Flags: blocking1.4a?

Updated

16 years ago
Flags: blocking1.3? → blocking1.3-

Comment 6

16 years ago
*** Bug 194057 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

16 years ago
Blocks: 61520
actually, I don't think \r\n or \r should be in the js here, it should be a fix
to  nsMsgCompUtils.cpp, to call PUSH_NEWLINE().
Assignee: ducarroz → sspitzer
hmm, maybe not.  as that would not allow for multiple "custom header" lines.

I'll test and land the fix.

fix checked in, re-assign to teilo+bugzilla@teilo.net for credit.
Assignee: sspitzer → teilo+bugzilla
fixed during 1.4 alpha.

thanks for the patch, James.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.4alpha
Comment on attachment 116256 [details] [diff] [review]
Patch to add \r\n to custom headers

sr noted.

when I checked in, I also added comments to the .js and C++ for why CRLF is
necessary.

nice work, James.
Attachment #116256 - Flags: superreview?(sspitzer) → superreview+
(Assignee)

Comment 12

16 years ago
Thanks

/James

Updated

16 years ago
Flags: blocking1.4a?
(Assignee)

Comment 13

15 years ago
*** Bug 196979 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.