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

Status: RESOLVED DUPLICATE of bug 1250376

(MailNews Core :: MIME, defect)

Description

[Patrick]

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, ...)

Comment 1

[rsx11m]

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.

Comment 2

[Patrick]

(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)

Comment 3

[Jorg K (CEST = GMT+2)]

... 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.

Comment 4

[Jorg K (CEST = GMT+2)]

(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.

Comment 5

[Patrick]

> 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.

Comment 6

[Jorg K (CEST = GMT+2)]

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.

Comment 7

[Jorg K (CEST = GMT+2)]

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.

Comment 8

[Jorg K (CEST = GMT+2)]

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.

Comment 9

[Patrick]

(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)
> <[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>
Results in the same headers as in comment #5

Debug console says:
> Zeitstempel: 24.07.2016 15:31:41
> Fehler: Error: Cannot encode <[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]> due to length.
> Quelldatei: resource:///modules/jsmime.jsm -> resource:///modules/jsmime/jsmime.js
> Zeile: 2746

Comment 10

[Jorg K (CEST = GMT+2)]

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.

Comment 11

[Jorg K (CEST = GMT+2)]

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.

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 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
<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>,<[e-mail]>
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.

Comment 13

[Kent James (:rkent)]

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.

Comment 14

[Jorg K (CEST = GMT+2)]

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.

Comment 15

[Jorg K (CEST = GMT+2)]

As per bug 1250376 comment #21, this bug is fixed by the fix in that other bug.

Comment 16

[Jorg K (CEST = GMT+2)]

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

Patch no longer needed, will be fixed by bug 1250376.