Closed Bug 111830 Opened 23 years ago Closed 23 years ago

freeze/hang when retrieving malformed response for GET /favicon.ico

Categories

(Core :: Graphics: ImageLib, defect)

x86
All
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla0.9.7

People

(Reporter: spam, Assigned: Biesinger)

References

()

Details

(Keywords: hang)

Attachments

(1 file)

current linux CVS

Steps to reproduce:

-Try load http://webmail.tele2.no in browser
-freeze

No idea about component - sorry. Top shows mozilla-bin at 99% CPU continously.
Stepping through a non-debug in gdb shows an endless loop before even gdb
eventually failed.

Received Trace/breakpoint trap in LWP 18439 while waiting for SIGSTOP.
Using pending wait status for LWP 18439.
Freezes with 2001111908, 2001112308 and current CVS.
Does NOT freeze with the 0.9.6 release.
Severity: normal → major
Confirmed.... In the statusbar the text says 'Resolving....' so it seems to be a
problem early in the process 
Not a linux problem, btw. Can confirm this under win98 with 2001111708.
OS: Linux → All
confirming with build 2001112308 on Linux even when JavaScript is disactivated.
I saved both the html and gif files (via sniffer) onto local filesystem but
can't reproduce lock when loading these files from filesystem or from a local
HTTPD daemon.
It works if I disable the favicon !
with favicon and win2k build 20011124.. i see a hang !

-> Imagelib (hyatt)

win2k stack trace :
nsICODecoder::ProcessData(const char * 0x058b658e, unsigned int 0) line 230 + 24
bytes
nsICODecoder::WriteFrom(nsICODecoder * const 0x04c4c0f8, nsIInputStream *
0x058a1990, unsigned int 14, unsigned int * 0x0012fb50) line 201 + 16 bytes
imgRequest::OnDataAvailable(imgRequest * const 0x0598d9d0, nsIRequest *
0x0598d8c0, nsISupports * 0x00000000, nsIInputStream * 0x058a1990, unsigned int
0, unsigned int 14) line 720 + 47 bytes
ProxyListener::OnDataAvailable(ProxyListener * const 0x0598dbd0, nsIRequest *
0x0598d8c0, nsISupports * 0x00000000, nsIInputStream * 0x058a1990, unsigned int
0, unsigned int 14) line 501
nsStreamListenerTee::OnDataAvailable(nsStreamListenerTee * const 0x04c4c020,
nsIRequest * 0x0598d8c0, nsISupports * 0x00000000, nsIInputStream * 0x0598e688,
unsigned int 0, unsigned int 14) line 56 + 51 bytes
nsHttpChannel::OnDataAvailable(nsHttpChannel * const 0x0598d8c4, nsIRequest *
0x0598e8bc, nsISupports * 0x00000000, nsIInputStream * 0x0598e688, unsigned int
0, unsigned int 14) line 2349 + 57 bytes
nsOnDataAvailableEvent::HandleEvent() line 193 + 70 bytes
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x0593b694) line 80
PL_HandleEvent(PLEvent * 0x0593b694) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00edcba0) line 520 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x000700aa, unsigned int 49392, unsigned int 0,
long 15584160) line 1071 + 9 bytes
USER32! 77e02e98()
USER32! 77e030e0()
USER32! 77e05824()
nsAppShellService::Run(nsAppShellService * const 0x00f0c668) line 303
main1(int 2, char * * 0x003526f0, nsISupports * 0x00000000) line 1304 + 32 bytes
main(int 2, char * * 0x003526f0) line 1630 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87d08()
Assignee: asa → hyatt
Severity: major → critical
Component: Browser-General → ImageLib
Keywords: hang
QA Contact: doronr → tpreston
Summary: freeze and 99%CPU before anything renders → freeze and 99% CPU before anything renders
complete http response to GET http://webmail.tele2.no/favicon.ico
(between '------') [actually this is the response for any 404 resource].

------
HTTP/1.1 200 OK
Server: MDNServer
Connection: close

file not found
------
Summary: freeze and 99% CPU before anything renders → freeze and 99%CPU before anything renders
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1
Need to pull this one back in, IMHO. Needs a little more defence in the icon 
decoder.

Target Milestone: mozilla1.1 → ---
Summary: freeze and 99%CPU before anything renders → freeze/hang when retrieving malformed response for GET /favicon.ico
*** Bug 113338 has been marked as a duplicate of this bug. ***
*** Bug 110357 has been marked as a duplicate of this bug. ***
*** Bug 113579 has been marked as a duplicate of this bug. ***
With debug messages it should be much easier to debug. One should run the
browser with nspr debug output enabled for "imgRequest" module.

E.g. on windows, in a batch file:
set NSPR_LOG_MODULES=imgRequest:5
set NSPR_LOG_FILE=C:\Temp\mozilla_log.txt
"C:\Program Files\mozilla.org\Mozilla\mozilla.exe"

Then attach the log file on this bug. 

It outputs nothing with my builds: (I have one from 2311 and 2911). Today's
build cause data-loss, so I won't test it.

Anybody?
Thanks to Gilles on #mozillazine, I got a log 

[...]
1024[8060dd8]: -1756220885 [this=86dab40] imgRequest::FrameChanged {EXIT}
1024[8060dd8]: -1756220884 [this=86fbdb8] imgRequest::GetURI
1024[8060dd8]: -1756220884 [this=86fbdb8] imgRequest::GetImage
1024[8060dd8]: -1756220883 [this=86dab40] imgRequest::GetImage
1024[8060dd8]: -1756220811 [this=88818c8] imgRequest::OnStartRequest {ENTER}
1024[8060dd8]: -1756220811 [this=88818c8] imgRequest::GetURI
1024[8060dd8]: -1756220811 [this=8839388] imgRequestProxy::OnStartRequest
(name="http://webmail.tele2.no/favicon.ico")
1024[8060dd8]: -1756220811 [this=88818c8] imgRequest::OnStartRequest {EXIT}
1024[8060dd8]: -1756220809 [this=88818c8] imgRequest::OnDataAvailable
(count="14") {ENTER}
1024[8060dd8]: -1756220809 [this=88818c8] imgRequest::OnDataAvailable |First
time through... finding mimetype| {ENTER}
1024[8060dd8]: -1756220809 [this=88818c8] imgRequest::OnDataAvailable |sniffing
of mimetype failed| {ENTER}
1024[8060dd8]: -1756220809 [this=88818c8] imgRequest::OnDataAvailable -- Got
content type from the channel
1024[8060dd8]: -1756220809 [this=88818c8] imgRequest::OnDataAvailable |sniffing
of mimetype failed| {EXIT}
1024[8060dd8]: -1756220809 [this=88818c8] imgRequest::OnDataAvailable (content
type="image/x-icon")
1024[8060dd8]: -1756220808 [this=88818c8] imgRequest::SetImage
1024[8060dd8]: -1756220808 [this=88818c8] imgRequest::OnDataAvailable |First
time through... finding mimetype| {EXIT}


That should help.
actually bug 113579 is a slightly different case, but essentially the same.
the request for http://www.babelfish.com/favicon.ico returns 0 bytes (closes
the socket immediately). 

[This (an empty buffer) actually should never get passed to any decoder by 
imgRequest::OnDataAvailable; that could detect the zero buffer when it 
does inStr->ReadSegments to sniff the first few bytes of the content.]
*** Bug 114011 has been marked as a duplicate of this bug. ***
*** Bug 114384 has been marked as a duplicate of this bug. ***
*** Bug 113314 has been marked as a duplicate of this bug. ***
*** Bug 114518 has been marked as a duplicate of this bug. ***
*** Bug 114618 has been marked as a duplicate of this bug. ***
*** Bug 114835 has been marked as a duplicate of this bug. ***
Bug 114835 dont seems to be a dupe of this one, in fact, the url 
http://webmail.registeredsite.com/en_US/cgi-bin/gx.cgi/AppLogic+moblogin freeze 
mozilla and it dont use any favicon in IE.
Julien, that's the issue -- there /isn't/ a favicon to get. See bug 110357. Your
bug is an exact dupe of that one -- even the server and cgi script look to be
the same. And that one, my bug, has been duped to this one. Check the http logs
in that bug to see why it's a dupe of this one.
*** Bug 114857 has been marked as a duplicate of this bug. ***
*** Bug 114984 has been marked as a duplicate of this bug. ***
*** Bug 115510 has been marked as a duplicate of this bug. ***
Now qualifies for MOSTFREQ, right? (12 dupes.)

 Though in this situation, I don't know who'd have the authority to put it on radar.
Blocks: 114455
Can we get a patch for 0.9.7?  drivers@mozilla.org request it.  If this is an
iloop or a near-infinite loop, can we defend with more sanity-checking (say, of
mNumIcons in nsICODecoder::ProcessData)?

/be 
Keywords: mozilla0.9.7
mNumIcons should be initialized in nsICODecoder::nsICODecoder, btw.  Cc'ing
biesi, who may be able to help quickly for 0.9.7.

/be
bbaetz is helping me review nsICODecoder::ProcessData, and he points out that
the while (mCurrIcon < mNumIcons) { ... } loop implicated by the stack here only
conditionally increments mCurrIcon -- that's bad.  It needs some sanity-checking
or other logic to avoid being an iloop for certain bad inputs.

/be
brendan: I had a look at this, but was unable to reproduce the hang, so fixing
it is probably difficult.

I found out, though, that the ICO Decoder doesn't check if the file is really an
.ico file... I can fix this.
copying url from dup here with which I can reproduce the hang
biesi: can't we at least make sure that the loop will terminate?  What is that
magic 22 buried in the condition guarding the mCurrIcon++?

/be
brendan: I can reproduce the bug now and am working on a fix
Assignee: hyatt → cbiesinger
Status: ASSIGNED → NEW
Target Milestone: --- → mozilla0.9.7
Attached patch FixSplinter Review
brendan: 22 is sizeof(mDirEntryArray) + DIRENTRYOFFSET

This patch also initializes mNumIcons in the constructor as suggested in the
bug.
And it does another small optimization in the decoder.
Comment on attachment 61981 [details] [diff] [review]
Fix

r=pavlov
Attachment #61981 - Flags: review+
sr=hyatt, use 2-space indentation rather than 4 on the return though.
Comment on attachment 61981 [details] [diff] [review]
Fix

a=brendan@mozilla.org for checkin to the 0.9.7 branch.

/be
Attachment #61981 - Flags: superreview+
Attachment #61981 - Flags: approval+
timeless checked this into the trunk
branch is closed atm, so not yet checked in there
timeless also checked this into the branch.

resolving.
Status: NEW → RESOLVED
Closed: 23 years ago
Keywords: fixed0.9.7
Resolution: --- → FIXED
Marking verified. The above url and the url referenced in bug 110357 wfm in
2001121808-trunk.
Status: RESOLVED → VERIFIED
No longer blocks: 114455
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: