Closed Bug 39975 Opened 24 years ago Closed 24 years ago

Cannot use Components object from XPCOM JS components

Categories

(Core :: XPConnect, defect, P3)

Product:
Core
Component:
XPConnect
Platform:
All
Windows NT
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: vishy, Assigned: shaver)

Details

(Whiteboard: [nsbeta2-])

Attachments

(1 file)

Untested patch.
5.52 KB, patch
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/4.72 [en] (WinNT; U)
BuildID:    May 20, 2000 tree

This seems to be a too conservative security check somwhere. 

Reproducible: Always
Steps to Reproduce:
1. Install May 21 or later Netscape commercial build. 
2. Start and log in to IM. 
3. From an old AIM client, invite the Netscape buddy to a Chat Room
4. Look at the message in the console. The relevant code which caused this 
exception is in the JS XPCOM component
ns/AIMGlue/src/nsJSAimChatRendezvous.js

This shd be nsbeta2+ as it is critical for IM. 

Actual Results:  Got this exceptionin the log
;ONE
************************************************************
** NOTE: This report will only be printed in DEBUG builds.**
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Access denied to XPConnect service. [nsIAimChatRendezvousCallback
::OnProposalReceived]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  l
ocation: "<unknown>"  data: no]
******

Expected Results:  popped up a window for chatInviteBuddy.xul
We're getting a failure when the scecurity manager attempts to find the principal 
associated with AIM's JS component. The JS function being called is a clone, and 
because of Norris's change of 4/15 or so, the security manager walks the 
function's scope chain looking for a principal, and falls off the end. This 
causes the failure. All principal checks must return a principal - in this case, 
it should be the system principal. In this particular case, JS component and 
cloned function, the principal is not being found. Reassigning to mccabe, who is 
attempting to write a testcase that doesn't involve AIM.
Assignee: mstoltz → mccabe
Ok, I have an answer to 'what's different about this case' - I managed to
reproduce this error by bringing up an instance of the JavaScript version of the
sample component, xpcom/sample/nsSample.js.  (I hacked in some c++ code to the
browser to bring up an instance and call a method of it.)

When I brought up the sample component in the browser, it runs OK.  However, its
methods are defined by assigning them to the prototype of the constructor
function... if I change one of these methods to be defined by assigning to
'this' in the constructor function, then if that method does XPConnect, I get
the 'access denied to XPConnect service' error.

There must be something about assigning functions in this way that leaves us
with a principal-less scope chain.

So... this is still a bug, as we shouldn't have this problem.  But there's an
easy workaround, which is to implement functions on the interface by assigning
them to constructorName.prototype; nsSidebar.js and nsSample.js both provide
examples.
I tried to define the callback function on the prototype instead. Had a weird 
result. The first time attempting to call the callback function I got 
************************************************************
** NOTE: This report will only be printed in DEBUG builds.**
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "JavaScript component does not have a method named: "OnProposalRec
eived" [nsIAimChatRendezvousCallback::OnProposalReceived]"  nsresult: "0x8057003
0 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)"  location: "<unknown>"  data: n
o]
************************************************************
So looks like the object didnt have that property (I cannot explain why)

Later on, when receiving a chat request, the old error reappeared
on callback
************************************************************
** NOTE: This report will only be printed in DEBUG builds.**
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Access denied to XPConnect service. [nsIAimChatRendezvousCallback
::OnProposalReceived]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  l
ocation: "<unknown>"  data: no]
************************************************************

BTW, this was still keeping it at the "HelloWorld" level. I.e. just access the 
Components object, nothing more than that. 
thanks, Vishy
I worked with Vishy to tweak his code a little more, and we got the workaround
working.  He was assigning to constructorName.prototype.fnName in the
constructorName function itself; moving this assignment outside the constructor
seemed to allow the xpconnect security check to find the appropriate principals
and be happy.

Shaver, have you been following this at all?  Were you aware of any problems
setting the scope principals for JS components?  I understand that there was a
bug filed to get them set in the first place; know any details?
Yeah, JS components get the system principal, at
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/loader/mozJSComponentLoader.cpp#891
.

Is the scope link being altered when we assign?  Are the functions being
cloned?  That would cause problems, since the scope chain would change, and we
look for principals at the top of the chain (I think).

Yes, I think the functions are being cloned (hit 'Function has been cloned; get
principals from scope' code at
nsScriptSecurityManager.cpp::GetFunctionObjectPrincipal)

Looks like a corner not addressed by a previous bug, 26725.
- 'Access denied to XPConnect service'

So, why are we doing this wacky check for cloned functions?  I can probably make
it work by making the global object for JS components handle nsIScriptOwner, but
I'm not yet convinced.

Can someone explain the GetFunctionObjectPrincipal logic to me?

Mike,
  I'm not sure either. Look at the first bug referenced in Norris's checkin 
comment for that line in nsScriptSecurityManager. I assume cloned functions are 
not assigned the correct principal without this code - but I don't know why. The 
basic rule of principals is that every script must have one - it should never be 
null.

We're not checking the script's principal, though, we're going through
GetObjectPrincipal's low-level XPConnect goop (aren't there utility functions
for that?) to walk the cloned function object's scope chain.

I guess I'll hack up an object to put at the top of that scope chain, and give
it the system principal.
The wacky check for cloned function objects is required by any "brutal sharing"
in XUL or XBL, or any such general scheme that precompiles one JSScript and
shares it via N>>1 function objects.  Only the first function object (which is
the clone-parent, not a clone) owns the JSFunction that is its private data, and
only that JSFunction owns its JSScript, in which the precompiling context's
principal is referenced.  But we must not use that principal (it's privileged,
or just wrong) if we're sharing the precompiled script across trust boundaries.

We must consult an unshared data structure, in the clone-child case.  Each clone
JSObject is a fine thing to consult, and we already had the get-object-principal
utility routine.  Norris and I got this going to unblock hyatt a while ago.  It
sounds like shaver is about to give component JS the magic ju-ju it needs at the
top of its parent chain, so I'm just writing here to fill in the history.

/be
Shaver's taking this one.
Assignee: mccabe → shaver
(waiting for approval and review for my existing loader patches, so that I can
take a swing at these -- should get into the tree today, god willing, and then
I'll have something for testing tomorrowish)

If nsbeta2 is rejected, I'll go for relnote.
Status: NEW → ASSIGNED
Keywords: nsbeta2
Target Milestone: --- → M16
Mike - How serious is this?  What would be the visibility to the user?  What it 
the risk of the fix?  Thanks!
Whiteboard: [NEED INFO]
This was originally needed for IM, and they now have a workaround (Alex Musil in
PDT).  Therefore putting on nsbeta2- radar.
Whiteboard: [NEED INFO] → [nsbeta2-]
Back on this one.  I'll try to have a patch ready for when the tree opens.

M16 has been out for a while now, these bugs target milestones need to be 
updated.
I haven't tested it yet, but it compiles.

dmose, you wanna give it a spin?

47354 has the patch that I think fixes this.

Can those seeing this problem try the patch there?

Should be fixed by the fix to 47354.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: