crash [@ EscapeFromSpaceLine(nsIOutputStream*, char*, char const*)]

RESOLVED FIXED in Thunderbird 3.1a1

Status

MailNews Core
Import
--
critical
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: Usul, Assigned: Jeff Beckley)

Tracking

({crash, fixed-seamonkey2.0.3, topcrash})

1.9.1 Branch
Thunderbird 3.1a1
crash, fixed-seamonkey2.0.3, topcrash

Firefox Tracking Flags

(blocking-thunderbird3.0 .1+, thunderbird3.0 .1-fixed)

Details

(Whiteboard: [needs verification post 3.0.1], crash signature, URL)

Attachments

(1 attachment, 1 obsolete attachment)

814 bytes, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
0  	thunderbird.exe  	EscapeFromSpaceLine  	mailnews/base/util/nsMsgUtils.cpp:796
1 	thunderbird.exe 	nsOutlookCompose::CopyComposedMessage 	mailnews/import/outlook/src/nsOutlookCompose.cpp:906
2 	thunderbird.exe 	nsOutlookMail::ImportMailbox 	mailnews/import/outlook/src/nsOutlookMail.cpp:510
3 	thunderbird.exe 	ImportOutlookMailImpl::ImportMailbox 	mailnews/import/outlook/src/nsOutlookImport.cpp:478
4 	thunderbird.exe 	ImportMailThread 	mailnews/import/src/nsImportMail.cpp:916
5 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:426
6 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:122
7 	mozcrt19.dll 	_callthreadstartex 	threadex.c:348
8 	mozcrt19.dll 	_threadstartex 	threadex.c:326
9 	kernel32.dll 	kernel32.dll@0x44910 	
10 	ntdll.dll 	ntdll.dll@0x3e4b5 	
11 	ntdll.dll 	ntdll.dll@0x3e488

Updated

8 years ago
Summary: crash @EscapeFromSpaceLine(nsIOutputStream*, char*, char const*) → crash [@ EscapeFromSpaceLine]

Comment 1

8 years ago
http://hg.mozilla.org/comm-central/annotate/860fb123cbac/mailnews/base/util/nsMsgUtils.cpp#l796
3.0b3 topcrash

comments are all about
importing data from Eudora
Crashed during import of Eudora settings
Keywords: topcrash
Summary: crash [@ EscapeFromSpaceLine] → crash [@ EscapeFromSpaceLine(nsIOutputStream*, char*, char const*)]
#5 crash for 3.0b4, 1.8% of crashes
#3 crash for 3.0, 3.2% of crashes (but it's early)

setting blocking?

a few are emerging with french comments, like this one about outlook ...
"Failed to import a number of emails from Outlook2000, often containing attachments, and organized in many directories"
bp-e90c254f-ff43-430b-b824-c3da42091126
Frame	Module	Signature [Expand]	Source
0	thunderbird.exe	EscapeFromSpaceLine	mailnews/base/util/nsMsgUtils.cpp:801
1	thunderbird.exe	nsOutlookCompose::CopyComposedMessage	mailnews/import/outlook/src/nsOutlookCompose.cpp:890
2	thunderbird.exe	nsOutlookMail::ImportMailbox	mailnews/import/outlook/src/nsOutlookMail.cpp:506
3	thunderbird.exe	ImportOutlookMailImpl::ImportMailbox	mailnews/import/outlook/src/nsOutlookImport.cpp:478
4	thunderbird.exe	ImportMailThread	mailnews/import/src/nsImportMail.cpp:916
5	nspr4.dll	_PR_NativeRunThread	nsprpub/pr/src/threads/combined/pruthr.c:426 

bp-9da9bbba-62b6-44bc-85b5-d8c9f2091127
bp-45abfad1-d480-4195-b826-90f752091123
0	thunderbird.exe	EscapeFromSpaceLine	 mailnews/base/util/nsMsgUtils.cpp:801
1	thunderbird.exe	nsEudoraCompose::CopyComposedMessage	mailnews/import/eudora/src/nsEudoraCompose.cpp:989
2	thunderbird.exe	nsEudoraMailbox::ImportMessage	mailnews/import/eudora/src/nsEudoraMailbox.cpp:578
3	thunderbird.exe	nsEudoraMailbox::ImportMailboxUsingTOC	mailnews/import/eudora/src/nsEudoraMailbox.cpp:423
4	thunderbird.exe	nsEudoraMailbox::ImportMailbox	mailnews/import/eudora/src/nsEudoraMailbox.cpp:271
5	thunderbird.exe	ImportEudoraMailImpl::ImportMailbox	mailnews/import/eudora/src/nsEudoraImport.cpp:598
6	thunderbird.exe	ImportMailThread	mailnews/import/src/nsImportMail.cpp:916
blocking-thunderbird3.0: --- → ?
(Reporter)

Comment 4

8 years ago
beckley anything you can do on this one ?
(Assignee)

Comment 5

8 years ago
Here's where it's crashing:

while ((pChar < end) && (*pChar != '\r') && (*(pChar+1) != '\n'))
  pChar++;

I see a flaw in the code in a particular scenario.  If the passed in buffer ends in a CR, then the check to see if the next character is a LF could go past the end of the buffer and produce an access violation.

I'll take a stab at making a fix.
Assignee: nobody → beckley
Status: NEW → ASSIGNED
(Assignee)

Comment 6

8 years ago
Created attachment 416150 [details] [diff] [review]
Proposed fix

Here's a potential fix. It treats a line that ends in just a CR like a partial line.
Attachment #416150 - Flags: superreview?(bienvenu)
Attachment #416150 - Flags: review?(bienvenu)
(Reporter)

Comment 7

8 years ago
bienvenu ping.

Comment 8

8 years ago
Comment on attachment 416150 [details] [diff] [review]
Proposed fix

Thx for the fix. Can you put spaces around the '+' in pChar+1 - two places?
Attachment #416150 - Flags: superreview?(bienvenu)
Attachment #416150 - Flags: superreview+
Attachment #416150 - Flags: review?(bienvenu)
Attachment #416150 - Flags: review+

Comment 9

8 years ago
I think with the patch we won't escape From lines that aren't CRLF terminated - is there any reason not to treat them as full lines?
(Assignee)

Comment 10

8 years ago
Created attachment 417514 [details] [diff] [review]
Better fix

Here's a new patch.  Note that this change only affects the case where there isn't a CRLF-terminated line at the end of the buffer.

With the current code (and remaining in the patch), if the line is only CR-terminated in the middle of the buffer it will treat that as a line.

If you get to the end of a buffer without a CRLF then you wind up in the the else case of the (pChar < end) check, and that portion does From escaping as well.
Attachment #416150 - Attachment is obsolete: true
Attachment #417514 - Flags: superreview?(bienvenu)
Attachment #417514 - Flags: review?(bienvenu)
blocking-thunderbird3.0: ? → .1+
Whiteboard: [has patch][needs review bienvenu]

Updated

8 years ago
Attachment #417514 - Flags: superreview?(bienvenu)
Attachment #417514 - Flags: superreview+
Attachment #417514 - Flags: review?(bienvenu)
Attachment #417514 - Flags: review+

Comment 11

8 years ago
fix pushed to trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Attachment #417514 - Flags: approval-thunderbird3.0.1?

Comment 12

8 years ago
changeset:   4554:fdddfa53e4f0

we'll let this bake on the trunk a little bit before landing for 3.01

Updated

8 years ago
Whiteboard: [has patch][needs review bienvenu] → [has patch]

Updated

8 years ago
Target Milestone: --- → Thunderbird 3.1a1

Updated

8 years ago
OS: Windows XP → All
Hardware: x86 → All
Attachment #417514 - Flags: approval-thunderbird3.0.1? → approval-thunderbird3.0.1+
Keywords: checkin-needed
Whiteboard: [has patch] → [needs landing on branch]
Checked in to branch: http://hg.mozilla.org/releases/comm-1.9.1/rev/a5a1ae991335
status-thunderbird3.0: --- → .1-fixed
Keywords: checkin-needed
Whiteboard: [needs landing on branch]
Comment on attachment 417514 [details] [diff] [review]
Better fix

>diff --git a/mailnews/base/util/nsMsgUtils.cpp b/mailnews/base/util/nsMsgUtils.cpp
>--- a/mailnews/base/util/nsMsgUtils.cpp
>+++ b/mailnews/base/util/nsMsgUtils.cpp
>@@ -793,17 +793,19 @@ nsresult EscapeFromSpaceLine(nsIOutputSt
> {
>   nsresult rv;
>   char *pChar;
>   PRUint32 written;
> 
>   pChar = start;
>   while (start < end)
>   {
>-    while ((pChar < end) && (*pChar != '\r') && (*(pChar+1) != '\n'))
>+    while ((pChar < end) && (*pChar != '\r') && ((pChar+1) < end) && (*(pChar+1) != '\n'))

Can't (pChar < end) and ((pChar+1) < end) be merged?
amazingly even though we are getting >100 crashes per day with 3.0.0, this is (and was) virtually non-existent on trunk, 3.0pre and 3.0.1pre [1]. so can't verify this is gone without a testcase.  But we should know within ~48 hours after 3.0.1 ships.


[1] http://crash-stats.mozilla.com/query/query?product=Thunderbird&version=Thunderbird%3A3.0.1pre&version=Thunderbird%3A3.0pre&version=Thunderbird%3A3.1a1pre&version=Thunderbird%3A3.2a1pre&date=12%2F15%2F2009&range_value=4&range_unit=weeks&query_search=signature&query_type=exact&query=EscapeFromSpaceLine%28nsIOutputStream*%2C+char*%2C+char+const*%29&build_id=&do_query=1#
Whiteboard: [needs verification post 3.0.1]

Updated

8 years ago
Keywords: fixed-seamonkey2.0.3
Crash Signature: [@ EscapeFromSpaceLine(nsIOutputStream*, char*, char const*)]
You need to log in before you can comment on or make changes to this bug.