386376 - Impossible to implement a content sniffer in JS due to recursive GetService calls (nsIContentSniffer, JavaScript)

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

The IOService loads all content sniffers registered in the category manager the first time it is loaded. If a content sniffer is implemented in JS then initializing it requires the ScriptSecurityManager to load. That, in turn, requires the IOService. This recursion is not allowed and so the content sniffer fails to load properly.

Not sure what to do about this. I had to reimplement my component in C++ :(

Comment 2

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

Attachment: patch

this should fix it, and it doesn't break the content sniffer unit test, but I haven't tested yet whether it fixes the actual bug here.

Comment 3

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

Attachment: test component

copy to components/; starting the second app startup you'll see a security manager init assertion and no "called" dumps

Comment 4

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

Comment on attachment 272406 [details] [diff] [review]
patch

ok, I tested the patch and it works

Comment 5

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

Checking in netwerk/base/src/nsIOService.h;
/cvsroot/mozilla/netwerk/base/src/nsIOService.h,v  <--  nsIOService.h
new revision: 1.64; previous revision: 1.63
done
Checking in xpcom/glue/nsCategoryCache.h;
/cvsroot/mozilla/xpcom/glue/nsCategoryCache.h,v  <--  nsCategoryCache.h
new revision: 1.2; previous revision: 1.1
done

Comment 6

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

Would be good to find a way to test this...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 272406 [details] [diff] [review]
patch

This is a really safe fix, and this bug is causing branch extension developers a good bit of grief.  Ben and I both think this should be safe to take on branch, and I'd ask us to think about doing just that...

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 272406 [details] [diff] [review]
patch

approved for 1.8.1.13, a=dveditz for release-drivers

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

(In reply to comment #6)
> Would be good to find a way to test this...
> 

Did anyone find a way to test this? This is hard to verify otherwise.

Comment 11

[Boris Zbarsky [:bzbarsky]]

For what it's worth, the extension developer who was having issues due to this has confirmed that his problem is fixed on branch.  Is that good enough to verify?

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Yes, that's good enough for us. Marking it...