Closed
Bug 1022723
Opened 10 years ago
Closed 10 years ago
Some headers are sent prefixed with "********:" when they were folded due to their length
Categories
(MailNews Core :: MIME, defect)
Tracking
(thunderbird31 unaffected, thunderbird32 fixed, seamonkey2.28 unaffected, seamonkey2.29 affected, seamonkey2.30 affected)
RESOLVED
FIXED
Thunderbird 33.0
Tracking | Status | |
---|---|---|
thunderbird31 | --- | unaffected |
thunderbird32 | --- | fixed |
seamonkey2.28 | --- | unaffected |
seamonkey2.29 | --- | affected |
seamonkey2.30 | --- | affected |
People
(Reporter: infofrommozilla, Assigned: jcranmer)
References
Details
(Keywords: dogfood, regression, Whiteboard: [regression:TB32])
Attachments
(1 file)
4.00 KB,
patch
|
squib
:
review+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:32.0) Gecko/20100101 Firefox/32.0 (Beta/Release) Build ID: 20140605202143 Steps to reproduce: Write a message (news or mail) with a long mail address as From. Actual results: The From header is scrambled (Stars are added). Example: news post in mozilla.test (news.mozilla.org) Message-ID: <kdKdnbcrY548RwjOnZ2dnUVZ_sqdnZ2d@mozilla.org> (three lines) From: *****: Alfred Peters <miteinere-mail-adresseindereinleitungszeilewirddieseoftzulang@geekmail.de>
Reporter | ||
Comment 1•10 years ago
|
||
This is a regression caused by bug 790855 Removing of https://hg.mozilla.org/comm-central/rev/29d10e5f69ab and https://hg.mozilla.org/comm-central/rev/8877b30a38c3 eliminates the error.
Regressions should block the originating bug report rather than depend on it.
Blocks: 790855
Component: Message Compose Window → MIME
No longer depends on: 790855
Keywords: regression
Product: Thunderbird → MailNews Core
Version: Trunk → 32
Updated•10 years ago
|
Whiteboard: [regression:TB32]
Updated•10 years ago
|
Comment 4•10 years ago
|
||
Adjusting summary since bug 1025879, which was made a dupe of this is about the subject (so this does happen to subjects) & adding some more keywords to make this easier to find :-)
Summary: Some headers (From:, To: but not Subject:) are sent broken when they were folded, because of their length. → Some headers are sent prefixed with "********:" when they were folded due to their length
Comment 5•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/8877b30a38c3 adds: 9.86 + encodeMimePartIIStr_UTF8: function (aHeader, aStructured, aCharset, 9.87 + aFieldNameLen, aLineLength) { ... 9.95 + // Compute the encoding options. The way our API is structured in this 9.96 + // method is really horrendous and does not align with the way that JSMime 9.97 + // handles it. Instead, we'll need to create a fake header to take into 9.98 + // account the aFieldNameLen parameter. 9.99 + let fakeHeader = '*'.repeat(aFieldNameLen); ... 9.108 + // Add the text to the be encoded. 9.109 + emitter.addHeaderName(fakeHeader); ... 9.127 + // Compute the output. We need to strip off the fake prefix added earlier 9.128 + // and the extra CRLF at the end. 9.129 + emitter.finish(true); 9.130 + let value = handler.value; 9.131 + value = value.substring(value.indexOf(': ') + 2); 9.132 + return value.substring(0, value.length - 2); 9.133 + }, We're presumably not stripping off this fake header properly.
Flags: needinfo?(Pidgeot18)
Updated•10 years ago
|
Comment 6•10 years ago
|
||
Allowing for my poor programming talents: 9.132 + return value.substring(0, value.length - 2); Shouldn't this just be: return value.substring(value, value.length);
Assignee | ||
Comment 7•10 years ago
|
||
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
Attachment #8442068 -
Flags: review?(irving)
Flags: needinfo?(Pidgeot18)
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #4) > Adjusting summary since bug 1025879, which was made a dupe of this is about > the subject (so this does happen to subjects) I had tested the subject header, but could not reproduce the error. But actually, if the *first* *word* is more than 61 characters long, the error occurs there as well.
Reporter | ||
Comment 9•10 years ago
|
||
(In reply to Joshua Cranmer [:jcranmer] from comment #7) > Created attachment 8442068 [details] [diff] [review] > Fix the issue Yes, it works for me.
Comment 10•10 years ago
|
||
(In reply to Alfred Peters from comment #8) > But actually, if the *first* *word* is more than 61 characters long, the > error occurs there as well. I guess "Donaudampfschifffahrtskaptiänsmützenbandstofffasermaterialeigenschaften neu geregelt" ist an orthodox and unlikely but still valid German email subject... ;-)
Comment 11•10 years ago
|
||
(In reply to Alfred Peters from comment #8) > But actually, if the *first* *word* is more than 61 characters long, the > error occurs there as well. For me, it doesn't need to just be the first word - but either way, glad this is almost fixed :-)
Comment 12•10 years ago
|
||
Please can we either back out bug 790855 or get the fix landed today? Would you mind chasing the review up for this? This has been broken for 19 days now :-(
Flags: needinfo?(Pidgeot18)
Comment 13•10 years ago
|
||
Comment on attachment 8442068 [details] [diff] [review] Fix the issue Review of attachment 8442068 [details] [diff] [review]: ----------------------------------------------------------------- Stealing review. Since people have run with this, and I mostly understand what's going on here (modulo one of the tests), r+. ::: mailnews/compose/test/unit/test_attachment.js @@ +73,5 @@ > RFC2047: 'Content-Type: text/plain; charset=US-ASCII;\r\n'+ > ' name="!\\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~.txt"\r\n', > > RFC2047WithCRLF: 'Content-Type: text/plain; charset=US-ASCII;\r\n'+ > + ' name="!\\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~.txt"\r\n', What made this change? Not that it really matters... ::: mailnews/mime/src/mimeJSComponents.js @@ +311,5 @@ > // Compute the output. We need to strip off the fake prefix added earlier > // and the extra CRLF at the end. > emitter.finish(true); > let value = handler.value; > + value = value.replace(new RegExp(fakeHeader + ":\\s*"), ""); I'd probably have written this as value.replace(/^-*:\s*/, ""), but your way is fine (and arguably more readable).
Attachment #8442068 -
Flags: review?(irving) → review+
Comment 14•10 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #12) > Please can we either back out bug 790855 or get the fix landed today? Trunk is still closed due to bug 1027241 (bustage from m-c change).
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to Jim Porter (:squib) from comment #13) > ::: mailnews/compose/test/unit/test_attachment.js > @@ +73,5 @@ > > RFC2047: 'Content-Type: text/plain; charset=US-ASCII;\r\n'+ > > ' name="!\\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~.txt"\r\n', > > > > RFC2047WithCRLF: 'Content-Type: text/plain; charset=US-ASCII;\r\n'+ > > + ' name="!\\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~.txt"\r\n', > > What made this change? Not that it really matters... I'm not totally sure. To be honest, we really shouldn't be doing anything other than RFC 2231 encoding, which means half of test_attachment.js is a messy test that needs to be culled. (In reply to Ed Morley [:edmorley UTC+0] from comment #12) > Please can we either back out bug 790855 or get the fix landed today? Would > you mind chasing the review up for this? This has been broken for 19 days > now :-( I could land it today, but it wouldn't due any good thanks to a Mr. Brian Smith.
Flags: needinfo?(Pidgeot18)
status-seamonkey2.28:
--- → unaffected
status-seamonkey2.29:
--- → affected
status-seamonkey2.30:
--- → affected
status-thunderbird31:
--- → unaffected
status-thunderbird32:
--- → affected
status-thunderbird33:
--- → affected
Comment 16•10 years ago
|
||
Please can this land, iven that it's a regression and now that the builds are no longer burning? (albeit still not entirely green...!)
Assignee | ||
Comment 17•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/dd53e637e5e4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 33.0
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8442068 [details] [diff] [review] Fix the issue [Approval Request Comment] Regression caused by (bug #): bug 790855 User impact if declined: Annoying *****s in subjects Testing completed (on c-c, etc.): The test code checks an actual malformed header in automated tests. Currently baking on nightlies. Risk to taking this patch (and alternatives if risky): Well, at worst, this patch ends up doing nothing and replaces the *****s with -----s so it doesn't look like we got censored :-)
Attachment #8442068 -
Flags: approval-comm-aurora?
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8442068 [details] [diff] [review] Fix the issue This now needs to be uplifted to beta as well.
Attachment #8442068 -
Flags: approval-comm-beta?
Comment 20•10 years ago
|
||
Comment on attachment 8442068 [details] [diff] [review] Fix the issue a=Standard8
Attachment #8442068 -
Flags: approval-comm-beta?
Attachment #8442068 -
Flags: approval-comm-beta+
Attachment #8442068 -
Flags: approval-comm-aurora?
You need to log in
before you can comment on or make changes to this bug.
Description
•