172104 - multiple subject headers causes different subject in thread pane than preview pane, should use first subject

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[juggy]

I just received an email in which - for whatever reason - two subject lines
existed. The first was shown when the mail was selected (in the subject field),
the second (=last?) in the message-list-window and mozillanews-title display.

Comment 1

[Robert Sander]

We have seen it here, too.

Comment 2

[Robert Sander]

Attachment: The screenshot show a message display window with a different title than the subject line contains

Comment 3

[Mike Cowperthwaite]

Still occurs with Mozilla 1.3 and 1.4b-0516.  This is closely related to bug 
156588.

Comment 4

[David :Bienvenu]

yes, the msg parsing code used to generate the db header will use the last
subject. We could easily make it use the first subject.

Comment 5

[George Stachovskis]

Greetings . . .

I noted the same problem when I received a "phising" email purportedly from PayPal.

The "Subject" line displayed in the Thunderbird InBox is . . .

     Important information PayPal customers. confirmation code 9832-JGQKY

. . . but the "Subject" line that is displayed when the e-mail is viewed is:

     -- Spam -- PayPal Security Request confirmation code  421-RRO

My ISP, COX Communications, inserted the "-- Spam -- " prefix in the latter Subject for know spammers, but somehow this is not the Subject line that is presented to my Thunderbird client, which has a Filter designed to move such incoming e-mail to the "Junk" folder.

Are MULTIPLE "Subject" lines allowed in an e-mail message?  UGH!

Thanks for your attention.

George

Comment 6

[Wayne Mery (:wsmwk)]

David in comment #4:
> msg parsing code used to generate the db header will use the last
> subject. We could easily make it use the first subject.

Is there a RFC that requires use of the first line or last line?

spamassassin only tags the FIRST subject line, thus filtering of [SPAM] via suite and thunderbird is defeated (as noted in bug 310189)

Comment 7

[Karsten Düsterloh]

> Is there a RFC that requires use of the first line or last line?

Not as such, but RfC 2822 limits the number of subject headers to 0-1, while RfC 822 just says:
            This specification permits multiple  occurrences  of  most
            fields.   Except  as  noted,  their  interpretation is not
            specified here, and their use is discouraged.
and there's no special remark about the subject header there.

Comment 8

[Wayne Mery (:wsmwk)]

hmm, which field is being used by junk filtering, the faulty(?) non-first subject shown in the thread pane?

RFC 2822 of course obsoletes rfc 822. I don't know what other spam packages do, but spamassassin flags only the FIRST subject field

related SA bugs: 
 http://issues.apache.org/SpamAssassin/show_bug.cgi?id=1239 *
 http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4927 

* SA v3.17 is not removing multiple subject lines, contrary to SAbug 1239 "In 2.50, multiple subjects are removed and replaced with a single rewritten one
(I believe the first subject)"

Comment 9

[Wayne Mery (:wsmwk)]

David in comment #4:
> yes, the msg parsing code used to generate the db header will use the last
> subject. We could easily make it use the first subject.

David, sounds like that's the way to go. Besides it should be consistent, no?

I filed SA bug - http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5181
- SA outputs two subjects, the first subject with [SPAM] and the second subject the untouched original.

Comment 10

[Wayne Mery (:wsmwk)]

example where it's better to use first subject.  filters and thread pane are using subject 2. But message preview pane shows subject 1.


-- after SA processsing - SA has tagged subject 1 --

To: [e-mail]
Subject: [SPAM] A few drops a day is all thats needed to create a leaner fitter you
From: "[e-mail] <to6332"@playandwin.co.uk
Content-Transfer-Encoding: 7bit
Content-Type: multipart/mixed; boundary="----------=_460125D6.BAF3E3EA"
Subject: A few drops a day is all thats needed to create a leaner fitter you
Message-Id: <[e-mail]>
Date: Wed, 21 Mar 2007 13:32:18 +0100 (CET)
X-Virus-Scanned: ClamAV 0.90.1/2892/Wed Mar 21 06:40:09 2007 on blah
X-Virus-Status: Clean
X-Spam-Flag: YES
X-Spam-Status: Yes, score=26.8 required=5.0 tests=HEADER_COUNT_SUBJECT,
	MAILTO_TO_SPAM_ADDR,RAZOR2_CF_RANGE_51_100,RAZOR2_CF_RANGE_E8_51_100,
	RAZOR2_CHECK,SARE_URI_DIGITS4,URIBL_BLACK,URIBL_JP_SURBL,URIBL_SBL,
	URIBL_SC_SURBL,URIBL_WS_SURBL autolearn=disabled version=3.1.8
X-Spam-Level: **************************
X-Spam-Checker-Version: SpamAssassin 3.1.8 (2007-02-13) on blah
...


-- original, before SA --

To: [e-mail]
Subject: PlayAndWin.co.uk - [e-mail]
From: "[e-mail] <to6332"@playandwin.co.uk
Content-Transfer-Encoding: 7bit
Content-Type: text/plain
Subject: A few drops a day is all thats needed to create a leaner fitter you

Comment 11

[Wayne Mery (:wsmwk)]

spam issue (with probably trivial solution) should be priority for TB3 - despite the fact that but has existed lo for 5 years. Tthis is just begging for some spammers to take advantage of this loophole on an big scale.

Junk processing affected (comment 8)?

Comment 12

[Kent James (:rkent)]

I'm pretty sure that the junk processing will add tokens from both subject headers. Haven't tested it though. So this issue is with filters only, which are probably used rarely enough that it would not be worth an exploit.

Comment 13

[Wayne Mery (:wsmwk)]

potentially affects gloda.
not good to be keying on one subject string, but user sees a different string.

Comment 14

[Karsten Düsterloh]

Maybe we should just coalesce all subject headers for display (only)?

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

(In reply to comment #13)
> potentially affects gloda.
> not good to be keying on one subject string, but user sees a different string.

Gloda gets the subject from the nsIMsgDBHdr, so the presentation issue is the same as with the thread pane.  Fix the thread pane consistency, fix gloda.

Comment 16

[Magnus Melin [:mkmelin]]

Wouldn't block for this edge case; blocking‑thunderbird3-

Comment 17

[Jae-Seong Lee-Russo]

Attachment: Use the first subject, 1

Comment 18

[David :Bienvenu]

You can just check m_subject.length(), like we do for other headers where we just want the first header, insead of adding the boolean.

Comment 19

[Jae-Seong Lee-Russo]

Attachment: Use the fist subject, 2: Address comment 18

Comment 20

[Karsten Düsterloh]

Comment on attachment 408272 [details] [diff] [review]
Use the fist subject, 2: Address comment 18

>+++ b/mailnews/local/src/nsParseMailbox.cpp
>-    if (header)
>-      header->value = value;
>+    if (header) {
>+      if (header != &m_subject)
>+        header->value = value;
>+      else
>+        // Bug 172104: Use the fist subject
>+        if (m_subject.length == 0)
>+          header->value = value;
>+    }

This is the wrong place to fix this. 
A couple of lines above, we already do process subject stuff explicitly:
<http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsParseMailbox.cpp#1020>, you should do patching there, cf. line 1040/1041 how to do it.

>     if (header)
>-      header->length = buf - header->value;
>+      if (header != &m_subject)
>+        header->length = buf - header->value;
>+      else
>+        // Bug 172104: Use the fist subject
>+        if(m_subject.length == 0)
>+          header->length = buf - header->value;

This will be unnecessary with the real fix above, because header will be null here in this case.

Thanks for tackling this!

Comment 21

[Jae-Seong Lee-Russo]

Attachment: Use the fist subject, 3: Address comment #20

Comment 22

[David :Bienvenu]

you need an sr before you can checkin.

Comment 23

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/c4f25121bdf7

Comment 24

[WADA:World Anti-bad-Duping Agency]

Removing "defeats subject filtering" part from bug summary to avoid confusion, because it still remains even after fix of this bug and is processed by bug 310189.