73694 - Incorrect date set on import of Eudora mail (sent messages only)

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect, P2)

Description

[Thomas Swan]

The import utility incorrectly sets the date on sent mail from both normal
folders and sent mail.

This occurs in both the out.mbx and any folder containing a message "moved" from
the out.mbx into itself (mailboxes containg both sent and received messages.)

Problem:

Eudora does not set a Date: header on sent messages, but instead relies upon the

From ???@??? Thu Oct 26 09:50:32 2000

separator between messages.

Solution:

Pull the date from the ???@??? line and override that value with the date pulled
from the Date: header if it exists.  It still provides a method for a linear
pass on the import.

Comment 1

[(not reading, please use [e-mail] instead)]

not a movemail bug.

we need an import bug category.

Comment 2

[Thomas Swan]

Attachment: Sample Inbox (Eudora)

Comment 3

[Thomas Swan]

Attachment: Sample Outbox (Eudora)

Comment 4

[Thomas Swan]

Attachment: Another Inbox (Clean)

Comment 5

[Thomas Swan]

Attachment: Another Outbox (Clean)

Comment 6

[Cavin Song]

I've got a fix for #57125, and I see what's going on.

taking this bug from seth.

Comment 7

[Thomas Swan]

Thanks

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

Comment 9

[Thomas Swan]

http://www.jjminer.org/eud2mbox/ might be able to give some help in 
understanding the Eudora mail format...  The scripts there will convert a 
Eudora format mailbox .mbx with the .toc file to the standard mbox format.  

That in and of itself might be easier to deal with...

Comment 10

[Ninoschka Baca]

Marking nsbeta1 because the dates,even if they are in the Sent folder should be
correct.

Comment 11

[Cavin Song]

Attachment: Proposed fix, v1

Parse and store the date info from the Eudora "From " lines and write out this
date header for Eudora msgs which do not contain any "Date:" header.

Comment 12

[Jean-Francois Ducarroz]

Comment on attachment 70999 [details] [diff] [review]
Proposed fix, v1

R=ducarroz

Comment 13

[(not reading, please use [e-mail] instead)]

1)

it's safe to change the caps on these strings, because the other code that uses 
it does strncasecmp(), right?

+static char *eudoraWeekDays[7] = {
+	"Mon",
+	"Tue",
+	"Wed",
+	"Thu",
+	"Fri",
+	"Sat",
+	"Sun"
+};
+
+static char *eudoraMonths[12] = {
+	"Jan",
+	"Feb",
+	"Mar",
+	"Apr",
+	"May",
+	"Jun",
+	"Jul",
+	"Aug",
+	"Sep",
+	"Oct",
+	"Nov",
+	"Dec"
+};
+

2)

is this change to ensure that tymStr is always null terminated?  if so, please 
add a comment to explain that.  if not, what does this change do?

-	char	tymStr[8];
+	char	tymStr[9];
 	PRBool	tym = PR_FALSE;
 	PRBool	remote = PR_FALSE;
 	PRBool	from = PR_FALSE;
@@ -795,6 +823,7 @@
 						tymStr[6] = '0';
 						tymStr[7] = '0';
 					}
+          tymStr[8] = 0;
 				

3)

 
+    // Generate the default date header in case we need it later when we
+    // write out headers. The header looks like "Date: Tue, 5 Feb 2002 
23:05:04"
+    char str[64];
+    sprintf(str, "Date: %s, %2d %s %4d %s", eudoraWeekDays[weekDay-1], day, 
eudoraMonths[month-1], year, tymStr);
+    defaultDate.Assign(str);

the code looks safe, but you should still do snprintf(str, 64, ...) to prevent 
potential buffer over runs.  at the least, switching to snprintf will prevent 
us from having to re-examine this code when we do buffer overrun audits.

Comment 14

[Cavin Song]

> it's safe to change the caps on these strings, because the other code that
> uses it does strncasecmp(), right?
>
Yes, that's correct.

> is this change to ensure that tymStr is always null terminated?  if so,
> please add a comment to explain that.  if not, what does this change do?
>
Yes, tymStr is always null terminated now because it's needed in the 
PR_snprintf() call (easier to use if it's null terminated).

> the code looks safe, but you should still do snprintf(str, 64, ...) to
> prevent potential buffer over runs.
>
OK, it's changed.

Comment 15

[Cavin Song]

Attachment: Incorporated seth's comments.

Comment 16

[(not reading, please use [e-mail] instead)]

Comment on attachment 71372 [details] [diff] [review]
Incorporated seth's comments.

sr=sspitzer

Here are two minor style comments.  No need for a new patch, but please change
before you check in:

+// 64 bytes is plenty to hold the date header.
+#define DATE_STR_LEN 64

+    char date_header_str[DATE_STR_LEN];
+    PR_snprintf(date_header_str, DATE_STR_LEN, "Date: %s, %2d %s %4d %s",
eudoraWeekDays[weekDay-1], day, eudoraMonths[month-1], year, tymStr);
+    defaultDate.Assign(date_header_str);

1) instead of "str", do "date_header_str".  "str" alone doesn't tell future
reads of the code much.
2) #define the constant, instead of having it twice.

Comment 17

[Jean-Francois Ducarroz]

Comment on attachment 71372 [details] [diff] [review]
Incorporated seth's comments.

R=ducarroz

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 71372 [details] [diff] [review]
Incorporated seth's comments.

a=asa (on behalf of drivers) for checkin to 1.0 trunk

Comment 19

[Cavin Song]

Fix checked into trunk.

Comment 20

[David :Bienvenu]

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