339595 - Saving message in local folder as .EML removes starting dot in all lines, and ignores line if single dot only line.

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Arkady Belousov]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows 98; ru) Opera 8.54
Build Identifier: http://mozilla.mirrors.tds.net/pub/mozilla.org/thunderbird/releases/1.5.0.2/win32/ru/Thunderbird%20Setup%201.5.0.2.exe

In some letter I insert fragment of makefile with implicit rule (which contains lines like ".c.obj:"). After saving this file to disk (File / Save as / File / File type: all files) in resulting file starting dots disappear (ie. I get "c.obj:").


Reproducible: Always

Comment 1

[Mike Cowperthwaite]

Huh.  I never thought to test the entire message body when I filed bug 227870.

This has been happening since Moz 1.5, at least.  I don't know whether non-Windows platforms are affected.

Comment 2

[Mike Cowperthwaite]

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

Comment 3

[Cesare Trapali]

A possible workaround for this bug is the following:

instead of the sequence (File / Save as / File ......

use (View / Message Source) and in the new window (File / Save Page As ......

The file saved in this way keeps startin dots.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

I can not reproduce this.
Can anyone still confirm this?

Comment 5

[Mike Cowperthwaite]

Yes, with 9.0.1, WinXP/64.

It's easy to reproduce; see the dupe.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Thank you for the immediate response, but unfortunately I can not reproduce it yet on my linux box. 

Is this bug possibly platform-specific?

Comment 7

[Jim Porter (:squib)]

I can't reproduce this on any of the following configurations:

* Win 7 64-bit, TB 9.0.1
* Linux 64-bit, TB 12.0a1
* Win XP 32-bit, TB 12.0a1

Comment 8

[WADA:World Anti-bad-Duping Agency]

Attachment: mail folder file, test mail with line starts with dot

Next line is contained in text/plain mail
.a.b.c
.A.B.C

Comment 9

[WADA:World Anti-bad-Duping Agency]

Attachment: .eml file saved by "Save As", ".eml"

Lines are changed to next by "Save As .eml"
a.b.c
A.B.C

Comment 10

[WADA:World Anti-bad-Duping Agency]

Attachment: SMTP log, POP3 log, IMAP log for ".email" line in message

Same mail with ".email" line is sent to both IMAP account and POP3 account.

As seen in IMAP log, mail data lines is as follows

Start of test
email
.email
. email
..email
...email
End of test

As seen in POP3 log, these lines in message is sent from POP3 server with starting dot escaped by "..".

Start of test
email
..email
.. email
...email
....email
End of test

As seen in SMTP log(SMTP log doesn't log mail data),
  354 Enter mail, end with "." on a line by itself
"." only line is "End of data indicator" when SMTP, so SMTP defines rule of "escaping of starting dot". 
SMTP client should send ".." if a line starts with dot, and Tb does do it, and SMTP server removes it upon receive, and SMTP server escapes it with ".." when passing mail data to other SMTP.
This "escaping of starting dot" is POP3 spec too, because "." only line is "End of data indicator" in POP3 too. So POP3 client always removes first dot when POP3.
However, IMAP doesn't have such spec, so IMAP server sends ".email" line as-is.

When folder of "Local Mail Folder", "removal of starting dot upon Save As .eml" was observed too.
However, if IMAP folder, "removal of starting dot upon Save As .eml" was not observed.
I guess this is reason why some one can see this bug but other one can not see this bug.

Tb perhaps considers mail data in local mail folder(POP3) as "mail data stream from POP3 server" when "Save As, .eml".
This may be due to old quirks in "Save As" for problem like "starting dot is not removed upon download from POP3 server".
Because Tb correctly removes starting dot upon download from POP3 server, Tb shouldn't remove starting dot once received from POP3 server.

Comment 11

[WADA:World Anti-bad-Duping Agency]

FYI.
Following is bug found by serach for "begin dot". bug 170727 looks for NNTP.
> bug  74446 FIXE Extra dot appearing at the beginning of lines in the body
> bug 170727 ---  A point [dot] "." at the beginning of a line will be doubled sometimes 
> bug 278329 DUPL Thunderbird adds a dot character if a line begins with dots.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Changing bug summary...

Comment 13

[Mike Cowperthwaite]

Thanks, WADA -- I would never have noticed the difference between saving from IMAP and saving from local.

Comment 14

[WADA:World Anti-bad-Duping Agency]

Patch for bug 74446 added logic for "removal of dot for escaping of dot" (corresponds to next code. not same source) to nsPop3Protocol.cpp.
> http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Protocol.cpp#3627
> 3624     /* Check if the line begins with the termination octet. If so
> 3625        and if another termination octet follows, we step over the
> 3626        first occurence of it. */
> 3627     else if (line_length > 1 && line[0] == '.' && line[1] == '.') {
> 3628         line++;
> 3629         line_length--;
So, after fix of bug 74446(fixed on 2001-04-04), "starting dot for escaping dot" was correctly removed when mail data line is written to file for local mail folder.

However, same "removal of starting dot" was still executed in nsMailboxProtocol.cpp.
> http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsMailboxProtocol.cpp#655
> 648       if (!line || (line[0] == '.' && line[1] == 0))
> 649       {
> 650         // we reached the end of the message!
> 651         ClearFlag(MAILBOX_PAUSE_FOR_READ);
> 652       } // otherwise process the line
> 653       else
> 654       {
> 655         if (line[0] == '.')
> 656           line++; // skip over the '.'
Further, because "End of Message detection by dot only line" was also coded in nsMailboxProtocol.cpp, skip of "dot only line" was also observed.
Original mail data.
.P.Q.R[CRLF]
.[CRLF]
.X.Y.Z[CRLF]
Save As to .eml file => Dot only line was lost.
.P.Q.R[CRLF]
.X.Y.Z[CRLF]

nsMailboxProtocol.cpp was perhaps initially created from copy of nsPop3Protocol.cpp, and who copied from nsPop3Protocol.cpp perhaps forgot to remove avove logic from nsMailboxProtocol.cpp.

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Attachment: A test

This test depends on the fix for bug 722920.

Unfortunately this test revealed another issue that saving a message appends two extra blank lines at the end of the message.

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Attachment: A test

I posted wrong patch..

Comment 17

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix

Fix suggested by WADA. Thanks!

Comment 18

[David :Bienvenu]

Comment on attachment 593702 [details] [diff] [review]
A test

the test fails because copyFileMessageInLocalFolder is not defined anywhere...

Comment 19

[David :Bienvenu]

(In reply to David :Bienvenu from comment #18)
> Comment on attachment 593702 [details] [diff] [review]
> A test
> 
> the test fails because copyFileMessageInLocalFolder is not defined
> anywhere...

ah, I see, you already explained that it was dependent on a different patch, sorry.

Comment 20

[David :Bienvenu]

Comment on attachment 593702 [details] [diff] [review]
A test

this comment is wrong:

+/*
+ * Test bug 460636 - nsMsgSaveAsListener sometimes inserts extra LF characters
+ */
+

In order to get this test to pass on windows, I had to change this line:

+function check_each_line(aExpectedLines, aActualLines) {
+  let expectedStrings = aExpectedLines.split("\n");

to use \r\n, because the "dot" file has crlf endings. Haven't tried it on mac...

Comment 21

[David :Bienvenu]

Comment on attachment 593732 [details] [diff] [review]
Fix

thx for the patch, seems to work...

some nits:
You don't need the extra braces here:

+        if (canonicalLineEnding)
+        {
+          rv = m_msgFileOutputStream->Write(CRLF, 2, &count);
+        }
+        else
+        {
+          rv = m_msgFileOutputStream->Write(MSG_LINEBREAK,
+                                            MSG_LINEBREAK_LEN, &count);
+        }

the PR_FREEIF can be just PR_Free:

-    while (line && !pauseForMoreData);
+    PR_FREEIF(line);

the break should be on it's own line (two instances of this):
+        if (NS_FAILED(rv)) break;

Comment 22

[Hiroyuki Ikezoe (:hiro)]

Attachment: Style fix

> the PR_FREEIF can be just PR_Free:
> 
> -    while (line && !pauseForMoreData);
> +    PR_FREEIF(line);

Can't. Because ReadNextLine possibly returns nsnull.

Comment 23

[David :Bienvenu]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #22)
> Created attachment 594874 [details] [diff] [review]
> Style fix
> 
> > the PR_FREEIF can be just PR_Free:
> > 
> > -    while (line && !pauseForMoreData);
> > +    PR_FREEIF(line);
> 
> Can't. Because ReadNextLine possibly returns nsnull.

PR_Free checks for a null pointer, and is fine with it. The difference between PR_Free and PR_FREEIF is that PR_FREEIF nulls out the line pointer, but since we're exiting the scope on the next statement, that doesn't buy us anything.

Comment 24

[Ludovic Hirlimann [:Usul]]

Hiroyuki Ikezoe , what's the status of these patches and this bug ?

Comment 25

[Hiroyuki Ikezoe (:hiro)]

Attachment: Use PR_Free instead of PR_FREEIF.

Ludovic, thanks for the notice.  I did forget this.

I actually misunderstand about PR_FREEIF. David, thank you for your pointing out.

Comment 26

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix MSGLINEBREAK

Use "\r\n" for both expected and actual strings on all platform.

I misunderstood that the linebreak is converted to "\n" on Linux and Mac..

Comment 27

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/comm-central/rev/0364b7b1d13e
http://hg.mozilla.org/comm-central/rev/dfe4636e667d

For future patches, please try to make the checkin message a summary of what the patch is changing rather than just reiterating the bug title.