Bug 81468 - JSCallbackFunction::toStringCallback/valueOfCallback do not handle 0 return value
Summary: JSCallbackFunction::toStringCallback/valueOfCallback do not handle 0 return v...
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
Keywords: InRadar
Depends on:
Reported: 2012-03-17 23:52 PDT by Gavin Barraclough
Modified: 2012-03-22 12:05 PDT (History)
2 users (show)

See Also:

Fix (10.68 KB, patch)
2012-03-18 00:05 PDT, Gavin Barraclough
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2012-03-17 23:52:44 PDT
The API specifies that convertToType may opt not to handle a conversion:
    "@result The objects's converted value, or NULL if the object was not converted."
In which case, it would propagate first up the JSClass hierarchy, calling its superclass's conversion functions, and failing that call the 

Unfortunately this behaviour was removed in bug#69677/bug#69858, and instead we now rely on the toStringCallback/valueOfCallback function introduced in bug#69156.  Even after a fix in bug#73368, these will return the result from the first convertToType they find, regardless of whether this result is null, and if no convertToType method is found in the api class hierarchy (possible if toStringCallback/valueOfCallback was accessed off the prototype chain), they will also return a null pointer.  This is unsafe.

It would be easy to make the approach based around toStringCallback/valueOfCallback continue to walk the api class hierarchy, but making the fallback to defaultValue would be problematic (since defaultValue calls toStringCallback/valueOfCallback, this would infinitely recurse).  Making the fallback work with toString/valueOf methods attached to api objects is probably not the right thing to do – instead, we should just implement the defaultValue trap for api objects.

In addition, this bug highlights that fact that JSCallbackFunction::call will allow a hard null to be returned from C to JavaScript - this is not okay.  Handle with an exception.

Comment 1 Gavin Barraclough 2012-03-17 23:58:51 PDT
oops, "and failing that call the" JSObject::defaultValue function.
Comment 2 Gavin Barraclough 2012-03-18 00:05:16 PDT
Created attachment 132485 [details]
Comment 3 Oliver Hunt 2012-03-18 11:32:42 PDT
Comment on attachment 132485 [details]

Can you add an additional test to testapi to verify that we don't crash on a convertToType function returning null?
Comment 4 Gavin Barraclough 2012-03-22 12:05:24 PDT
Fixed in r111162