Closed
Bug 195965
Opened 22 years ago
Closed 22 years ago
custom headers omit \r
Categories
(MailNews Core :: Composition, defect)
MailNews Core
Composition
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.4alpha
People
(Reporter: teilo+bugzilla, Assigned: teilo+bugzilla)
References
()
Details
Attachments
(1 file)
(deleted),
patch
|
teilo+bugzilla
:
review+
sspitzer
:
superreview+
|
Details | Diff | Splinter Review |
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•22 years ago
|
||
Assignee | ||
Comment 2•22 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•22 years ago
|
Attachment #116256 -
Flags: review?(ducarroz)
Assignee | ||
Comment 3•22 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 on attachment 116256 [details] [diff] [review]
Patch to add \r\n to custom headers
r=ssu for the trivial fix.
Assignee | ||
Updated•22 years ago
|
Attachment #116256 -
Flags: superreview?(sspitzer)
Attachment #116256 -
Flags: review?(ducarroz)
Attachment #116256 -
Flags: review+
Assignee | ||
Comment 5•22 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•22 years ago
|
Flags: blocking1.3? → blocking1.3-
Comment 6•22 years ago
|
||
*** Bug 194057 has been marked as a duplicate of this bug. ***
Comment 7•22 years ago
|
||
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
Comment 8•22 years ago
|
||
hmm, maybe not. as that would not allow for multiple "custom header" lines.
I'll test and land the fix.
Comment 9•22 years ago
|
||
fix checked in, re-assign to teilo+bugzilla@teilo.net for credit.
Assignee: sspitzer → teilo+bugzilla
Comment 10•22 years ago
|
||
fixed during 1.4 alpha.
thanks for the patch, James.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.4alpha
Comment 11•22 years ago
|
||
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•22 years ago
|
||
Thanks
/James
Updated•22 years ago
|
Flags: blocking1.4a?
Assignee | ||
Comment 13•21 years ago
|
||
*** Bug 196979 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•