433711 - Unable to wrap interface for DOM object when interface is not in classinfo (nsHttpChannel::InstallCacheListener init fails when listener is a DOM object and tee is implemented in JS)

Status: NEW

(Core :: Networking: HTTP, defect, P5)

Description

[John J. Barton]

I am copying this from email to make it more visible. This is related to bug 411814, especially see 
https://bugzilla.mozilla.org/show_bug.cgi?id=411814#c66

Jan 'Honza' Odvarko:
I am solving one tough problem (I have found yesterday) with the "@mozilla.org/network/stream-listener-tee;1" JS wrapper, which is used to capture the response bodies.

First of all, let me explain a bit how it works. The original tee-listener component is used to write all response bodies into the cache. The component implements nsIStreamListener (and also nsIStreamListenerTee) and so can be inserted in between a HTTP channel and it’s default listener. See snippet of the code that installs the tee-listener

nsHttpChannel::InstallCacheListener(PRUint32 offset)

{

    nsCOMPtr<nsIStreamListenerTee> tee =

        do_CreateInstance(kStreamListenerTeeCID, &rv);

    if (NS_FAILED(rv)) return rv;

 

    rv = tee->Init(mListener, out);

    if (NS_FAILED(rv)) return rv;

 

    mListener = tee;

    return NS_OK;

}

In firebug-cache I have created a new (javascript) component that implements nsIStreamListenerTee interface and is registered under the same CID and ContractID as the original tee-listener component, so replaces it. It’s actually used only as a wrapper as all method calls are redirected into inner (the original) tee-listener component. So, the chain of nsIStreamListener(s) is as follows:

HTTP channel -> fb-tee-listener -> original-tee-listener - > channel’s listener (created by channel’s owner)

All this works, however… the problem is when the channel is created by nsXMLHttpRequest and so, the channel’s stream listener is the nsXMLHttpRequest object itself.

In such a case the init method call (see the code above) fails as the underlying nsXPCWrappedJSClass class can’t convert the first parameter (the mListner) to JS. Specifically the XPCConvert::NativeInterface2JSObject return false)

There is even a comment in XPCConvert::NativeData2JS

// XXX The OBJ_IS_NOT_GLOBAL here is not really right. In

// fact, this code is depending on the fact that the

// global object will not have been collected, and

// therefore this NativeInterface2JSObject will not end up

// creating a new XPCNativeScriptableShared.

nsCOMPtr<nsIXPConnectJSObjectHolder> holder;

if(!NativeInterface2JSObject(ccx, getter_AddRefs(holder),

                             iface, iid, scope, PR_TRUE,

                             OBJ_IS_NOT_GLOBAL, pErr))

    return JS_FALSE;

Is there anything I am doing wrong? I have no other idea about a workaround than write the fb-tee-lsitener in C++. However, this is not what I would like...

Do you have any tips?

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I have created a test-case see:
http://www.janodvarko.cz/firebug/tests/Issue679.htm

It should be easy to reproduce the problem with FB 1.2.0a27.

If it would make things easier I can provide a simple FF extension that shows the problem too.

Comment 2

[John J. Barton]

Is it possible to handle the nsXMLHttpRequest case with another mechanism as a workaround to make this workaround work?

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

The only workaround that occurs to me is to implement the new tee-listener component in C++. In such a case the conversion (of method parameters) into JS would not be performed and it could work as expected.

Just to note that this approach isn't necessary for XHRs. It's actually easy to get response body from a XHR (there is responseText property in nsIXMLHttpRequest, this is used by e.g. spy.js). This approach is used mainly for requests made by a page during the load time. Unfortunately I don't see any way how to "switch off" this approach just for XHRs...

Comment 4

[Justin Dolske [:Dolske]]

Hmm. It seems like this should just work, since nsXMLHttpRequest implements nsIStreamListener, but there's been some oddness like this before. Sicking?

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ugh, this is tricky. I believe this is due to the fact that in the classinfo for XMLHttpRequest sets the CLASSINFO_INTERFACES_ONLY flag. This is intended so that webpages can't access scriptable interfaces that aren't listed in classinfo. Unfortunately it also means that chrome can't access those interfaces either.

Jst, any suggestions here?

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I would love to have this bug fixed as it would solve big trouble in Firebug. Anyway, I have to remove the tee-listener from Firebug1.2 branch for now as it breaks AJAX requests.

Here is a link to Firebug 1.2.0a27X XPI, if anybody would like to test/reproduce the problem.
 
http://getfirebug.com/releases/firebug/1.2/firebug-1.2.0a27X.xpi

Anyway, I am ready to assist any effort in fixing this.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm not sure this can be fixed without huge architectural changes to XPConnect,
something I'm not sure is ever going to happen.

The reason goes something like this:
The way that XPConnect makes things scriptable is that it sets the properties
(attributes and methods) of used interfaces as properties on the script-object.
So if you were to access the nsIStreamListener interface from javascript we'd
set all the properties on that interface on the scriptobject. However that is
obviously not something you want to happen to a scriptobject which is used by
content javascript.

Comment 8

[John J. Barton]

Does anyone have an alternative way to obtain the source content? This looks like a dead end. Bug 430155 looks promising but not for FF3.0 and even at that it seems like its not a slam dunk. 

Comment 9

[Brendan Eich [:brendan]]

Jonas, is this an incompatible change from past Gecko releases?

/be

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Brendan: No, this is how things have always worked.


So I talked with jst, and came up with a sort of ugly, but possibly workable
solution. However it would definitely not be something we could do for
gecko1.9/firefox3.

There is something called tearoff wrappers. For non-DOM objects you can do
stuff like:

myXPCOMObject.nsIFoo.someFunction(...)

Where nsIFoo is an interface that myXPCOMObject implements. (this is obviously
disabled on DOM objects). We could reuse that tearoff wrapper for this, so that
when we

* Are wrapping an XPCOM object in order to hand it to JS
* The JS code we're about to hand the object to is chrome code
* Have the CLASSINFO_INTERFACES_ONLY set on the XPCOM object

we could forward the tearoff wrapper rather than the usual wrapper to the JS
code.

Similarly, we could do the same thing when

* Script is calling .QueryInterface on an XPCOM object
* The JS code we're executing is chrome code
* Have the CLASSINFO_INTERFACES_ONLY set on the XPCOM object

we could make QueryInterface return a tearoff wrapper rather than the normal
wrapper.

Comment 11

[John J. Barton]

Since Bug 430155 landed do we need this any more?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Yes.  That bug added a nasty hack to work around this one; we'd like to be able to remove that hack.

Comment 13

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258