Thunderbird creates email with no headers when using a long comma-separated custom "References" header

RESOLVED DUPLICATE of bug 1250376

Status

MailNews Core
MIME
--
minor
RESOLVED DUPLICATE of bug 1250376
2 years ago
2 years ago

People

(Reporter: Patrick, Assigned: Jorg K (GMT+1))

Tracking

Trunk
x86_64
Windows 10

Thunderbird Tracking Flags

(thunderbird47?)

Details

Attachments

(1 obsolete attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160604131506

Steps to reproduce:

I added the "References" header as a custom header. (mail.compose.other.header)
In the compose window, I then set a References header with more than 5 Message-IDs.


Actual results:

All headers in the sent mail were truncated.
Only the following headers were set:

Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


Expected results:

The email should have been sent with all headers. (To, From, Date, References, ...)
(Reporter)

Updated

2 years ago
tracking-thunderbird47: --- → ?
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64

Comment 1

2 years ago
There was bug 726281 - Remove References header before saving as template - recently, but that should only remove the "References:" header when the message is saved as a template, not in other cases.
(Reporter)

Comment 2

2 years ago
(In reply to rsx11m from comment #1)
> There was bug 726281 - Remove References header before saving as template -
> recently, but that should only remove the "References:" header when the
> message is saved as a template, not in other cases.

Strange thing is that it only happens when the "References" header is rather long. (more than 3 Message-IDs or so)
(Assignee)

Comment 3

2 years ago
... and more importantly, bug 726281 is not in TB 47 beta (and never will be) ;-)

In general, mail.compose.other.header was disabled with the advent of jsmime, right? See for example bug 1180209 or bug 1250376 or bug 1260073. But maybe that doesn't apply to References.

> All headers in the sent mail were truncated.
What do you mean by truncated? No headers at all (bug 1197686)? Can you please attach the resulting faulty message. Anything on the debug console?

BTW, does it make sense to hand-craft "References"? Also, if you do, they need to be space separated. Look at bug 1197686.
(Assignee)

Comment 4

2 years ago
(In reply to Jorg K (GMT+2, PTO during summer) from comment #3)
> In general, mail.compose.other.header was disabled with the advent of
> jsmime, right?
That's incorrect. Plain custom headers still work. However, there is a problem with Disposition-Notification-To (bug 1250376).

> Anything on the debug console?
When reproducing bug 1250376, I get this in the Error console:
TypeError: invalid 'in' operand addr - jsmime.js:3074:1

So I guess with your custom References you might get some error coming from JSMime as well. Do try to separate them with spaces.
(Reporter)

Comment 5

2 years ago
> Content-Type: text/plain; charset=utf-8
> Content-Transfer-Encoding: 7bit
> 
> TEST TEST
That's the whole email source.

>  Jorg K, comment #3) Also, if you do, they need to be space separated. 
> (Jorg K, comment #4) Do try to separate them with spaces.
I just re-read the RFCs, I'm using Outlook too much :)
(For reference: I tried to seperate them with commas, Hotmail still does that) Seperated with spaces, the JSMime parsing works.
Thanks! I'll mark the bug as Resolved/Invalid.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INVALID
(Assignee)

Comment 6

2 years ago
Apparently the spaces are optional, see:
https://dxr.mozilla.org/comm-central/source/mailnews/mime/jsmime/jsmime.js#261

We should still fix it so it doesn't misbehave. But you can work in the meantime if you add spaces.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
(Assignee)

Updated

2 years ago
Component: Message Compose Window → MIME
Product: Thunderbird → MailNews Core
Version: 47 Branch → Trunk
(Assignee)

Comment 7

2 years ago
We should make sure that the custom header also gets preprocessed here:
https://dxr.mozilla.org/comm-central/source/mailnews/mime/jsmime/jsmime.js#263

I'm setting the importance to "minor" since this is a rare case with a clear workaround.
Severity: normal → minor
Status: REOPENED → NEW
Summary: Thunderbird truncates headers when using a custom "References" header → Thunderbird creates email with no headers when using a long comma-separated custom "References" header
(Assignee)

Updated

2 years ago
See Also: → bug 1250376
(Assignee)

Comment 8

2 years ago
I've been digging around the header preparation code for bug 1250376 and I've added some debugging here:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompUtils.cpp#489

That code gets run for those custom Reference headers and it's also trying to reduce the length.

Once upon a time I had a patch (attachment 8603752 [details] [diff] [review] for bug 1154521) that changed commas to spaces at that spot. So perhaps we should add that patch after all.

Patrick, for testing purposes, can you please just paste one sample of References here which fails for you.
Flags: needinfo?(Patrick.Schultz1)
(Reporter)

Comment 9

2 years ago
(In reply to Jorg K (GMT+2, PTO during summer) from comment #8)
> Patrick, for testing purposes, can you please just paste one sample of
> References here which fails for you.

Here's a sample: (I've munged the domain names)
> <55B68AFD.5070609@net.invalid>,<leka3ctbtq1ki1l9ux233cye.1434797051726@email.com.invalid>,<55944B9E.7090407@net.invalid>,<5584BF20.7020508@net.invalid>,<201507271948.t6RJmMSI025225@myinvalidsite.invalid>,<23e6159e20364daa9a926554a4169d73@valid.in.invalid>,<80994217-2e94-baed-e763-61bf08733317@net.invalid>,<81750091-60d9-d741-552e-ed4a6281ceb7@net.invalid>
Results in the same headers as in comment #5

Debug console says:
> Zeitstempel: 24.07.2016 15:31:41
> Fehler: Error: Cannot encode <55B68AFD.5070609@net.invalid>,<leka3ctbtq1ki1l9ux233cye.1434797051726@email.com.invalid>,<55944B9E.7090407@net.invalid>,<5584BF20.7020508@net.invalid>,<201507271948.t6RJmMSI025225@myinvalidsite.invalid>,<23e6159e20364daa9a926554a4169d73@valid.in.invalid>,<80994217-2e94-baed-e763-61bf08733317@net.invalid>,<81750091-60d9-d741-552e-ed4a6281ceb7@net.invalid> due to length.
> Quelldatei: resource:///modules/jsmime.jsm -> resource:///modules/jsmime/jsmime.js
> Zeile: 2746
Flags: needinfo?(Patrick.Schultz1)
(Assignee)

Comment 10

2 years ago
Thanks, yes, I can reproduce the error with that. And with spaces instead of commas, it all works.
Your References header, if not broken by spaces, is 358 characters long.
(Assignee)

Comment 11

2 years ago
Patrick, I just want you to understand why I'm interested in this bug: In bug 1154521 and bug 1197686 we fixed References processing so that it would actually split over-long headers at the commas. Somehow your custom References headers don't go through this processing.

Equally we have another bug, bug 1250376, where a custom variation of a standard header, Disposition-Notification-To, doesn't work. So somehow, when people use, or my I say, abuse, standard headers as custom headers, something is going wrong. So perhaps we can kill two birds with one stone.

So while your workaround is clear, separate the references with commas, the guy in the other bug has no option.
(Assignee)

Comment 12

2 years ago
Created attachment 8774133 [details] [diff] [review]
Proposed solution (v1a).

Hi Kent, another JSMime problem, this time our beloved References again.

Turns out then when the user (ab)uses the feature to supply custom headers, the system falls over.

To test this, set preference mail.compose.other.header to References. Compose a new message and enter
<55B68AFD.5070609@net.invalid>,<leka3ctbtq1ki1l9ux233cye.1434797051726@email.com.invalid>,<55944B9E.7090407@net.invalid>,<5584BF20.7020508@net.invalid>,<201507271948.t6RJmMSI025225@myinvalidsite.invalid>,<23e6159e20364daa9a926554a4169d73@valid.in.invalid>,<80994217-2e94-baed-e763-61bf08733317@net.invalid>,<81750091-60d9-d741-552e-ed4a6281ceb7@net.invalid>
into Reference compose field. Sent the message (later).

Result: It doesn't work. With the patch it does. All we do is define an encoder just like we defined a decoder. Sadly there is no good example for an encoder, there is only
structuredEncoders.set("Content-Transfer-Encoding", writeUnstructured);
so I stuck the line from writeUnstructured() into my encoder and it worked.

Of course it's a little questionable whether the user should be messing with the standard headers.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #8774133 - Flags: review?(rkent)

Comment 13

2 years ago
Comment on attachment 8774133 [details] [diff] [review]
Proposed solution (v1a).

Review of attachment 8774133 [details] [diff] [review]:
-----------------------------------------------------------------

r+ = me though the changes were simple enough, and isolated enough, that I just looked it over without testing.
Attachment #8774133 - Flags: review?(rkent) → review+
(Assignee)

Comment 14

2 years ago
Comment on attachment 8774133 [details] [diff] [review]
Proposed solution (v1a).

Hmm, there was a rare sighting of Joshua today, so perhaps he can take a look at this one, too, and then say ... (bug 1250376 comment #19):

  I'm not sure this is the best approach.
  Ideally, the code management for setting the custom header
  should be to parse the header and then re-emit
  it corrected if necessary.
Attachment #8774133 - Flags: review?(Pidgeot18)
(Assignee)

Comment 15

2 years ago
As per bug 1250376 comment #21, this bug is fixed by the fix in that other bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1250376
(Assignee)

Comment 16

2 years ago
Comment on attachment 8774133 [details] [diff] [review]
Proposed solution (v1a).

Patch no longer needed, will be fixed by bug 1250376.
Attachment #8774133 - Attachment is obsolete: true
Attachment #8774133 - Flags: review?(Pidgeot18)
Attachment #8774133 - Flags: review+
You need to log in before you can comment on or make changes to this bug.