Changeset 111162 in webkit for trunk/Source/JavaScriptCore/API


Ignore:
Timestamp:
Mar 19, 2012, 1:12:05 AM (13 years ago)
Author:
[email protected]
Message:

JSCallbackFunction::toStringCallback/valueOfCallback do not handle 0 return value from convertToType
​https://bugs.webkit.org/show_bug.cgi?id=81468 <rdar://problem/11034745>

Reviewed by Oliver Hunt.

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 JSObject::defaultValue function.

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.

  • API/JSCallbackFunction.cpp:

(JSC::JSCallbackFunction::call):

  • Should be null checking the return value.

(JSC):

  • Remove toStringCallback/valueOfCallback.
  • API/JSCallbackFunction.h:

(JSCallbackFunction):

  • Remove toStringCallback/valueOfCallback.
  • API/JSCallbackObject.h:

(JSCallbackObject):

  • Add defaultValue mthods to JSCallbackObject.
  • API/JSCallbackObjectFunctions.h:

(JSC::::defaultValue):

  • Add defaultValue mthods to JSCallbackObject.
  • API/JSClassRef.cpp:

(OpaqueJSClass::prototype):

  • Remove toStringCallback/valueOfCallback.
  • API/tests/testapi.js:
    • Revert this test, now we no longer artificially introduce a toString method onto the api object.
Location:
trunk/Source/JavaScriptCore/API
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/API/JSCallbackFunction.cpp

    r103083 r111162  
    3131#include "CodeBlock.h"
    3232#include "ExceptionHelpers.h"
    33 #include "JSCallbackObject.h"
    3433#include "JSFunction.h"
    3534#include "FunctionPrototype.h"
    … …  
    7776        throwError(exec, toJS(exec, exception));
    7877
     78    // result must be a valid JSValue.
     79    if (!result)
     80        return throwVMTypeError(exec);
     81
    7982    return JSValue::encode(toJS(exec, result));
    8083}
    … …  
    8689}
    8790
    88 JSValueRef JSCallbackFunction::toStringCallback(JSContextRef ctx, JSObjectRef, JSObjectRef thisObject, size_t, const JSValueRef[], JSValueRef* exception)
    89 {
    90     JSObject* object = toJS(thisObject);
    91     if (object->inherits(&JSCallbackObject<JSNonFinalObject>::s_info)) {
    92         for (JSClassRef jsClass = jsCast<JSCallbackObject<JSNonFinalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
    93             if (jsClass->convertToType)
    94                 return jsClass->convertToType(ctx, thisObject, kJSTypeString, exception);
    95     } else if (object->inherits(&JSCallbackObject<JSGlobalObject>::s_info)) {
    96         for (JSClassRef jsClass = jsCast<JSCallbackObject<JSGlobalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
    97             if (jsClass->convertToType)
    98                 return jsClass->convertToType(ctx, thisObject, kJSTypeString, exception);
    99     }
    100     return 0;
    101 }
    102 
    103 JSValueRef JSCallbackFunction::valueOfCallback(JSContextRef ctx, JSObjectRef, JSObjectRef thisObject, size_t, const JSValueRef[], JSValueRef* exception)
    104 {
    105     JSObject* object = toJS(thisObject);
    106     if (object->inherits(&JSCallbackObject<JSNonFinalObject>::s_info)) {
    107         for (JSClassRef jsClass = jsCast<JSCallbackObject<JSNonFinalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
    108             if (jsClass->convertToType)
    109                 return jsClass->convertToType(ctx, thisObject, kJSTypeNumber, exception);
    110     } else if (object->inherits(&JSCallbackObject<JSGlobalObject>::s_info)) {
    111         for (JSClassRef jsClass = jsCast<JSCallbackObject<JSGlobalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
    112             if (jsClass->convertToType)
    113                 return jsClass->convertToType(ctx, thisObject, kJSTypeNumber, exception);
    114     }
    115     return 0;
    116 }
    117 
    118 
    11991} // namespace JSC

  • trunk/Source/JavaScriptCore/API/JSCallbackFunction.h

    r103243 r111162  
    5656    }
    5757
    58     static JSValueRef toStringCallback(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception);
    59     static JSValueRef valueOfCallback(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception);
    60 
    6158private:
    6259    static CallType getCallData(JSCell*, CallData&);

  • trunk/Source/JavaScriptCore/API/JSCallbackObject.h

    r103243 r111162  
    176176    static void destroy(JSCell*);
    177177
     178    static JSValue defaultValue(const JSObject*, ExecState*, PreferredPrimitiveType);
     179
    178180    static bool getOwnPropertySlot(JSCell*, ExecState*, const Identifier&, PropertySlot&);
    179181    static bool getOwnPropertyDescriptor(JSObject*, ExecState*, const Identifier&, PropertyDescriptor&);

  • trunk/Source/JavaScriptCore/API/JSCallbackObjectFunctions.h

    r102065 r111162  
    185185
    186186template <class Parent>
     187JSValue JSCallbackObject<Parent>::defaultValue(const JSObject* object, ExecState* exec, PreferredPrimitiveType hint)
     188{
     189    const JSCallbackObject* thisObject = jsCast<const JSCallbackObject*>(object);
     190    JSContextRef ctx = toRef(exec);
     191    JSObjectRef thisRef = toRef(thisObject);
     192    ::JSType jsHint = hint == PreferString ? kJSTypeString : kJSTypeNumber;
     193
     194    for (JSClassRef jsClass = thisObject->classRef(); jsClass; jsClass = jsClass->parentClass) {
     195        if (JSObjectConvertToTypeCallback convertToType = jsClass->convertToType) {
     196            JSValueRef exception = 0;
     197            JSValueRef result = convertToType(ctx, thisRef, jsHint, &exception);
     198            if (exception) {
     199                throwError(exec, toJS(exec, exception));
     200                return jsUndefined();
     201            }
     202            if (result)
     203                return toJS(exec, result);
     204        }
     205    }
     206   
     207    return Parent::defaultValue(object, exec, hint);
     208}
     209
     210template <class Parent>
    187211bool JSCallbackObject<Parent>::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor)
    188212{

  • trunk/Source/JavaScriptCore/API/JSClassRef.cpp

    r108010 r111162  
    205205     */
    206206
    207     if (convertToType) {
    208         if (!prototypeClass)
    209             prototypeClass = OpaqueJSClass::create(&kJSClassDefinitionEmpty).leakRef();
    210         if (!prototypeClass->m_staticFunctions)
    211             prototypeClass->m_staticFunctions = adoptPtr(new OpaqueJSClassStaticFunctionsTable);
    212         const Identifier& toString = exec->propertyNames().toString;
    213         const Identifier& valueOf = exec->propertyNames().valueOf;
    214         prototypeClass->m_staticFunctions->add(StringImpl::create(toString.characters(), toString.length()), adoptPtr(new StaticFunctionEntry(&JSCallbackFunction::toStringCallback, 0)));
    215         prototypeClass->m_staticFunctions->add(StringImpl::create(valueOf.characters(), valueOf.length()), adoptPtr(new StaticFunctionEntry(&JSCallbackFunction::valueOfCallback, 0)));
    216     }
    217 
    218207    if (!prototypeClass)
    219208        return 0;

  • trunk/Source/JavaScriptCore/API/tests/testapi.c

    r93017 r111162  
    312312}
    313313
     314static JSValueRef MyObject_convertToTypeWrapper(JSContextRef context, JSObjectRef object, JSType type, JSValueRef* exception)
     315{
     316    UNUSED_PARAM(context);
     317    UNUSED_PARAM(object);
     318    UNUSED_PARAM(type);
     319    UNUSED_PARAM(exception);
     320    // Forward to default object class
     321    return 0;
     322}
     323
    314324static bool MyObject_set_nullGetForwardSet(JSContextRef ctx, JSObjectRef object, JSStringRef propertyName, JSValueRef value, JSValueRef* exception)
    315325{
    … …  
    356366};
    357367
     368JSClassDefinition MyObject_convertToTypeWrapperDefinition = {
     369    0,
     370    kJSClassAttributeNone,
     371   
     372    "MyObject",
     373    NULL,
     374   
     375    NULL,
     376    NULL,
     377   
     378    NULL,
     379    NULL,
     380    NULL,
     381    NULL,
     382    NULL,
     383    NULL,
     384    NULL,
     385    NULL,
     386    NULL,
     387    NULL,
     388    MyObject_convertToTypeWrapper,
     389};
     390
     391JSClassDefinition MyObject_nullWrapperDefinition = {
     392    0,
     393    kJSClassAttributeNone,
     394   
     395    "MyObject",
     396    NULL,
     397   
     398    NULL,
     399    NULL,
     400   
     401    NULL,
     402    NULL,
     403    NULL,
     404    NULL,
     405    NULL,
     406    NULL,
     407    NULL,
     408    NULL,
     409    NULL,
     410    NULL,
     411    NULL,
     412};
     413
    358414static JSClassRef MyObject_class(JSContextRef context)
    359415{
    … …  
    361417
    362418    static JSClassRef jsClass;
    363     if (!jsClass)
    364         jsClass = JSClassCreate(&MyObject_definition);
    365    
     419    if (!jsClass) {
     420        JSClassRef baseClass = JSClassCreate(&MyObject_definition);
     421        MyObject_convertToTypeWrapperDefinition.parentClass = baseClass;
     422        JSClassRef wrapperClass = JSClassCreate(&MyObject_convertToTypeWrapperDefinition);
     423        MyObject_nullWrapperDefinition.parentClass = wrapperClass;
     424        jsClass = JSClassCreate(&MyObject_nullWrapperDefinition);
     425    }
     426
    366427    return jsClass;
    367428}

  • trunk/Source/JavaScriptCore/API/tests/testapi.js

    r96627 r111162  
    156156shouldBe("+MyObject", 1); // toNumber
    157157shouldBe("(Object.prototype.toString.call(MyObject))", "[object MyObject]"); // Object.prototype.toString
    158 shouldBe("(MyObject.toString())", "MyObjectAsString"); // toString
     158shouldBe("(MyObject.toString())", "[object MyObject]"); // toString
    159159shouldBe("String(MyObject)", "MyObjectAsString"); // toString
    160160shouldBe("MyObject - 0", 1); // toNumber
Note: See TracChangeset for help on using the changeset viewer.