Bug 4166

Confirmed with ToT.

Created attachment 3253 [details] Test case

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

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.

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.

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

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.

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

Created attachment 11056 [details] Patch

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').

Created attachment 11059 [details] Patch Revised - removed {} from one-statement if - moved *'s and &'s - made the test case more understandable

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 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??

(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.

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.

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.

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.

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

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

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.