274339 - [FIX]###!!! ASSERTION: Give me something to work with: '!aMIMEType.IsEmpty() || !aFileExt.IsEmpty()', file /home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2527

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P3)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

loading invalid js uri gives assertions from exthandler

steps to reproduce:
1. load http://www.mozilla.org (probably not necessary)
2. load javascript:void(document.body.style.MozColumnWidth='200

actual results:
spec=/index
WARNING: malformed url: no scheme, file
/home/chb/mozilla/netwerk/base/src/nsStandardURL.cpp, line 700
###!!! ASSERTION: Give me something to work with: '!aMIMEType.IsEmpty() ||
!aFileExt.IsEmpty()', file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2527
Break: at file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2527
###!!! ASSERTION: Empty aExtension parameter!: '!aExtension.IsEmpty()', file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2829
Break: at file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2829
LoadPlugin()
/home/chb/builds/debug-trunk/modules/plugin/samples/unixprinting/libunixprintplugin.so
returned 8736738
GetMIMEDescription() returned "application/x-print-unix-nsplugin:.pnt:Demo Print
Plugin for Unix/Linux"
LoadPlugin()
/home/chb/builds/debug-trunk/modules/plugin/samples/default/unix/libnullplugin.so
returned 8756fc0
GetMIMEDescription() returned "*:.*:All types"
###!!! ASSERTION: Give me something to work with: '!aMIMEType.IsEmpty() ||
!aFileExt.IsEmpty()', file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2527
Break: at file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2527
###!!! ASSERTION: Empty aExtension parameter!: '!aExtension.IsEmpty()', file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2829
Break: at file
/home/chb/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2829

I can't say for sure whether that spec=/index line happened after loading that
js url or before...

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

> 2. load javascript:void(document.body.style.MozColumnWidth='200

actually it looks like you have to paste that url in the urlbar and hit
shift+enter, to hit the assertion

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: JS Stack

this function is called directly from JS, via this stack.

top of stack:
0 [native frame]
1 getMIMEInfoForType(aMIMEType = null, aExtension = "")
["chrome://communicator/content/contentAreaUtils.js":688]
    this = [object ChromeWindow @ 0x81d2700]
2 getDefaultExtension(aFilename = "index", aURI = [xpconnect wrapped nsIURI @
0x882b6e8], aContentType = null)
["chrome://communicator/content/contentAreaUtils.js":816]

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

OK... so getDefaultExtension tries to ask the mimeservice for a mimeinfo, for
checking whether the url extension is a valid extension for the given type/ext
combination... just in this case, the URL has no extension, and a content type
was not given.

hm... this code access url.fileExtension at line 811 (in my tree at least),
before the mime service call. later on, it accesses fileExtension _again_, in a
try-catch block, with the comment "Now try the extension from the URI"...

http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/contentAreaUtils.js#800
see #800 and #815

this code needs to do better when it has neither an extension nor a type. for
example, it could check for that case and return "" or something.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

-> default owner... don't think I'll fix this anytime soon

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

> later on, it accesses fileExtension _again_, in a try-catch block, with the
> comment "Now try the extension from the URI"...

But it assigns a different object to "url" in between...

Anyway, this patch fixes both seamonkey and browser to not violate the api
here.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 171469 [details] [diff] [review]
Patch

> But it assigns a different object to "url" in between...

oh, oops, missed that

Comment 7

[Boris Zbarsky [:bzbarsky]]

mike, could you give this a 2-sec OK?  This is just putting this code in line
with the API comments for the api it's calling....

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 171469 [details] [diff] [review]
Patch

r=me for the toolkit bits

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed.