548982 - Hang (for several minutes) trying to view a specific message

Status: RESOLVED DUPLICATE of bug 115160

(MailNews Core :: MIME, defect)

Description

[juan becerra [:juanb]]

Attachment: mail that causes hang

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.8) Gecko/20100226 Lightning/1.0b1 Shredder/3.0.3pre

Someone noticed that their Thunderbird 3.0.2 would hang if they tried to open this mail containing some css attachment.

Steps.
1. Try to view said mail.

Expected:
I'm able to see the contents of the mail

Actual: Thunderbird hangs.

It happens in at least Mac and Linux

Comment 1

[juan becerra [:juanb]]

Attachment: process sample while hanging

Comment 2

[Daniel Veditz [:dveditz]]

There doesn't seem to be anything special about these CSS attachments. They're big, though, (116871 and 163350 characters respectively) and all on a single line. Looking at the stack they don't appear to be treated as CSS despite the Content-Type: text/css, the code is in the text-to-html converter code.

Hopefully this is just super-inefficient mime converter string code

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

I don't currently see a reason to believe that this is a frequent occurrence for a large number of people, so marking blocking- for now.  If I'm wrong about that, feel free to renominate...

Comment 4

[Wayne Mery (:wsmwk)]

Juan, does this reproduce with version 3.1 or (better yet) a trunk build?

Comment 5

[juan becerra [:juanb]]

It reproduces on this build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.12pre) Gecko/20101003 Lightning/1.0b2 Lanikai/3.1.5pre

I let it beach ball for several minutes, and it eventually opened the message, but for a while the application was not responsive.

Comment 6

[Wayne Mery (:wsmwk)]

wow, that was a long wait.
juan, do you know whether v2 did not have this problem?

mime perf: https://bugzilla.mozilla.org/buglist.cgi?type1-0-0=substring&keywords=perf%20hang&keywords_type=anywords&field0-0-0=short_desc&type0-0-1=substring&field0-0-1=component&type1-0-1=allwordssubstr&resolution=---&classification=Client%20Software&classification=Components&query_format=advanced&value0-0-1=mime&type0-0-0=anywordssubstr&value0-0-0=mime&field1-0-0=short_desc&product=Core&product=MailNews%20Core&field1-0-1=short_desc

css perf: https://bugzilla.mozilla.org/buglist.cgi?type1-0-0=substring&keywords=perf%20hang&keywords_type=anywords&field0-0-0=short_desc&type0-0-1=substring&field0-0-1=component&type1-0-1=allwordssubstr&resolution=---&classification=Client%20Software&classification=Components&query_format=advanced&value0-0-1=css&type0-0-0=anywordssubstr&value0-0-0=css&field1-0-0=short_desc&product=Core&field1-0-1=short_desc

Comment 7

[Wayne Mery (:wsmwk)]

Attachment: windows trunk stacktrace

shows mime also

Comment 8

[David :Bienvenu]

yeah, that code is well-known to be sub-optimal. I tried to optimize it years and years ago, but BenB had a lot of issues with my proposed patch - see bug 115160

Comment 9

[juan becerra [:juanb]]

On Thunderbird 2.0.0.24 the application also stopped responding for a few minutes, but in the end it also displayed the message.

Comment 10

[Wayne Mery (:wsmwk)]

juan, Mine finished after 5-10 minutes.  So maybe you need someone to buy you a new machine. :)

Comment 11

[Ben Bucksch (:BenB)]

I don't see a security bug justifying embargo here. That we're very slow with extreme amounts (100-1000s) of links is known in public bugs since many years. I'd like to remove the "Security-Sensitive Core Bug" status.

That we try to render the text/css is a bug. That's already filed for XML, see bug 548277 comment 17:
> we should only run the recognizer on text/plain attachments and
> not on all of text/*. For example, it should not parse text/x-diff either.
[nor on text/xml nor text/css]

Shall I mark this bug a dup of that?

David Bienvenu, I don't see a strong objection from my side on bug 115160, in fact I said "thanks". I only asked to make sure that you don't change the logic. Given that this is a technical string use issue, this should be possible without problem. If you want to resume that, let's discuss that there.

Comment 12

[David :Bienvenu]

(In reply to comment #11)

> 
> David Bienvenu, I don't see a strong objection from my side on bug 115160, in
> fact I said "thanks". I only asked to make sure that you don't change the
> logic. Given that this is a technical string use issue, this should be possible
> without problem. If you want to resume that, let's discuss that there.

Heh, I seem to remember some probably well-founded concerns expressed over IRC (or perhaps a different bug) but now that we have a unit test infrastructure, we may be able to make progress while maintaining a reasonable level of confidence that we won't introduce regressions. Marking as a dup.