Last Comment Bug 386376 - Impossible to implement a content sniffer in JS due to recursive GetService calls (nsIContentSniffer, JavaScript)
: Impossible to implement a content sniffer in JS due to recursive GetService c...
Status: RESOLVED FIXED
: verified1.8.1.13
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Christian :Biesinger (don't email me, ping me on IRC)
:
Mentors:
: 386880 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-29 14:05 PDT by Ben Turner (not reading bugmail, use the needinfo flag!)
Modified: 2008-03-14 20:14 PDT (History)
7 users (show)
cbiesinger: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.93 KB, patch)
2007-07-15 11:31 PDT, Christian :Biesinger (don't email me, ping me on IRC)
benjamin: review+
dveditz: approval1.8.1.13+
Details | Diff | Review
test component (2.53 KB, text/plain)
2007-07-18 12:08 PDT, Christian :Biesinger (don't email me, ping me on IRC)
no flags Details

Description Ben Turner (not reading bugmail, use the needinfo flag!) 2007-06-29 14:05:52 PDT
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 1 Phil Ringnalda (:philor) 2007-07-04 12:43:20 PDT
*** Bug 386880 has been marked as a duplicate of this bug. ***
Comment 2 Christian :Biesinger (don't email me, ping me on IRC) 2007-07-15 11:31:27 PDT
Created attachment 272406 [details] [diff] [review]
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) 2007-07-18 12:08:51 PDT
Created attachment 272837 [details]
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) 2007-07-18 12:46:38 PDT
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) 2007-07-24 16:47:44 PDT
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) 2007-07-24 16:48:04 PDT
Would be good to find a way to test this...
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-03-04 21:50:33 PST
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] 2008-03-05 11:31:28 PST
Comment on attachment 272406 [details] [diff] [review]
patch

approved for 1.8.1.13, a=dveditz for release-drivers
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-03-05 12:38:28 PST
Fixed on branch.
Comment 10 Al Billings [:abillings] 2008-03-14 16:41:20 PDT
(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 [:bz] (Out June 25-July 6) 2008-03-14 19:53:52 PDT
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] 2008-03-14 20:14:58 PDT
Yes, that's good enough for us. Marking it...

Note You need to log in before you can comment on or make changes to this bug.