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

RESOLVED FIXED in Thunderbird 13.0

Status

MailNews Core
Backend
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: Arkady Belousov, Assigned: hiro)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 13.0
x86
All

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 4 obsolete attachments)

(Reporter)

Description

11 years ago
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

11 years ago
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.
Status: UNCONFIRMED → NEW
Component: Mail Window Front End → MailNews: Backend
Ever confirmed: true
Product: Thunderbird → Core
Summary: Saving letter removes starting dot in all lines → Saving as .EML removes starting dot in all lines
Version: unspecified → Trunk

Comment 2

11 years ago
*** Bug 227870 has been marked as a duplicate of this bug. ***
Blocks: 269826
Assignee: mscott → nobody
QA Contact: backend
Product: Core → MailNews Core

Comment 3

9 years ago
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.
(Assignee)

Comment 4

5 years ago
I can not reproduce this.
Can anyone still confirm this?

Comment 5

5 years ago
Yes, with 9.0.1, WinXP/64.

It's easy to reproduce; see the dupe.
(Assignee)

Comment 6

5 years ago
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

5 years ago
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
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
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
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.
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.
(Assignee)

Comment 12

5 years ago
Changing bug summary...
Summary: Saving as .EML removes starting dot in all lines → Saving message in local folder as .EML removes starting dot in all lines

Comment 13

5 years ago
Thanks, WADA -- I would never have noticed the difference between saving from IMAP and saving from local.
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.
Summary: Saving message in local folder as .EML removes starting dot in all lines → Saving message in local folder as .EML removes starting dot in all lines, and ignores line if single dot only line.
OS: Windows 98 → All
(Assignee)

Comment 15

5 years ago
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.
(Assignee)

Comment 16

5 years ago
Created attachment 593702 [details] [diff] [review]
A test

I posted wrong patch..
Attachment #593671 - Attachment is obsolete: true
(Assignee)

Comment 17

5 years ago
Created attachment 593732 [details] [diff] [review]
Fix

Fix suggested by WADA. Thanks!
Attachment #593732 - Flags: review?(dbienvenu)

Comment 18

5 years ago
Comment on attachment 593702 [details] [diff] [review]
A test

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

Comment 19

5 years ago
(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

5 years ago
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

5 years ago
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;
Attachment #593732 - Flags: review?(dbienvenu) → review+
(Assignee)

Comment 22

5 years ago
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.
Assignee: nobody → hiikezoe
Attachment #593732 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #594874 - Flags: review+

Comment 23

5 years ago
(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.
Hiroyuki Ikezoe , what's the status of these patches and this bug ?
(Assignee)

Comment 25

5 years ago
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.
Attachment #594874 - Attachment is obsolete: true
Attachment #601801 - Flags: review+
(Assignee)

Comment 26

5 years ago
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..
Attachment #593702 - Attachment is obsolete: true
Attachment #601811 - Flags: review?(dbienvenu)

Updated

5 years ago
Attachment #601811 - Flags: review?(dbienvenu) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
You need to log in before you can comment on or make changes to this bug.