413512 - Crash loading www.pricerunner.de [@ imgTools::EncodeScaledImage]

Status: VERIFIED FIXED

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

Description

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012204 Minefield/3.0b3pre ID:2008012204

1. New profile, start firefox
2. Go to http://www.pricerunner.de/pl/229-197511/Receiver-Humax-F2-FOX-T-preisvergleich-shopping

CRASH!

Signature	imgTools::EncodeScaledImage(imgIContainer*, nsACString_internal const&, int, int, nsIInputStream**)
UUID	f7c6808a-c911-11dc-b941-001a4bd46e84
Time	2008-01-22 09:46:31-08:00
Build ID	2008012204
OS	Windows NT
OS Version	5.1.2600 Service Pack 3, v.3264
CPU	x86
CPU Info	AuthenticAMD family 15 model 67 stepping 3
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x0

0  	imgTools::EncodeScaledImage(imgIContainer*, nsACString_internal const&, int, int, nsIInputStream**)  	 mozilla/modules/libpr0n/src/imgTools.cpp:292
1 	nsFaviconService::OptimizeFaviconImage(unsigned char const*, unsigned int, nsACString_internal const&, nsACString_internal&, nsACString_internal&) 	mozilla/toolkit/components/places/src/nsFaviconService.cpp:850
2 	nsFaviconService::SetFaviconData(nsIURI*, unsigned char const*, unsigned int, nsACString_internal const&, __int64) 	mozilla/toolkit/components/places/src/nsFaviconService.cpp:564
3 	FaviconLoadListener::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) 	mozilla/toolkit/components/places/src/nsFaviconService.cpp:965
4 	nsStreamListenerTee::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) 	mozilla/netwerk/base/src/nsStreamListenerTee.cpp:65
5 	nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) 	mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:4412
6 	nsInputStreamPump::OnStateStop() 	mozilla/netwerk/base/src/nsInputStreamPump.cpp:576
7 	nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) 	mozilla/netwerk/base/src/nsInputStreamPump.cpp:401
8 	nsInputStreamReadyEvent::Run() 	mozilla/xpcom/io/nsStreamUtils.cpp:190
9 	nsThread::ProcessNextEvent(int, int*) 	mozilla/xpcom/threads/nsThread.cpp:510
10 	NS_ProcessNextEvent_P(nsIThread*, int) 	nsThreadUtils.cpp:227
11 	nsBaseAppShell::Run() 	mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:154
12 	nsAppStartup::Run() 	mozilla/toolkit/components/startup/src/nsAppStartup.cpp:181
13 	PR_GetEnv 	
14 	NS_internal_main(int, char**) 	mozilla/browser/app/nsBrowserApp.cpp:158
15 	wmain 	mozilla/toolkit/xre/nsWindowsWMain.cpp:55
16 	__tmainCRTStartup 	crtexe.c:594
17 	kernel32.dll@0x17066

Comment 1

[WildcatRay]

For reference on this bug:
http://crash-stats.mozilla.com/report/index/9c752dc0-c915-11dc-b7f9-001a4bd43e5c

Comment 2

[James Schwinabart [:mutantmonkey] (he/him)]

Happens on Linux as well

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008012204 Minefield/3.0b3pre

http://crash-stats.mozilla.com/report/index/8019636b-c921-11dc-b408-001a4bd43ef6

Comment 3

[Justin Dolske [:Dolske]]

Huh, weird. Crash at line 292:

287              nsCOMPtr<gfxIImageFrame> frame;
288              rv = aContainer->GetFrameAt(0, getter_AddRefs(frame));
289              NS_ENSURE_SUCCESS(rv, rv);
290            
291              PRInt32 w,h;
292              frame->GetWidth(&w);
293              frame->GetHeight(&h);

Maybe GetFrameAt() at can sometimes return a null even with rv == NS_SUCCESS? I'll look into this today.

Comment 4

[Mike Schroepfer]

(In reply to comment #3)
> Huh, weird. Crash at line 292:
> 
> 287              nsCOMPtr<gfxIImageFrame> frame;
> 288              rv = aContainer->GetFrameAt(0, getter_AddRefs(frame));
> 289              NS_ENSURE_SUCCESS(rv, rv);
> 290            
> 291              PRInt32 w,h;
> 292              frame->GetWidth(&w);
> 293              frame->GetHeight(&h);
> 
> Maybe GetFrameAt() at can sometimes return a null even with rv == NS_SUCCESS?
> I'll look into this today.
> 

Able to repro today over to dolkse

http://crash-stats.mozilla.com/report/index/80d7ed5c-c92e-11dc-aab8-001a4bd43e5c

Comment 5

[Justin Dolske [:Dolske]]

Attachment: Bad favicon

The favicon here is bad (or, at best, weird). Doesn't display in Firefox 2 either.

OS X's Preview and /usr/bin/file seem to claim there are 6 icons in it. Preview says's it's 0x0, and can only display the 5th icon (16x16). Gimp on Solaris opens it as a 48x48 canvas with a single later holding a 16x16 icon (normall each icon gets a layer in Gimp). ImageMagick doesn't want anything to do with the file at all.

End result is that GetFrameAt() does return a null pointer. Looks like other code in the tree checks for that, so we should too here.

Comment 6

[Justin Dolske [:Dolske]]

Looks like something else weird might also be going on here, I get some strange asserts in my console when loading the page...

Login Manager: domEventListener: got event DOMContentLoaded
Login Manager: Counting logins matching host: http://www.pricerunner.de, formSubmitURL: , httpRealm: null
###!!! ASSERTION: writer returned an error with non-zero writeCount: '(NS_FAILED(rv) ? (*writeCount == 0) : PR_TRUE)', file /Users/dolske/ff/trunk1/mozilla/xpcom/io/nsInputStreamTee.cpp, line 105
###!!! ASSERTION: unexpected progress values: 'progress <= progressMax', file /Users/dolske/ff/trunk1/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line 4477
###!!! ASSERTION: writer returned an error with non-zero writeCount: '(NS_FAILED(rv) ? (*writeCount == 0) : PR_TRUE)', file /Users/dolske/ff/trunk1/mozilla/xpcom/io/nsInputStreamTee.cpp, line 105
###!!! ASSERTION: OnDataAvailable implementation consumed no data: 'Error', file /Users/dolske/ff/trunk1/mozilla/netwerk/base/src/nsInputStreamPump.cpp, line 529
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /Users/dolske/ff/trunk1/mozilla/toolkit/components/places/src/nsFaviconService.cpp, line 851

Comment 7

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

This prevents the crash for me.

I started writing a testcase, but it gets tripped up at the assert [fatal for xpcshell tests] in DecodeImageData()...

NS_ASSERTION(written == length, "decoder didn't eat all of its vegetables");

Looks like the icon decoder only user ("wrote") 12288 of the 17759 bytes in the .ico. I decoded the headers for the ICO, and that offset is in the 4th icon's data, 4 bytes before the beginning offset of the 5th icon. Not sure if that's expected, of if it means there's a deeper bug in the ICO decoder.

Comment 8

[Justin Dolske [:Dolske]]

Fix checked in, leaving this bug open for the moment while I poke at the ICO decoder a little more.

Checking in modules/libpr0n/src/imgTools.cpp;
/cvsroot/mozilla/modules/libpr0n/src/imgTools.cpp,v  <--  imgTools.cpp
new revision: 1.2; previous revision: 1.1
done

Comment 9

[Justin Dolske [:Dolske]]

Ok, favicon just seems to be bogus. We're able to read the header at the beginning of the file, select the 5th icon, jump to it's offset, and read the header there [ProcessInfoHeader() in nsICODecoder.cpp]... However the whole header (40 bytes) is just filled with 0x00, so the |switch(mBIH.bpp){}| to set mNumColors ends up returning NS_ERROR_FAILUURE and the decoding fails.

I'll downgrade the assert I was hitting into a warning and add the test.

Comment 10

[Justin Dolske [:Dolske]]

Testcase checked in, marking fixed.

Comment 11

[Reed Loden [:reed]]

Please remember to set in-testsuite+ when you land a test for a fix.

Comment 12

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012304 Minefield/3.0b3pre ID:2008012304

VERIFIED no crash at http://www.pricerunner.de/pl/229-197511/Receiver-Humax-F2-FOX-T-preisvergleich-shopping anymore

(aside: it's expected for the default favicon to be shown, right?)

Comment 13

[Justin Dolske [:Dolske]]

(In reply to comment #12)

> (aside: it's expected for the default favicon to be shown, right?)

Yes, the favicon isn't a valid image so we just show the browser's default/generic icon.