215701 - hang while reading tokens for junk mail due to corrupt training.dat

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect)

Description

[Andrew Schultz]

Mozilla hangs while downloading a piece of spam.  debugging shows that it's hung
up in readTokens in nsBayesianFilter.cpp

Comment 1

[Andrew Schultz]

Attachment: stacktrace

0x460a831e in readTokens (stream=0x89d6878, tokenizer=@0x882d4ac) at
nsBayesianFilter.cpp:925
925			newBufferSize *= 2;
(gdb) p tokenCount
$1 = 25484
(gdb) p i
$2 = 17664
(gdb) p size
$3 = 3267347362     <== impressive
(gdb) p newBufferSize
$4 = 0
(gdb) p bufferSize
$5 = 0

Comment 2

[Andrew Schultz]

Attachment: bzip2ed training.dat

a clean profile can successfully download mail.  clean profile with this
training.dat file hangs downloading the mail.

I also tried deleting the specific piece of spam that was causing the hang
before, and the clean profile hung on a different one.

Comment 3

[Andrew Schultz]

a clean profile could download the mail properly.  a clean profile with my
training.dat file hit this bug.

Comment 4

[Andrew Schultz]

looking at the training.dat file, there seem to be a couple sections that are
uncharacteristic.  the first such section looks like a xine configuration file.
 My system was having trouble distinguishing up from down for a while, and so
the file getting corrupted is probably INVALID.  lowering severity.

it would still be nice if Mozilla didn't hang...

Comment 5

[Wayne Mery (:wsmwk)]

hang=critical

blocking bug 262743 and bug 317798 on the probability that these are related. 
bug 317798 also has a test training.dat file.
also related? bug 285927, bug 240788

Comment 6

[Andrew Schultz]

Attachment: don't try to allocate memory that has no chance of being used

bug 240788 helped here, but we still attempt to allocate 2GB of memory on bogus sizes (which would bring things to a stand-still).  Also, if a 2GB token *was* read in and then the next token claimed to be bigger, we'd still hit unchecked overflow.

I'm not sure what mGoodCount and mBadCount have as a purpose in life, but if they're supposed to be related to the number of tokens, then this code needs to update their values on failure.

Comment 7

[David :Bienvenu]

Comment on attachment 239465 [details] [diff] [review]
don't try to allocate memory that has no chance of being used

this looks ok to me, but I haven't tried it out...

Comment 8

[Wayne Mery (:wsmwk)]

OS=ALL?

Comment 9

[Andrew Schultz]

FIXED

Comment 10

[Wayne Mery (:wsmwk)]

Comment on attachment 239465 [details] [diff] [review]
don't try to allocate memory that has no chance of being used

after sufficient baking of course.

Comment 11

[Scott MacGregor]

Comment on attachment 239465 [details] [diff] [review]
don't try to allocate memory that has no chance of being used

I think this has had sufficient bake time :)

Comment 12

[Andrew Schultz]

Scott, can you grant approval for bug 240788 as well?  It'd be easier if this one doesn't land first.

Comment 13

[Scott MacGregor]

(In reply to comment #12)
> Scott, can you grant approval for bug 240788 as well?  It'd be easier if this
> one doesn't land first.
> 

done! Thanks for pointing that bug out.

Comment 14

[Andrew Schultz]

landed on branch

Comment 15

[Carsten Book [:Tomcat]]

Verified for 1.8.1.2 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.2pre) Gecko/20070105 Thunderbird/2.0b1 ID:2007010503 on Fedora FC 6 with testcase training.dat