248121 - "X-Mozilla-Status" and spaces inserted into received mail

Status: RESOLVED FIXED

(MailNews Core :: Movemail, defect)

Description

[smcx1]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040113
Build Identifier: 1.7

When reading mail with Mozilla 1.7 frequently (but not in every message).
I see stuff like:
X-Mozilla-Status: 8000
X-Mozilla-Status2: 00000000 
in the middle of the messages. The messages were not sent with Mozilla.
I also see additional spaces (some spaces appear to be duplicated) in received
messages.
This makes mozilla 1.7 unusable for me and I've reverted to 1.6
Using Red Hat Fedora on an Intel machine and movemail.

Reproducible: Sometimes
Steps to Reproduce:
1. Read mail messages!
2.
3.

Actual Results:  
See bogus X-Mozilla-Status messages and spaces in received mail

Expected Results:  
Shown the message as sent!

Comment 1

[Christian Eyrich]

X-Mozilla-Status and X-Mozilla-Status2 are header lines inserted by each (in any
version) _receiving_ Mozilla. They hold informations on the status of the mail
and things like the label markings.
But all these X-Mozilla lines (also X-Account-Key and X-UIDL) should be in the
topmost lines of the message source, not the middle of the message.
Er, what do you meant with "middle of the message"? And where are these spaces
"inserted"? The header part or the body?

Would be nice if you could attach one or two of these affected messages (in .eml
format with "Create a New Attachment" above)

Comment 2

[Mike Cowperthwaite]

ref bug 196749

Comment 3

[Bertram Franz]

Hi,

I was about to submit this bug again.

I have a little more Info about it (see
http://forums.mozillazine.org/viewtopic.php?p=670527):

- it happens only with HTML email
- it happens only, if the HTML source has a line break immediately before the
word 'from'

For instance the following paragraph from the email message source:

<P><SPAN LANG=3D"de"><FONT FACE=3D"Arial">Towards the end of July, each =
Business Group in EMEA will publish its strategic action plan to address =
each of the above focus areas. You'll be hearing more about this work =
from your organization's management team, and during the upcoming August =
X-Mozilla-Status: 8000
X-Mozilla-Status2: 00000000
quarterly Coffee talks. </FONT></SPAN></P>

I'm using Mozilla 1.7/HP-UX (Mozilla/5.0 (X11; U; HP-UX 9000/785; en-US; rv:1.7)
Gecko/20040617).

(In reply to comment #1)

Comment 4

[Andy Giles]

I have also seen this problem but it does not seem to be restricted to HTML
formated messages. If there is a "from" at the begining of the line in the
received message something is inserting

X-Mozilla-Status: 8000
X-Mozilla-Status2: 00000000

into the message. This can be seen as soon as the message is displayed in the
inbox. I use movemail and looking through the source a very likely culprit is
around line 429 in nsMovemailService.cpp.

        // 'From' lines delimit messages
        if (isMore && !PL_strncasecmp(buffer.get(), "From ", 5)) {
            buffer = NS_LITERAL_CSTRING("X-Mozilla-Status: 8000" MSG_LINEBREAK);
            newMailParser->HandleLine(buffer.BeginWriting(), buffer.Length());
            outFileStream << buffer.get();
            buffer = NS_LITERAL_CSTRING("X-Mozilla-Status2: 00000000"
MSG_LINEBREAK);
            newMailParser->HandleLine(buffer.BeginWriting(), buffer.Length());
            outFileStream << buffer.get();
        }

However, a fix does not leap out at me as I haven't had time to digest the
movemail process.

I'm using mozilla 1.7.1 on Solaris 9 sparc compiled with Sun Forte 7.

Comment 5

[Mike Cowperthwaite]

Original report also mentions MoveMail.

Bertram Franz, are you also using MoveMail?

Is this bug a regression?

Comment 6

[Bertram Franz]

Yes, I'm also using MoveMail.

I had seen this only in HTML email, because my mailer (or some bridge?) always
inserts a '>' before any 'From' in a new line of any non-HTML email.

Bertram

Comment 7

[Mike Cowperthwaite]

Updating component; all three reporters are using Movemail.

Comment 8

[Philip K. Warren]

I know exactly why this is occuring - lines beginning with "From" delimit
messages in Movemail accounts - however we are currently doing a case
insensitive comparison, which I'm not sure is really needed.

http://lxr.mozilla.org/seamonkey/source/mailnews/local/src/nsMovemailService.cpp#426

Comment 9

[timeless]

pkw, it seems you accidentally introduced this, 
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/local/src/
nsMovemailService.cpp&rev=1.42&mark=429#422

Comment 10

[Philip K. Warren]

Attachment: Patch v1

This was definitely my mistake.

Comment 11

[David :Bienvenu]

that's only somewhat better - you can still have a line that starts with "From "
as opposed to "from ", which will still cause the bug.

Is there a way in movemail to know when we've actually got a new message?

Comment 12

[Philip K. Warren]

Well this is the behavior that is has been for several years before I broke it
in Bug 239583.

We could definitely improve the parsing. The FreeBSD mail.local man page
describes the exact format we are looking for in the mail spool file:

"Individual mail messages in the mailbox are delimited by an empty  line
followed  by  a	line beginning with the string ``From ''.  A line con-
taining the string ``From '', the sender's name and  a  time  stamp  is
prepended  to each delivered mail message.  A blank line is appended to
each message.  A greater-than character (``>'')	is  prepended  to  any
line  in  the message which could be mistaken for a ``From '' delimiter
line (that is, a line beginning with the five characters ``From '' fol-
lowing a blank line)."

Comment 13

[David :Bienvenu]

Comment on attachment 155711 [details] [diff] [review]
Patch v1

ah, yes, you're right. Can you file a new bug on improving the parsing for the
"From " line?

Comment 14

[Philip K. Warren]

Bug 255048 filed. bienvenu: Can I check this in currently with your review +
super review?

Comment 15

[David :Bienvenu]

Comment on attachment 155711 [details] [diff] [review]
Patch v1

yes, that's fine, thx.

Comment 16

[Philip K. Warren]

Attachment: Patch v2

This patch takes it a step further, enforcing the rule that the preceding line
to a "From " line should be an empty line.

Comment 17

[David :Bienvenu]

Comment on attachment 155720 [details] [diff] [review]
Patch v2

cut and paste error:

+    mozillaStatus2.AssignLiteral("X-Mozilla-Status: 00000000" MSG_LINEBREAK);

this needs to be "X-Mozilla-Status2:

other than that, it looks OK, but could you run a few tests and make sure
everything looks OK in the resulting folder? In particular, the first new
message should work OK, because you've initialized prevLineEmpty to PR_TRUE,
and receiving multiple messages should trigger the prevLineEmpty code correctly
(so we're sure that movemail is putting a blank line before the start of
messages?)

Comment 18

[Philip K. Warren]

(In reply to comment #17)
> +    mozillaStatus2.AssignLiteral("X-Mozilla-Status: 00000000" MSG_LINEBREAK);

Good catch.

> other than that, it looks OK, but could you run a few tests and make sure
> everything looks OK in the resulting folder? In particular, the first new
> message should work OK, because you've initialized prevLineEmpty to PR_TRUE,
> and receiving multiple messages should trigger the prevLineEmpty code correctly
> (so we're sure that movemail is putting a blank line before the start of
> messages?)

Yes it is working as expected. I have tested on AIX and Linux here, using both
Sendmail and Postfix to deliver messages. Sending a message which contains "From
" at the beginning of a line in the body of a message results in it being
escaped by a ">" whether or not the previous line in the message was empty. I am
not sure what the behavior is with other MTAs.

I think we could just land the simpler first patch to address the problem
immediately (and would probably be able to land it on the 1.7 branch more
easily). There were never any reports of issues with inserting the
X-Mozilla-Status lines in messages until my checkin for Bug 239583.

Comment 19

[Philip K. Warren]

Checked in the first patch.

Checking in nsMovemailService.cpp;
/cvsroot/mozilla/mailnews/local/src/nsMovemailService.cpp,v  <-- 
nsMovemailService.cpp
new revision: 1.49; previous revision: 1.48
done

Comment 20

[Philip K. Warren]

Comment on attachment 155711 [details] [diff] [review]
Patch v1

This is a simple fix which should be checked in to the 1.7 branch to fix a
regression from Bug 239583.

Comment 21

[Mike Cowperthwaite]

Fast work!

See: bug 121946, bug 121947, bug 194382, bug 248726

Comment 22

[Philip K. Warren]

Requesting additional approval to check in to the aviary branch.

Comment 23

[David :Bienvenu]

Phillip, go ahead and check it into the aviary branch - for mail stuff, you just
need Scott or my blessing, I believe. Thx.

Comment 24

[Philip K. Warren]

Fixed on aviary branch.

Checking in nsMovemailService.cpp;
/cvsroot/mozilla/mailnews/local/src/nsMovemailService.cpp,v  <-- 
nsMovemailService.cpp
new revision: 1.42.6.2; previous revision: 1.42.6.1
done

Comment 25

[Philip K. Warren]

Fixed on 1.7 branch.

Checking in nsMovemailService.cpp;
/cvsroot/mozilla/mailnews/local/src/nsMovemailService.cpp,v  <-- 
nsMovemailService.cpp
new revision: 1.42.2.1; previous revision: 1.42
done

Comment 26

[Peter B. West]

Is this supposed to be fixed in 1.7.3?  (It isn't.)

Comment 27

[David :Bienvenu]

no, I think it was fixed after 1.7.3 came out, hence the keyword set to 1.7.x,
not 1.7.3.     1.7.4 or later should have the fix.

Comment 28

[Philip K. Warren]

*** Bug 263533 has been marked as a duplicate of this bug. ***