Closed Bug 195965 Opened 22 years ago Closed 22 years ago

custom headers omit \r

Categories

(MailNews Core :: Composition, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.4alpha

People

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

References

()

Details

Attachments

(1 file)

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)
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?
Attachment #116256 - Flags: review?(ducarroz)
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 on attachment 116256 [details] [diff] [review] Patch to add \r\n to custom headers r=ssu for the trivial fix.
Attachment #116256 - Flags: superreview?(sspitzer)
Attachment #116256 - Flags: review?(ducarroz)
Attachment #116256 - Flags: review+
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?
Flags: blocking1.3? → blocking1.3-
*** Bug 194057 has been marked as a duplicate of this bug. ***
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
Closed: 22 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+
Thanks /James
Flags: blocking1.4a?
*** 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.

Attachment

General

Creator:
Created:
Updated:
Size: