Last Comment Bug 339595 - Saving message in local folder as .EML removes starting dot in all lines, and ignores line if single dot only line.
: Saving message in local folder as .EML removes starting dot in all lines, and...
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 13.0
Assigned To: Hiroyuki Ikezoe (:hiro)
:
Mentors:
: 227870 (view as bug list)
Depends on:
Blocks: 269826
  Show dependency treegraph
 
Reported: 2006-05-29 09:06 PDT by Arkady Belousov
Modified: 2012-03-05 17:01 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
mail folder file, test mail with line starts with dot (493 bytes, text/plain)
2012-01-13 17:19 PST, WADA
no flags Details
.eml file saved by "Save As", ".eml" (458 bytes, text/plain)
2012-01-13 17:21 PST, WADA
no flags Details
SMTP log, POP3 log, IMAP log for ".email" line in message (65.07 KB, text/plain)
2012-01-13 18:19 PST, WADA
no flags Details
A test (1.76 KB, patch)
2012-02-01 17:33 PST, Hiroyuki Ikezoe (:hiro)
no flags Details | Diff | Splinter Review
A test (3.29 KB, patch)
2012-02-01 18:58 PST, Hiroyuki Ikezoe (:hiro)
no flags Details | Diff | Splinter Review
Fix (4.14 KB, patch)
2012-02-01 22:49 PST, Hiroyuki Ikezoe (:hiro)
mozilla: review+
Details | Diff | Splinter Review
Style fix (4.11 KB, patch)
2012-02-06 17:49 PST, Hiroyuki Ikezoe (:hiro)
hiikezoe: review+
Details | Diff | Splinter Review
Use PR_Free instead of PR_FREEIF. (4.10 KB, patch)
2012-02-29 15:35 PST, Hiroyuki Ikezoe (:hiro)
hiikezoe: review+
Details | Diff | Splinter Review
Fix MSGLINEBREAK (3.27 KB, patch)
2012-02-29 16:03 PST, Hiroyuki Ikezoe (:hiro)
mozilla: review+
Details | Diff | Splinter Review

Description Arkady Belousov 2006-05-29 09:06:15 PDT
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 2006-05-30 08:01:43 PDT
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 2006-05-30 08:01:58 PDT
*** Bug 227870 has been marked as a duplicate of this bug. ***
Comment 3 Cesare Trapali 2008-09-26 03:51:49 PDT
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) 2012-01-11 17:50:08 PST
I can not reproduce this.
Can anyone still confirm this?
Comment 5 Mike Cowperthwaite 2012-01-12 19:07:36 PST
Yes, with 9.0.1, WinXP/64.

It's easy to reproduce; see the dupe.
Comment 6 Hiroyuki Ikezoe (:hiro) 2012-01-12 20:26:06 PST
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) 2012-01-12 21:54:30 PST
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 2012-01-13 17:19:15 PST
Created attachment 588564 [details]
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 2012-01-13 17:21:21 PST
Created attachment 588566 [details]
.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 2012-01-13 18:19:03 PST
Created attachment 588581 [details]
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 2012-01-13 18:36:09 PST
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) 2012-01-13 18:55:59 PST
Changing bug summary...
Comment 13 Mike Cowperthwaite 2012-01-14 11:27:52 PST
Thanks, WADA -- I would never have noticed the difference between saving from IMAP and saving from local.
Comment 14 WADA 2012-01-14 19:24:37 PST
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) 2012-02-01 17:33:35 PST
Created attachment 593671 [details] [diff] [review]
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) 2012-02-01 18:58:45 PST
Created attachment 593702 [details] [diff] [review]
A test

I posted wrong patch..
Comment 17 Hiroyuki Ikezoe (:hiro) 2012-02-01 22:49:21 PST
Created attachment 593732 [details] [diff] [review]
Fix

Fix suggested by WADA. Thanks!
Comment 18 David :Bienvenu 2012-02-03 11:29:02 PST
Comment on attachment 593702 [details] [diff] [review]
A test

the test fails because copyFileMessageInLocalFolder is not defined anywhere...
Comment 19 David :Bienvenu 2012-02-03 11:30:25 PST
(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 2012-02-03 13:05:01 PST
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 2012-02-03 13:45:42 PST
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) 2012-02-06 17:49:36 PST
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.
Comment 23 David :Bienvenu 2012-02-06 19:17:40 PST
(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] 2012-02-29 06:49:36 PST
Hiroyuki Ikezoe , what's the status of these patches and this bug ?
Comment 25 Hiroyuki Ikezoe (:hiro) 2012-02-29 15:35:12 PST
Created attachment 601801 [details] [diff] [review]
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) 2012-02-29 16:03:37 PST
Created attachment 601811 [details] [diff] [review]
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] 2012-03-05 17:01:46 PST
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.

Note You need to log in before you can comment on or make changes to this bug.