Bug 4166 – Function object does not support caller property

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Jeff Watkins]

the following code does not work correctly because the function object either
doesn't have a caller property or the value is always null:

/** Output the string along with the name of the calling function to the
    trace DIV.
 **/
function trace( string )
{
    var callingFn= trace.caller;
    var callingFnText;
    var callingFnName;
    var paren;
    var traceElement= document.getElementById( "trace" );

    if (callingFn)
    {
        callingFnText= callingFn.toString();
        paren= callingFnText.indexOf( "(" );
        callingFnName= callingFnText.substring( 9, paren );
    }
    else
    {
        callingFnName= "global";
    }

    var text= callingFnName + ": " + string;
    traceElement.appendChild( document.createTextNode( text ) );
}

[Mark Rowe (bdash)]

Confirmed with ToT.

[Mark Rowe (bdash)]

Created attachment 3253 [details]
Test case

[Arthur Langereis]

I have an implementation of this at home. I'll update my source and check if it still works and make a 
patch soon.

BTW, here's another testcase:
http://www.xfinitegames.com/safari/callstack.html

[Gavin Kistner]

FWIW, the caller property is not part of the official ECMAScript-262 spec. There is a 'callee' property of 
arguments (section 10.1.8), but no caller property. This is, however, a very useful property that should be 
supported for compatibility with other browsers that support it.

[Jesse Costello-Good]

I am evaluating Safari for a port of TIBCO(R) General Interface, a mature AJAX platform that is currently IE 
only. Fixing this bug would make the port more feasible. 

[Arthur Langereis]

Created attachment 7359 [details]
proposed patch + test

This is a proposed patch to support the caller property in JSC. The caller property is set to either the calling function or jsNull if the function was called from the root context, mirroring Mozilla's behavior wrt this property.

The test checks for both cases. Also, another test of mine that builds a callstack works with this patch:
http://www.xfinitegames.com/safari/callstack.html

[Geoffrey Garen]

Comment on attachment 7359 [details]
proposed patch + test

Arthur,

This is the first patch I've seen you author for the WebKit OpenSource project. Welcome!

Allow me to further welcome you with an r-. :)

Issues that require an r-:

1. ChangeLog. Each patch requires one. The prepare-ChangeLog script will do some of the work for you. Then you need to fill in an explanation of why you changed what you changed.

2. Copyright. To keep things legal, you need to add your name to the copyright notice of any file you changed. You can find examples of this scattered throughout our source code.

3. Modifying the caller property. The test doesn't verify that the property cannot be deleted. Also, should the property be read-only? Since Mozilla invented this, what does Firefox do?

4. The code adds the caller property as a permanent fixture to the function object's property map, but the Mozilla doc says, "The caller property is available only within the body of a function. If used outside a function declaration, the caller property is null." So I think the code is wrong in a case like this:

foo();
alert(foo.caller) // should be "null"

At the very least you would have to delete the property after executing the function. However,

5. Adding and deleting an extra property during every function call may have a negative effect on performance. 

Since caller is a specially funny property, a better approach might be to provide a special funny accessor in FunctionImp::getOwnPropertySlot. That would avoid the twp property map hits. The argumentsGetter and lengthGetter methods are examples. In callerGetter, you could get at the calling context through exec->context()->callingContext(). You could recurse up through calling contexts to (1) find the caller function and (2) determine if the current function were executing (since you would need to return jsNull if it weren't.)

Other issues:

1. NULL. Our preferred style is to use 0 for NULL in c/c++ code, and nil for NULL in objc code.

2. if (curImp). Can a Context ever have a NULL imp? I don't think so. No other code checks for such a thing.

[Geoffrey Garen]

Actually, it looks like argumentsGetter is exactly the code we want for callerGetter. Copy + paste = done.

[Vladimir Olexa (vladinecko)]

Created attachment 11056 [details]
Patch

[Geoffrey Garen]

Comment on attachment 11056 [details]
Patch

A few minor quibbles, then I think this is ready to land:

- Our style guidelines call for "* " and "& " rather than " *" and " &".

- Your test case is very confusting. If it started failing, it would take me a while to figure out why. Two suggestions: (1) choose 'inner' and 'outer' or 'parent' and 'child' -- don't use both; (2) Make your shouldBe explain what they test more clearly. For example, shouldBe('innerHasCaller', 'false'), shouldBe('innerHasCallerWhenCalledFromGlobalCode', 'false'), shouldBe('innerHasCallerWhenCalledFromFunction'), 'true').

[Vladimir Olexa (vladinecko)]

Created attachment 11059 [details]
Patch Revised

- removed {} from one-statement if
- moved *'s and &'s
- made the test case more understandable

[Dave Hyatt]

Just a heads up that this needs very close security scrutiny.  Mozilla had a number of security issues surrounding the caller property.  This property is really dangerous.

[Edward Rudd]

I will put this to some tests tomorrow, with the web app I am developing that makes use of the caller property.  Is there a list of the security issues that mozilla has had w/ respect to this property so they can be reviewed and checked against this code??

[Vladimir Olexa (vladinecko)]

(In reply to comment #12)
> Just a heads up that this needs very close security scrutiny.  Mozilla had a
> number of security issues surrounding the caller property.  This property is
> really dangerous.
> 

I think you're referring to the __caller__ property they removed for security reasons since it returned the activation object of the caller (and thus allowed for stack reconstruction). They have a note about it in the specs for the caller property on a function. They also had arguments.caller which became obsolete with this function.caller property. I couldn't find any security issue that would relate to function.caller specifically, but maybe I'm overlooking something. 

[Geoffrey Garen]

From my reading of the Bugzilla bug [https://bugzilla.mozilla.org/show_bug.cgi?id=65683] and the Mozilla documentation [http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Global_Objects:Function:caller], the issue was with __caller__, not caller. Also, it pertained to "trusted" JS in the Firefox XUL UI calling "untrusted" remote JS in a webpage, which we never do.

[Geoffrey Garen]

Comment on attachment 11059 [details]
Patch Revised

Some of the *'s and >'s are still wrong. 

Also, 'childHasCallerWhenCalledFromGlobalCode' is incorrect -- it's really 'childHasCallerWhenExecutingGlobalCode'.

Anyway, I think we can land this.

[Edward Rudd]

Sweet.
Running SVN 17068 with attachment 11059 [details] , My framework now correctly works in safari (aside from a few rendering issues not related to this bug)

So this patch works in a real world app using this functionality.

[Maciej Stachowiak]

Moving to webkit-unassigned to make clear anyone can land this - it's not Geoff's patch.

[Alexey Proskuryakov]

Committed revision 17483 (moved the stars and renamed childHasCallerWhenCalledFromGlobalCode).

[Jesse Costello-Good]

Thanks to the Safari team for being so responsive regarding requests for better advanced AJAX support. Safari is quickly becoming a 1st class AJAX platform.