745919 - TB 11.0.forwarding plain-text (non-MIME) emails creates empty "Attached Message Part" even when forward messages is set Inline, if no Content-Type header, no parameter in Content-Type:.

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Jim Lawson]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120310194926

Steps to reproduce:

I received a plain-text (non-MIME) message and forwarded it, while using "forward inline" mode.


Actual results:

Thunderbird forwarded the message inline as I requested, but also added a zero-length attachment of Content-Type: text/plain name="Attached Message Part"


Expected results:

The original message should be forwarded inline, with no empty attachment.  The empty attachment is confusing to the recipient.

Comment 1

[Jim Lawson]

This was seen on Thunderbird 11.0.1 on Win7 and OSX 10.7.3.

Comment 2

[Jim Lawson]

This problem also exists in 12.0.b4

Comment 3

[Hashem Masoud]

(In reply to Jim Lawson from comment #2)
Works for me:
User Agent: Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20120327 Thunderbird/11.0.1
Application Build ID: 20120327115527

Try to start Thunderbird without add-ons and check.

Comment 4

[Jim Lawson]

Thanks for trying to recreate.  Starting without add-ons does not make the problem go away.

IMAP server is Dovecot 2.1.4.  Although copying the original plain text message to a local folder does not change the behavior.

I will try a Linux install of 11.0.1 when I get to that setup...

Comment 5

[Jim Lawson]

Was able to recreate problem in 11.0.1 on Linux (Ubuntu 10.04.4 LTS) on x86_64

Name=Thunderbird
Version=11.0.1
BuildID=20120327115527

Try an RFC822 complaint message, one WITHOUT MIME-Version:, Content-Type:, and Content-Transfer-Encoding: headers.  The problem is not triggered if these headers are present.

Comment 6

[WADA:World Anti-bad-Duping Agency]

Problem is observed in next cases(With MIME-Version: 1.0 existent).
(1) No Content-Type: header
(2) Content-Type: (no parameter)
(3) Content-Type: unknown/unknown
(4) Content-Type: ???### (syntactically invalid)
Problem doesn't occur if major type is text.
(5) Content-Type: text/??? (syntactically invalid)
(6) Content-Type: text/unknown
Perhaps a regression by "quirks for malformed mail of non-displayable content-type like audio/wav, application/pdf". By it, such malformed mail is internally treated as if multipart/mixed in order to show the non-displayable message body of RFC822 mail as if attachment.

http://tools.ietf.org/html/rfc2045#section-5.2 says default is following.
  Content-type: text/plain; charset=us-ascii

What mailer or mail system did send mail without Content-Type: header?

Comment 7

[Jim Lawson]

Sendmail which receives a "bare" message without headers will not add Content-Type headers, as it is not required for compliance with RFC2822 (or, it appears, RFC2042 if text/plain is the default.)

Comment 8

[WADA:World Anti-bad-Duping Agency]

Attachment: Mail folder file (8 mails for test)

Problem is observed also on multipart/mixed mail with no attachment part.
(7) multipart/mixed, single part only in multipart/mixed,
    no Content-Type: header of text data part(message body part)
(8) multipart/mixed, single part only in multipart/mixed,
    Content-Type: unknown/unknown of text data part(message body part)
In (3) Content-Type: unknown/unknown and (4) Content-Type: ???### cases, phenomenon of "message body is not shown by view" is also observed in Tb 11.0.1.

Quick check result with a Tb 7.0a2 build. 
> Mozilla/5.0 (Windows NT 5.1; rv:7.0a2) Gecko/20110816 Thunderbird/7.0a2
Note: "This bug occurs" in Tb 7.0a2 = Attachment part is created by Forward,
                                      data is message body text instead of null.
(1) No Content-Type: header         This bug doesn't occur
(2) No parameter in Content-Type:   This bug doesn't occur
(3) Content-Type: unknown/unknown   Attachment is shown by view.
                                    This bug occurs. 
(4) Content-Type: ???###            Attachment is shown by view.
                                    This bug occurs. Bug 738284 occurs.
(5) Content-Type: text/???          This bug occurs.
(6) Content-Type: text/unknown      This bug occurs.
(7) multipart/mixed, single part only in multipart/mixed,
    no Content-Type: header of text data part(message body part)
                                    This bug doesn't occur.
(8) multipart/mixed, single part only in multipart/mixed,
    Content-Type: unknown/unknown of text data part(message body part)
                                    Attachment is shown by view.
                                    This bug occurs.

No Content-Type: case and No parameter of Content-Type: case looks regression.
Other cases seems old problem.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: [checked in] Fix

This bug is a regression by bug 679476.

Comment 11

[David :Bienvenu]

Comment on attachment 629984 [details] [diff] [review]
[checked in] Fix

thx, Hiro, yes, that gets the code back to what it did before the regression.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/d5a2b2d481d5

Comment 13

[lightsolphoenix]

A few employees at my company just upgraded to Thunderbird 15, and we're seeing this bug again; phantom empty "nsmail.tmp" being created when someone hits Forward.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Please read comment 12. This issue will be solved in Thunderbird 16. Sorry for the patience.

Comment 15

[Thiago]

This bug also exists in TB 16.0.1(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)
> Please read comment 12. This issue will be solved in Thunderbird 16. Sorry
> for the patience.

This bug also exists in TB 16.0.1

Comment 16

[Steve L]

We are seeing this problem but only with a few machines here. We are running TB 16.0.2 on Windows XP pro. Anyone have the solution?

Comment 17

[robert1]

Still seeing this at TB 17.0 running on Windows 7 64 bit.

Comment 18

[seans]

Confirmed and seeing this problem on Thunderbird Windows 7 x64 in 17.0.2 even with add-ons disabled

Comment 19

[Bassam]

I'm having this issue as well, it doesn't do it on all emails, but the ones it does do this on if you reply it does no attachment. This only happens when forwarding.

Comment 20

[Serg Iv]

The problem is still here in 17.0.2. Can someone reopen this bug?

Comment 21

[Serg Iv]

Attachment: email that cause the problem

email that cause the problem

Comment 22

[WADA:World Anti-bad-Duping Agency]

> http://hg.mozilla.org/comm-central/annotate/65b8e2fa4eba/mailnews/mime/src/mimedrft.cpp#l489
>           hg@0 489 //It's possible we must treat the message body as attachment!
>       mwu@8587 490 bool bodyAsAttachment = false;
> hiikezoe@10396 491 if (mdd->messageBody &&
> hiikezoe@10396 492 mdd->messageBody->m_type.Find("text/html", CaseInsensitiveCompare) == -1 &&
> hiikezoe@10396 493 mdd->messageBody->m_type.Find("text/plain", CaseInsensitiveCompare) == -1 &&
> hiikezoe@10396 494 !mdd->messageBody->m_type.LowerCaseEqualsLiteral("text"))
>  bugzilla@9082 495 bodyAsAttachment = true;
Patch by this bug is still active.

I also see problem in Tb 17.0.3 on Win-XP as Serg Iv says, and as seen in Bug 846343. With any test mail of my attachment 616015 [details] , I could still see problem of this bug.

Re-opening.

Ikezoe-san, did you actually test the patch?

Is default string of "text/plain" set in mdd->messageBody when no Content-Type: header?
If no, it looks for me the patch resolved problem with "Content-Type: Text" like case...

Comment 23

[WADA:World Anti-bad-Duping Agency]

mistake in my pasting... sorry for spam.
  mdd->messageBody  ==>  mdd->messageBody->m_type

Comment 24

[Ed L]

Also seeing this issue in Tbird 17.0.3 on Win 7 64bit, x-ref bug report 846343

Comment 25

[Bassam]

Hello  WADA ,

Is there anything I can do to help you guys solve this? Where on Thunderbird can I see the default string for content type?

This issue is still on 17.03.

Comment 26

[WADA:World Anti-bad-Duping Agency]

Following is compose format determination logic.
> http://mxr.mozilla.org/comm-central/source/mailnews/mime/src/mimedrft.cpp#1336
> 1336     if (mdd->messageBody)
> 1337     {
> 1338       MSG_ComposeFormat composeFormat = nsIMsgCompFormat::Default;
> 1339       if (!mdd->messageBody->m_type.IsEmpty())
> 1340       {
> 1341         if(mdd->messageBody->m_type.Find("text/html", CaseInsensitiveCompare) != -1)
> 1342           composeFormat = nsIMsgCompFormat::HTML;
> 1343         else if (mdd->messageBody->m_type.Find("text/plain", CaseInsensitiveCompare) != -1 ||
> 1344                  mdd->messageBody->m_type.LowerCaseEqualsLiteral("text"))
> 1345           composeFormat = nsIMsgCompFormat::PlainText;
> 1346         else
> 1347           //We cannot use this kind of data for the message body! Therefore, move it as attachment
> 1348           bodyAsAttachment = true;
> 1349       }
> 1350       else
> 1351         composeFormat = nsIMsgCompFormat::PlainText;

Ikezoe-san, I think following is better. Is it right?
> 489   //It's possible we must treat the message body as attachment!
> 490   bool bodyAsAttachment = false;
> 491   if (mdd->messageBody &&    ===>    if (mdd->messageBody && !mdd->messageBody->m_type.IsEmpty() &&
> 492       mdd->messageBody->m_type.Find("text/html", CaseInsensitiveCompare) == -1 &&
> 493       mdd->messageBody->m_type.Find("text/plain", CaseInsensitiveCompare) == -1 &&
> 494       !mdd->messageBody->m_type.LowerCaseEqualsLiteral("text"))
> 495      bodyAsAttachment = true;

Comment 27

[WADA:World Anti-bad-Duping Agency]

And, if TEXT, Text, texT(no / nor subtype) is considered text, I think TEXT/, Text/, texT/ (with / and no subtype) is better considered text.

Comment 28

[WADA:World Anti-bad-Duping Agency]

Some other thoughts on code.
1. If texT/ like one is cared, comparison like CaseInsensiveStartWith("text/") is better.
2. If logic like 1336 to 1351 is used at multiple places, Objects's method like mdd->messageBody->m_type.Is_text_family() may be useful, because duplicate code is eliminated.

Comment 31

[Jonathan Kamens]

I have regressed bug 745919 and confirmed that the first nightly build in which it appears is 2011-09-23. That means that it was almost certainly caused by the commit in bug 679476, which landed on 2011-09-22.

That means that the bug has been in the code base for over 18 months, and for over 9 months since it was allegedly fixed by the commit in bug 745919 which didn't actually fix it.

It would be nice if one of the people responsible for writing or reviewing the commit in bug 679476, or one of the people responsible for writing or reviewing the patch in bug 745919, would step up and fix this bug properly.

It's pretty appalling that it's still in the code base after over 18 months.

Comment 32

[Magnus Melin [:mkmelin]]

I have a fix for this, thx for the detective work WADA!

Comment 33

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Fix the case of non-mime message (with no content type), and add test. (Well, testing bug 856456 which this also fixes, and tagging on the test there's no attachment either.)

Comment 34

[Bassam]

So what is the fix for this problem now, is there even one?

Comment 35

[Magnus Melin [:mkmelin]]

If you do your own build, apply the patch. Otherwise wait until the patch is reviewed and checked in, then it is fixed in nightly builds.

Comment 36

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 737167 [details] [diff] [review]
proposed fix

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

Just some nits here, but I think this looks alright.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +2558,5 @@
>  {
>    var send_default_charset = gMsgCompose.compFields.defaultCharacterSet;
>  //  dump("send_default_charset is " + send_default_charset + "\n");
>  
> +  var compFieldsCharset = gMsgCompose.compFields.characterSet ||

While you're here, let not var.

::: mail/test/mozmill/composition/test-save-changes-on-quit.js
@@ +34,5 @@
>    collector.getModule("prompt-helpers").installInto(module);
>    collector.getModule("window-helpers").installInto(module);
>  
>    folder = create_folder("PromptToSaveTest");
> +  

Trailing whitespace.

@@ +44,5 @@
>  
> +function msgSource(aSubject, aContentType) {
> +  let msgId = Components.classes["@mozilla.org/uuid-generator;1"]
> +                        .getService(Components.interfaces.nsIUUIDGenerator)
> +                        .generateUUID() +"@invalid";

Space after +

@@ +247,5 @@
> +/**
> + * Test there's no prompt on close when no changes was made in reply/forward
> + * windows - for the case the original msg had content type "text".
> + */
> +function test_no_prompt_on_close_for_unmodified_content_type_text() {

Nice tests.

::: mailnews/mime/src/mimedrft.cpp
@@ +478,5 @@
>    nsMsgAttachedFile           *tmpFile = NULL;
>  
>    //It's possible we must treat the message body as attachment!
>    bool bodyAsAttachment = false;
> +  if (mdd->messageBody && !mdd->messageBody->m_type.IsEmpty() &&

Let's put this new term on its own line.

Comment 37

[Bassam]

I don't have my own build, do you know how long it will be before Thunderbird adds this fix to their newer builds?

Comment 38

[Magnus Melin [:mkmelin]]

It will be in nigthly builds in a guess a day or a few, depending on when i have the chance to update the patch and check it in. (When i mark this bug FIXED, it's in the next nigthly build.)

Comment 39

[Magnus Melin [:mkmelin]]

http://hg.mozilla.org/comm-central/rev/eabed5c2dfe1 -> FIXED

Removing the part about corrupted mime-type from summary. The patch didn't change those, and i don't know how/if those shoud be handled - in any case they should be rare.

Comment 40

[tony]

I'm seeing this bug again when inline forwarding an email with a pdf attachment.

I get a composition window with the attachment "Attached Message part" which when opened with notepad, shows that it's actually an html file with the entire email in it.

Switching preferences to send forwards as attached emails works as expected.

Comment 41

[tony]

(In reply to tony from comment #40)

I'm seeing this bug again when inline forwarding an email with a pdf attachment.

I get a composition window with the attachment "Attached Message part" which when opened with notepad, shows that it's actually an html file with the entire email in it.

Switching preferences to send forwards as attached emails works as expected.

Sorry I forgot the version.
91.4.1 64 bit
Release update channel.