19031 - crash when image is truncated or corrupt

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P3)

Description

[Doug Turner (:dougt)]

if il_image_complete() set the ic->state to IC_BAD, the webpage will crash
while loading.  Here is a stack crawl:

IL_StreamWriteReady(il_container_struct * 0x039193f0) line 864 + 16 bytes
NetReaderImpl::WriteReady(NetReaderImpl * const 0x0391afe0, unsigned int *
0x0012fcfc) line 76 + 12 bytes
ImageConsumer::OnDataAvailable(ImageConsumer * const 0x0391ae50, nsIChannel *
0x039760e0, nsISupports * 0x00000000, nsIInputStream * 0x0388e308, unsigned int
0x00000000, unsigned int 0x0000041d) line 225 + 16 bytes
nsChannelListener::OnDataAvailable(nsChannelListener * const 0x0391a960,
nsIChannel * 0x039760e0, nsISupports * 0x00000000, nsIInputStream * 0x0388e308,
unsigned int 0x00000000, unsigned int 0x0000041d) line 1395
nsChannelListener::OnDataAvailable(nsChannelListener * const 0x039751d0,
nsIChannel * 0x039760e0, nsISupports * 0x00000000, nsIInputStream * 0x0388e308,
unsigned int 0x00000000, unsigned int 0x0000041d) line 1395
nsHTTPResponseListener::OnDataAvailable(nsHTTPResponseListener * const
0x0388e380, nsIChannel * 0x0391d080, nsISupports * 0x039760e0, nsIInputStream *
0x0388e308, unsigned int 0x00000000, unsigned int 0x0000041d) line 175 + 47
bytes
nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x03868e90)
line 417
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x03868e40) line 173 + 12 bytes
PL_HandleEvent(PLEvent * 0x03868e40) line 537 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x014bb3c0) line 498 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x051503ba, unsigned int 0x0000c0c0, unsigned
int 0x00000000, long 0x014bb3c0) line 972 + 9 bytes
USER32! 77e71268()
014bb3c0()


We are trying to dereference ic->imgdec which was deleted by
il_delete_container()

Comment 1

[Suresh Duddi (gone)]

Are we sure it is truncated or corrupt images only. Eli do we have a test case
for this ?

Comment 2

[Doug Turner (:dougt)]

I am collecting more data....

I know that channel passed to ImageConsumer::On*Request must be a HTTP url.
FILE urls work fine.

Comment 3

[Suresh Duddi (gone)]

Attachment: Here is a bad jpeg. If I access it as a file url, I dont see a crash. Let me try http...

Comment 4

[Doug Turner (:dougt)]

I am seeing the server return an error 401.  Then a crash.  ccing gagan.

Comment 5

[Gagan]

401 is auth related. What/which server are you running this on?

Comment 6

[Doug Turner (:dougt)]

ok.  now I am starting to understand this bug.  When an image is download
without an Content-type, our mime logic uses the file's extension.  If one does
not exists, it will set the Content-Type to text/html, which is wrong.

Here is an example:

http://grok/u/dougt/contenttype5/test.htm

There are three buttons on that page, each will set the content type, then
download a gif image.  The first will set the server to download as text/html,
the second as "unknown", and the last as image/gif.

Comment 7

[Suresh Duddi (gone)]

But image lib will resort to figureing out the mimetype based on the image data.

necko is doing the wrong thing. Do you understand why imglib with its logic to
figureout the mimetype core dumps...

Comment 8

[Judson Valeski]

the fact that we're defaulting to text/html is wrong. doing the right thing
happens when AsyncOpen lands (probably at least a week).

Comment 9

[Suresh Duddi (gone)]

So the bug for me now is that if imglib gets a img with text/html mimetype, we
coredump ?

Comment 10

[Doug Turner (:dougt)]

The bug that the image can not handle onStopRequest's notifications durning a
redirect.

Comment 11

[pnunn]

I'm back.
If the mimetype is text/html, the only way the imglib
will be called is if a <img> tag is used.  When I tried
the test page on grok, I got error messages from the scanner,
which is handling the data as text.

Since the crash is in the image lib, the error might be happening
with an internal icon. and/or our ref count is screwy for error
conditions....

-p

Comment 12

[pnunn]

I've got a sneaky suspicion this bug is related to 19394.
Just adding notes for my own enjoyment.
-p

Comment 13

[pnunn]

I'm not sure what this means, but I want to document it here.
This is from a tree pulled 11-23-99.

During testing, I got to a state where it looked like netlib forgot
about gif mime mapping. The browser handled jpg's and png's fine, but
would give the following message on any gif:

The mContentType for the mChannel on these gifs was text/html.

-------------------------------------------------------------
Displayed message:

nsDebug::WarnIfFalse
WARNING: found a null character: '0x000 != unichars(i)', file
e:\grapter\mozilla\htmlparser\src\nsScanner.cpp,
line 285
Do you wish to continue running?

-------------------------------------------------------------
Call stack:

NTDLL! 77f7629c()
nsDebug::WarnIfFalse(const char * 0x01c88050, const char * 0x01c88038, const
char * 0x01c88000, int 285) line 183 + 21 bytes
nsScanner::Append(const char * 0x00d30ab0, unsigned int 2404) line 285 + 42
bytes
nsParser::OnDataAvailable(nsParser * const 0x01ee9114, nsIChannel * 0x01eeada0,
nsISupports * 0x00000000, nsIInputStream * 0x01ee9e28, unsigned int 0, unsigned
int 2404) line 1303
nsDocumentBindInfo::OnDataAvailable(nsDocumentBindInfo * const 0x01eeabb0,
nsIChannel * 0x01eeada0, nsISupports * 0x00000000, nsIInputStream * 0x01ee9e28,
unsigned int 0, unsigned int 2404) line 1405 + 43 bytes
nsChannelListener::OnDataAvailable(nsChannelListener * const 0x01eeae10,
nsIChannel * 0x01eeada0, nsISupports * 0x00000000, nsIInputStream * 0x01ee9e28,
unsigned int 0, unsigned int 2404) line 1558
nsFileChannel::OnDataAvailable(nsFileChannel * const 0x01eeada4, nsIChannel *
0x01ee9bc0, nsISupports * 0x00000000, nsIInputStream * 0x01ee9e28, unsigned int
0, unsigned int 2404) line 471
nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x01eed830)
line 417
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x01eed7e0) line 173 + 12 bytes
PL_HandleEvent(PLEvent * 0x01eed7e0) line 537 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00c4f930) line 498 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x03e60198, unsigned int 49364, unsigned int 0,
long 12908848) line 972 + 9 bytes
USER32! 77e71820()
00c4f930()
-------------------------------------------------------------
I'd appreciate it if any Necko Guys could peruse this.
Any helpful hints?
-pn

Comment 14

[pnunn]

I think we have some apples mixed in with the oranges here.
The crash call stack indicates a problem that is most likely
related to the problem described in 19394.

The mimetype problem is something else.
-pn

Comment 15

[pnunn]

Just an aside note. For an image with simple flat color
fields and text, gif would give you better quality and
good compression. jpeg is good for images with lots of
graduations.
-p

Comment 16

[pnunn]

I can't reproduce the crash.
Perhaps my fix to bug#19394
fixed it.
Are you still seeing this bug??
-pn

Comment 17

[pnunn]

I'm closing. Please reopen if
you continue to see a problem.
-pn

Comment 18

[Eli Goldberg]

Accessing test case by http:// (clicking on link to attachment), I'm not seeing a
crash on the 1999120208 Mac OS or 1999120308 Win32 & Linux commercial builds.

E-mailed dougt to request his $.02 prior to verifying as fixed.

Comment 19

[Doug Turner (:dougt)]

The bug is an interaction between necko and libimage.  If necko issues a
OnStopRequest notification to libimage during a redirect (either 3xx series ,
or 401 http status), libimage will crash.  You probably can reproduce this by
removing the code we have in the http protocol to shield libimage from these
requests.

It looks like gagan and restructured this code since I put the workaround in.
have to talk to him about this bug.

Comment 20

[Warren Harris]

I can't tell from Doug's comments whether this is still a problem or not, but
it sounds like something Rick mentioned today. Cc'ing him.

Comment 21

[Eli Goldberg]

[E-mailed rpotts to be sure he saw warren's comment. Will rubber-stamp as
verified if no protests received in next few days...]

Comment 22

[Eli Goldberg]

rpotts believes that this is fixed. So, per his comments and prior testing,
marking this as verified. Thanks!