Closed Bug 380408 Opened 17 years ago Closed 17 years ago

Mail Import Crashes with Divide by Zero

Categories

(Thunderbird :: Migration, defect)

x86
Windows NT
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dh4ca, Assigned: gwenger)

Details

(Whiteboard: fixed-penelope)

Attachments

(3 files, 1 obsolete file)

User-Agent:       Opera/9.10 (Windows NT 4.0; U; en)
Build Identifier: 2.0.0.0 (20070326)

With the sample Eudora inbox, the mail import function crashes with a Divide by Zero error.  Dr. Watson for Windows NT reports it as "Exception: divide by zero (0xc0000094), Address: 0x00976479".  The problem is caused by a malformed Content-Type header which has been merged with a Message-ID header on a single line.  Adding a Newline in front of the Message-Id header resolves the problem.

Reproducible: Always

Steps to Reproduce:
1.Import the sample Eudora inbox.
2.
3.



I can't tell if the malformed headers were mangled by Eudora or by the originating mail client/server but I eventually found a total of five messages in my Eudora mailboxes which were all mangled in the same way.  Although the workaround is relatively easy, it was a painful introduction to Thunderbird and not one that you should inflict on many more Eudora refugees (or other newbies if it's not a Eudora specific issue).
Version: unspecified → 2.0
Not sure if this is fixed or not.
Reassigned bug to me. I'll take a look at it. In general I would say feel free to assign any Eudora importer bugs to me.
Assignee: mscott → gwenger
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Added maximum length sanity check for charset parameter of Content-Type header when importing.
Attachment #274822 - Flags: superreview?
Attachment #274822 - Flags: review?
Comment on attachment 274822 [details] [diff] [review]
[checked in] Patch that fixes bug. Note patch was not created with CVS.

Added maximum length sanity check for charset parameter of Content-Type header when importing.
Attachment #274822 - Flags: superreview?(bienvenu)
Attachment #274822 - Flags: superreview?
Attachment #274822 - Flags: review?(bienvenu)
Attachment #274822 - Flags: review?
Whiteboard: fixed-penelope
Geoffrey, where are we crashing? Do you have a stack trace? I'm worried that we're papering over a problem elsewhere in the code - the related code in the diff shouldn't care how long the content type is, so the crash must be elsewhere...
It was crashing in "apply_rfc2047_encoding", which is in comi18n.cpp. The problem was a divide by zero in this line:
PRInt32 maxNumLines = (strlen(src) / charsPerLine) + 1;

charsPerLine was zero when perLineOverhead was just the right (or wrong) value (I think 72).

On the importers end, the long charset value was clearly bad data, so I thought it was justified to fix it there. I do, however, think that apply_rfc2047_encoding should be made more robust by checking and avoiding that possibility too.
Comment on attachment 274822 [details] [diff] [review]
[checked in] Patch that fixes bug. Note patch was not created with CVS.

this makes sense; I'll land it on the trunk, thx. I'll probably remove the part of the comment that refers to a bug # - we don't do that much, since cvsblame usually suffices. I'll also attach a fix that prevents the crash.
Attachment #274822 - Flags: superreview?(bienvenu)
Attachment #274822 - Flags: superreview+
Attachment #274822 - Flags: review?(bienvenu)
Attachment #274822 - Flags: review+
Attached patch fix divide by 0 error (obsolete) — Splinter Review
Attachment #275462 - Flags: superreview?(mscott)
Attachment #274822 - Attachment description: Patch that fixes bug. Note patch was not created with CVS. → [checked in] Patch that fixes bug. Note patch was not created with CVS.
Attachment #275462 - Flags: superreview?(mscott) → superreview+
I just noticed I attached the wrong diff before - this is what was checked in.
Attachment #275462 - Attachment is obsolete: true
marking fixed
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: