nsExternalHelperAppService::AddMimeInfoToCache passing null? to nsCStringKey ctor [@ nsCStringKey::nsCStringKey]

VERIFIED FIXED

Status

SeaMonkey
UI Design
VERIFIED FIXED
17 years ago
13 years ago

People

(Reporter: dbaron, Assigned: bz)

Tracking

({crash, qawanted, topcrash})

Trunk
x86
Linux
crash, qawanted, topcrash

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PDT+], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
There were 4 crashes in talkback recently (three submitted by the same person)
with the stack:

nsCStringKey::nsCStringKey()
nsExternalHelperAppService::AddMimeInfoToCache()
nsOSHelperAppService::GetFromMIMEType()
nsExternalHelperAppService::DoContent()
nsDocumentOpenInfo::DispatchContent()
nsDocumentOpenInfo::OnStartRequest()
nsHttpChannel::ProcessNormal()
nsHttpChannel::ProcessResponse()
nsHttpChannel::OnStartRequest()
nsOnStartRequestEvent::HandleEvent()
nsARequestObserverEvent::HandlePLEvent()
PL_HandleEvent()
PL_ProcessPendingEvents()
nsEventQueueImpl::ProcessPendingEvents()
event_processor_callback()
our_gdk_io_invoke()
libglib-1.2.so.0 + 0xec10 (0x40342c10)
libglib-1.2.so.0 + 0x102d9 (0x403442d9)
libglib-1.2.so.0 + 0x108e3 (0x403448e3)
libglib-1.2.so.0 + 0x10a7c (0x40344a7c)
libgtk-1.2.so.0 + 0x8dd97 (0x40266d97)
nsAppShell::Run()
nsAppShellService::Run()
main1()
main()

User comments were:
  mouse 1 click on some *.rpm file
  Mozilla seems to crash every time one tries to open a file with extension .asf
even if it is empty.

Might there need to be some check that you're not passing null to the
nsCStringKey constructor, at some level?

(Not marking as topcrash because I don't think 4 reports make it one, although
with the number of people using talkback with nightlys lately and the stability
of those builds, it actually shows up.)

Comment 1

17 years ago
see bug 97960
*** Bug 97137 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 3

17 years ago
Might it make sense to dupe the other way around?  Oh well...
(Reporter)

Comment 4

17 years ago
Adding topcrash keyword -- this has been pretty consistent (8 reports in the
past 10 days), and is the #3 clearly identifiable topcrash on linux (although if
all the libc.so.6 crashes or libxpcom.so crashes are the same thing, it's not
really #3).
Keywords: crash, topcrash

Comment 5

17 years ago
Adding the signature to the summary and cc'ing those lovely talkback people.
Summary: nsExternalHelperAppService::AddMimeInfoToCache passing null? to nsCStringKey ctor → nsExternalHelperAppService::AddMimeInfoToCache passing null? to nsCStringKey ctor [@ nsCStringKey::nsCStringKey]
OK. Here's what seems to be happening. There are actually 2 crashes here, I
think....

If there is an entry in a mailcap file for a mime type (so there is a helper for
it) but no entry in a mime.types file or no extensions listed in such an entry,
then the Unix mailcap/mime.types code was creating an nsIMIMEInfo with an empty
extension list.  Notice that such a beast is nonsensical on Windows, where there
can be no helper if there is no extension.

Now, this is what happened next. 
nsExternalHelperAppService::AddMimeInfoToCache() called
nsMIMEInfoImpl::GetFileExtensions() which proceded to return NS_OK without
setting the extension count out var to 0 if there were no extensions.  Since
AddMimeInfoToCache() did not initialize |count| before calling
GetFileExtensions(), |count| had some bogus value and we proceeded looping
through an empty array, trying to assign its nonexistent entries into a
nsCStringKey variable.  This crashed.  I could reproduce the crash with a simple
install of realplayer...

Now the second case is an empty mimetype.  I can't think of a good reason this
would ever happen if GetFromMIMEType is being called. However, when it does
AddMimeInfoToCache() does not check the return value of GetMIMEType() and can
once again crash, since GetMIMEType() will not initialize the pointer if the
type string has length 0.

Attaching a patch that should fix both crashes (and should fix bug 69864 as
well).  Reviews?
Assignee: mscott → bzbarsky
Keywords: patch, review
ccing valeski who wrote nsMIMEInfoImpl::GetFileExtensions().

mscott, valeski, what do you think of the patch?

Comment 9

17 years ago
Comment on attachment 49716 [details] [diff] [review]
Proposed patch

sr=mscott

(In the future if you need a sr from me please send me email instead of cc'ing me on the bug per the reviewers guidelines. Thanks!)
Attachment #49716 - Flags: superreview+

Updated

17 years ago
Attachment #49716 - Flags: review+
checked in on trunk.  nominating for branch
Status: NEW → ASSIGNED
Keywords: nsbranch

Comment 11

17 years ago
*** Bug 100232 has been marked as a duplicate of this bug. ***

Comment 12

17 years ago
When this has reviews completed, please mark nsbranch+ and write to
pdt2@netscape.com.  We'd like to get this on the PDT radar for Netscape branch
checkin
Keywords: nsbranch → nsbranch+

Comment 13

17 years ago
Thanks for the fix, can you put it on the branch - PDT+
Whiteboard: [PDT+]
checked in on the branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 15

17 years ago
The latest Talkback data shows this last crashed with build 2001092408 on the
MozillaTrunk, we'll have to wait a little while to verify the fix on the branch. 

Tom: Check in a few days to see what the Talkback data looks like for this crash
on the N620 branch.

Comment 16

17 years ago
note: 97960 does not seem to be related in any way.

unlike the zero-byte mime.types file crashers, this bug looks like it is XP
apps. I've asked boris for clarifcation...

Comment 17

17 years ago
greer: helped verify the other bug w/ crash data, hopefully he will mark this
fixed as well.

No response from bz about this, but after thinking about this last night:

I don't even know how to set up a mime type and NULL the extension. So assuming
I deserve to have this job, it is probably something someone else has been
testing...

so I'm sending this to XP apps, which is actually where most of the dupes for
this bug live.
Component: Networking → XP Apps
QA Contact: benc → sairuh
hrm, i'm not sure how to test/verify this one either. back to benc to see if he
knows of another person who might, and adding qawanted.
Keywords: qawanted
QA Contact: sairuh → benc

Comment 19

17 years ago
I'm finding no incidents on the N620 branch with this signature. Marking 
Verified Fixed per Talkback data and comments from jpatel on 10-02.
Status: RESOLVED → VERIFIED

Comment 20

17 years ago
(sarah: I fixed the problem for you. :) <Thanks greer!>)
Product: Core → Mozilla Application Suite
Crash Signature: [@ nsCStringKey::nsCStringKey]
You need to log in before you can comment on or make changes to this bug.