428590 - XPConnect shouldn't blame innocent callers when it can't call a method on a JS object

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

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

Bug 428557 highlighted the following scenario:

1. JS creates a listener (from browser.js), passes it to C++ (download manager) for later action.
2. Other JS calls a function, that
3. Calls C++, that
4. Tries to call a method on the listener (from 1) that the object doesn't actually implement.

In this case XPConnect will issue an NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED exception. The problem is that it uses the top frame of the JS stack to get source file and line information. In this case we never actually called an inner method so XPConnect only sees and blames 2 above.

We can't currently figure out where the object (1) was defined or instantiated so XPConnect can't actually provide a useful error in this case. Nevertheless we should fix it to not blame the unwitting caller.

Some ideas for this, post 1.9:

1. In debug builds (or, potentially, with the report_all_js_exceptions pref set in release builds) check to make sure a JS object implements all advertised interfaces the first time it's passed to C++ and throw an exception there if the implementation is incomplete. This would help us catch the error closer to the guilty party than we can currently.

2. When we're about to generate the _NO_FUNCTION_NAMED exception we should suppress the incorrect file and line info if the method called on the next stack frame isn't the same as the method we tried to call.

Comment 1

[Serge Gautherie (:sgautherie)]

(I like the first idea.)

Could we get the second one before the 1.9 release ?

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

(In reply to comment #0)
> 1. In debug builds (or, potentially, with the report_all_js_exceptions pref set
> in release builds) check to make sure a JS object implements all advertised
> interfaces the first time it's passed to C++ and throw an exception there if
> the implementation is incomplete. This would help us catch the error closer to
> the guilty party than we can currently.

"Just implement what you need" is an intentional feature of XPConnect right now.  In a JS2 world that included "like", we might well be tempted to enforce the implementation's completeness through that model, so an interim change could either be a good migration measure, or a way to piss people off twice in consecutive releases. :)

> 2. When we're about to generate the _NO_FUNCTION_NAMED exception we should
> suppress the incorrect file and line info if the method called on the next
> stack frame isn't the same as the method we tried to call.

3. Look up the object's [[Parent]] chain until you find a global with a __LOCATION__ property or something with a principal, and report that file in the error.

Comment 3

[Serge Gautherie (:sgautherie)]

Is it what causes bug 396502 to report
{{
Source File: XPCSafeJSObjectWrapper.cpp
Line: 445
}}
?

Comment 4

[Johnny Stenback (:jst)]

Not wanted, but patches would still be considered.

Comment 5

[polonus]

Also consider the bug I reported as bug # 449081 "Permission denied for <http://www.example.com> to call method XULControllers.getControllerForCommand in XPCSafeJSObjectWrapper.cpp" the XPCWrappedJS method call pushes
> null prinicipals disallowing a lot of things (also see bug# 352791) - also Permissions denied for MOZ-safe, etc, What is making the invalid calls, also nsServiceManager.JS is involved IMHO.
For bug # 449081 "Permission denied for <http://www.example.com> to call method XULControllers.getControllerForCommand in XPCSafeJSObjectWrapper.cpp" the Error Console also reports:
Source File: XPCSafeJSObjectWrapper.cpp
Line: 445

polonus

Comment 6

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.