502959 - Permission denied for <http://sendsome.org> to create wrapper for object of class UnnamedClass

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[John J. Barton]

The original reports are from Firebug users:
http://code.google.com/p/fbug/issues/detail?id=1971
http://code.google.com/p/fbug/issues/detail?id=1948

The case I looked into uses Firebug 1.5a10:
http://getfirebug.com/releases/firebug/1.5X/firebug-1.5X.0a10.xpi
and 
http://sendsome.org/kunden/XNG/test.html

Comment 1

[John J. Barton]

Some info from Boris on the newsgroup:
John J. Barton wrote:
> Most of the loads on my machine work ok. But some (30%?) fail with the subject message.

The subject message typically means that script running with the permissions of http://sendsome.org attempted to access an object that it's not allowed to access.  Specifically, the "wrapper" in this case is the JS reflection of the underlying C++ object.  Permission to create that was denied.  UnnamedClass means that the C++ object did not indicate what kind of object it is (normal for objects not meant to be exposed to JavaScript) and that the error reporter therefore can't tell you what sort of object it was
.....
John J. Barton wrote:
> Ok, maybe we are getting closer. The call stack points to this code in Firebug:
>         if (spy.onreadystatechange)
>             spy.onreadystatechange.handleEvent(event);

OK, this shouldn't obviously be a problem...  assuming |event| could be exposed to web script to start with.

> Here the property onreadystatechange is the request's onreadystatechange:
>      this.onreadystatechange = this.xhrRequest.onreadystatechange;

Right, which is where your safe JS object wrapper came from.

> But the missing bit here is that most of the time the code works. So the problem has to relate to some other condition, maybe a network error or something.

I'd still really like a testcase I could point my browser at to reproduce this.  At the very least I might be able to tell you what the C++ object involved is.

Comment 2

[John J. Barton]

Bug 501962 is about the same issue I guess

Comment 4

[John J. Barton]

I would like this bug not to be marked duplicate to 501962.

Bug 501962 is about an ajax problem. This bug is about a specific error message with a specific test case that I have already investigated. They may be duplicates but we do not know that.

Comment 5

[John J. Barton]

Any hints Boris? We are getting quite a lot of complaints about this. I can't see anything from the Firebug side that could be related. There is a workaround, but it disables a favorite feature of the very folks who have the problem (ajax traffic tracing, Console > ShowXMLHttpRequests).

Comment 6

[Boris Zbarsky [:bzbarsky]]

John, what are the actual steps to reproduce here?  Install that Firebug version.  Load this site.  Then what?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ok, I enabled the net panel and by reloading a bunch of times managed to reproduce the bug eventually...

So what's happening here is that onHTTPSpyReadyStateChange (in the firebug code) calls XPC_SJOW_Call (presumably for the safe js object wrapper around the content function), which calls XPC_SJOW_CallWrapper (via some js_Invoke stuff), which calls XPC_WN_CallMethod.  The js stack here is:

0 [native frame]
1 [native frame]
2 anonymous([function], [function], [object Event @ 0x1c144fb0 (native @ 0x1c4def70)]) ["XPCSafeJSObjectWrapper.cpp":812]
    i = 3
    args = [function]
    this = [xpconnect wrapped nsIDOMEventListener @ 0x1c480060 (native @ 0x183bbe60)]
3 [native frame]
4 onHTTPSpyReadyStateChange(event = [object Event @ 0x1c144fb0 (native @ 0x1c4def70)], spy = [object Object]) ["chrome://firebug/content/spy.js":509]
    rethrow = undefined
    error = undefined

Now XPC_WN_CallMethod does the usual xpconnectish stuff until we end up doing XPCWrappedNative::FindTearOff (called from XPCCallContext::CanCallNow). The aInterface passed to FindTearOff is nsIDOMEventListener.  FindTearOff lands us in InitTearOff, which does the CanCreateWrapper check.  In that check, aObj is the XPCWrappedJS for the content function, which is NOT a DOM object (I seem to recall mrbkap hitting that sort of issue in other circumstances too).  The subject principal is the one from the safe js object wrapper, so we fail out of CanCreateWrapper with a security error: only system code is allowed to create wrappers for non-DOM objects that are not security-checked components.

So one obvious issue here is that the XPCWrappedJS is not a DOM object.  As I said, that's come up before.  But more interesting to me is the fact that we end up in InitTearOff at all.  Presumably the tearoff got GCed, which is why this is not reliably reproducible.  But the nsIDOMEventListener interface is what got handed out from the onreadystatechange getter.  Did the chrome code somehow end up with the main jsobject instead of the tearoff jsobject there?  If so, was it supposed to?  It's really weird to me that this tearoff can just go away like that....

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK, I can confirm that XPC_SJOW_CallWrapper passes the mFlatJSObject of our XPCWrappedNative as the |obj| argument to CallWithoutStatics.  Presumably that means that the caller is in fact holding on to the mFlatJSObject....

Why do we have these tearoff jsobjects again?  :(

Comment 9

[Boris Zbarsky [:bzbarsky]]

Sure isn't a DOM issue in any case.  Blake or Peter, thoughts?

Comment 10

[John J. Barton]

(In reply to comment #6)
> John, what are the actual steps to reproduce here?  Install that Firebug
> version.  Load this site.  Then what?

For others:

statusbar icon context menu enable all panels
Console panel > minimenu > showxmlhttprequest On
Reload multiple times

Comment 11

[John J. Barton]

Just before the |try| block that fails, I called jsdIDebuggerService.GC():
---
try
    {
        fbs.forceGarbageCollection();
        spy.context.onReadySpy = spy; // maybe the handler will eval(), we want the URL.
        if (spy.onreadystatechange)
            spy.onreadystatechange.handleEvent(event);
    }
-------
I get the subject error message every time I reload.

Comment 12

[John J. Barton]

As I understand it, this seems like serious issue which may affect more than just Firebug.  Requesting blocking.

Comment 13

[John J. Barton]

Just to make sure this is known to mozilla: this problem is the number one problem with Firebug on FF 3.5.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

I can't reproduce this on trunk. I'll try 1.9.1 next.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Blake, I could repro on trunk.... You might want to add the GC() call per comment 11, or ask John to point you to an XPI of firebug with that added.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Can't block on this, but would definitely take and are definitely interested in a fix.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #8)
> OK, I can confirm that XPC_SJOW_CallWrapper passes the mFlatJSObject of our
> XPCWrappedNative as the |obj| argument to CallWithoutStatics.  Presumably that
> means that the caller is in fact holding on to the mFlatJSObject....

So, the tearoff objects are totally transient. As far as I can tell, they are all collected every time we GC. So it doesn't actually matter which JSObject pointer you have.

> Why do we have these tearoff jsobjects again?  :(

To store the offset C++ pointer. Why they have to be marked and swept, I don't know.

So, re-instating the code bug 493074 removed fixes this bug. Since the backout there didn't actually appear to help the perf regression it was trying to fix, I think (and jst agrees) that we should just check it back in.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

(sorry, mid-aired with beltzner)

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: De-backout

Comment 20

[John J. Barton]

After badgering you all, we separately looked into alternatives on the Firebug side and found that we can offer the Firebug ShowXMLHttp request feature without intercepting the onreadystate call. So we don't trigger this problem in our code any more.  Our fix should come out in Firebug 1.4.1, Friday-ish.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 390087 [details] [diff] [review]
De-backout

Looks good.  Please add a test (can use XHR and gc() to reproduce reliably).

Comment 22

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: For checkin

This is ready to go!

Comment 23

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/5c751b13e16a

Comment 24

[Helder "Lthere" Magalhães]

Firebug issue 2501 [1] notes that this wasn't back-ported. Although a Firebug-specific workaround was made, shouldn't this fix make it to the next 3.5.x release? More than Firebug might currently be affected. ;-)

Of course this should make it to 3.6.x as well, if it's not fixed there yet: I'm not sure about when the branching occurred, but I guess it was before this issue was fixed...?

[1] http://code.google.com/p/fbug/issues/detail?id=2501

Comment 25

[Boris Zbarsky [:bzbarsky]]

> I'm not sure about when the branching occurred

A few weeks after this fix was checked in.  You can look that sort of thing up, by the way; the source repository is available and all....

Comment 26

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 391234 [details] [diff] [review]
For checkin

This basically applies as-is (some tweaking to the xpconnect makefile is all).

Comment 27

[Daniel Veditz [:dveditz]]

Comment on attachment 391234 [details] [diff] [review]
For checkin

Approved for 1.9.1.8, a=dveditz for release-drivers

Comment 28

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fd1b1037ff98