[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

oliver at apple.com oliver at apple.com
Wed Apr 7 23:44:56 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 7a9871fdd2cd7ab39f7c9418cdadbfe74fd3310a
Author: oliver at apple.com <oliver at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Tue Nov 17 08:41:10 2009 +0000

    Incorrect use of JavaScriptCore API in DumpRenderTree
    https://bugs.webkit.org/show_bug.cgi?id=31577
    
    Reviewed by Maciej Stachowiak
    
    Return undefined rather than a literal null.
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@51068 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/API/APICast.h b/JavaScriptCore/API/APICast.h
index 3b65e09..4284c44 100644
--- a/JavaScriptCore/API/APICast.h
+++ b/JavaScriptCore/API/APICast.h
@@ -51,16 +51,20 @@ typedef struct OpaqueJSValue* JSObjectRef;
 
 inline JSC::ExecState* toJS(JSContextRef c)
 {
+    ASSERT(c);
     return reinterpret_cast<JSC::ExecState*>(const_cast<OpaqueJSContext*>(c));
 }
 
 inline JSC::ExecState* toJS(JSGlobalContextRef c)
 {
+    ASSERT(c);
     return reinterpret_cast<JSC::ExecState*>(c);
 }
 
-inline JSC::JSValue toJS(JSC::ExecState*, JSValueRef v)
+inline JSC::JSValue toJS(JSC::ExecState* exec, JSValueRef v)
 {
+    ASSERT_UNUSED(exec, exec);
+    ASSERT(v);
 #if USE(JSVALUE32_64)
     JSC::JSCell* jsCell = reinterpret_cast<JSC::JSCell*>(const_cast<OpaqueJSValue*>(v));
     if (!jsCell)
@@ -73,8 +77,10 @@ inline JSC::JSValue toJS(JSC::ExecState*, JSValueRef v)
 #endif
 }
 
-inline JSC::JSValue toJSForGC(JSC::ExecState*, JSValueRef v)
+inline JSC::JSValue toJSForGC(JSC::ExecState* exec, JSValueRef v)
 {
+    ASSERT_UNUSED(exec, exec);
+    ASSERT(v);
 #if USE(JSVALUE32_64)
     JSC::JSCell* jsCell = reinterpret_cast<JSC::JSCell*>(const_cast<OpaqueJSValue*>(v));
     if (!jsCell)
diff --git a/JavaScriptCore/API/JSCallbackObjectFunctions.h b/JavaScriptCore/API/JSCallbackObjectFunctions.h
index 9b726e8..36e07cc 100644
--- a/JavaScriptCore/API/JSCallbackObjectFunctions.h
+++ b/JavaScriptCore/API/JSCallbackObjectFunctions.h
@@ -131,15 +131,15 @@ bool JSCallbackObject<Base>::getOwnPropertySlot(ExecState* exec, const Identifie
                 JSLock::DropAllLocks dropAllLocks(exec);
                 value = getProperty(ctx, thisRef, propertyNameRef.get(), &exception);
             }
-            exec->setException(toJS(exec, exception));
-            if (value) {
-                slot.setValue(toJS(exec, value));
-                return true;
-            }
             if (exception) {
+                exec->setException(toJS(exec, exception));
                 slot.setValue(jsUndefined());
                 return true;
             }
+            if (value) {
+                slot.setValue(toJS(exec, value));
+                return true;
+            }
         }
         
         if (OpaqueJSClassStaticValuesTable* staticValues = jsClass->staticValues(exec)) {
@@ -184,7 +184,8 @@ void JSCallbackObject<Base>::put(ExecState* exec, const Identifier& propertyName
                 JSLock::DropAllLocks dropAllLocks(exec);
                 result = setProperty(ctx, thisRef, propertyNameRef.get(), valueRef, &exception);
             }
-            exec->setException(toJS(exec, exception));
+            if (exception)
+                exec->setException(toJS(exec, exception));
             if (result || exception)
                 return;
         }
@@ -202,7 +203,8 @@ void JSCallbackObject<Base>::put(ExecState* exec, const Identifier& propertyName
                         JSLock::DropAllLocks dropAllLocks(exec);
                         result = setProperty(ctx, thisRef, propertyNameRef.get(), valueRef, &exception);
                     }
-                    exec->setException(toJS(exec, exception));
+                    if (exception)
+                        exec->setException(toJS(exec, exception));
                     if (result || exception)
                         return;
                 } else
@@ -240,7 +242,8 @@ bool JSCallbackObject<Base>::deleteProperty(ExecState* exec, const Identifier& p
                 JSLock::DropAllLocks dropAllLocks(exec);
                 result = deleteProperty(ctx, thisRef, propertyNameRef.get(), &exception);
             }
-            exec->setException(toJS(exec, exception));
+            if (exception)
+                exec->setException(toJS(exec, exception));
             if (result || exception)
                 return true;
         }
@@ -301,7 +304,8 @@ JSObject* JSCallbackObject<Base>::construct(ExecState* exec, JSObject* construct
                 JSLock::DropAllLocks dropAllLocks(exec);
                 result = toJS(callAsConstructor(execRef, constructorRef, argumentCount, arguments.data(), &exception));
             }
-            exec->setException(toJS(exec, exception));
+            if (exception)
+                exec->setException(toJS(exec, exception));
             return result;
         }
     }
@@ -325,7 +329,8 @@ bool JSCallbackObject<Base>::hasInstance(ExecState* exec, JSValue value, JSValue
                 JSLock::DropAllLocks dropAllLocks(exec);
                 result = hasInstance(execRef, thisRef, valueRef, &exception);
             }
-            exec->setException(toJS(exec, exception));
+            if (exception)
+                exec->setException(toJS(exec, exception));
             return result;
         }
     }
@@ -363,7 +368,8 @@ JSValue JSCallbackObject<Base>::call(ExecState* exec, JSObject* functionObject,
                 JSLock::DropAllLocks dropAllLocks(exec);
                 result = toJS(exec, callAsFunction(execRef, functionRef, thisObjRef, argumentCount, arguments.data(), &exception));
             }
-            exec->setException(toJS(exec, exception));
+            if (exception)
+                exec->setException(toJS(exec, exception));
             return result;
         }
     }
@@ -435,7 +441,8 @@ double JSCallbackObject<Base>::toNumber(ExecState* exec) const
             }
 
             double dValue;
-            return toJS(exec, value).getNumber(dValue) ? dValue : NaN;
+            if (value)
+                return toJS(exec, value).getNumber(dValue) ? dValue : NaN;
         }
             
     return Base::toNumber(exec);
@@ -459,7 +466,8 @@ UString JSCallbackObject<Base>::toString(ExecState* exec) const
                 exec->setException(toJS(exec, exception));
                 return "";
             }
-            return toJS(exec, value).getString();
+            if (value)
+                return toJS(exec, value).getString();
         }
             
     return Base::toString(exec);
@@ -507,13 +515,14 @@ JSValue JSCallbackObject<Base>::staticValueGetter(ExecState* exec, const Identif
                         JSLock::DropAllLocks dropAllLocks(exec);
                         value = getProperty(toRef(exec), thisRef, propertyNameRef.get(), &exception);
                     }
-                    exec->setException(toJS(exec, exception));
+                    if (exception) {
+                        exec->setException(toJS(exec, exception));
+                        return jsUndefined();
+                    }
                     if (value)
                         return toJS(exec, value);
-                    if (exception)
-                        return jsUndefined();
                 }
-                    
+
     return throwError(exec, ReferenceError, "Static value property defined with NULL getProperty callback.");
 }
 
@@ -560,11 +569,12 @@ JSValue JSCallbackObject<Base>::callbackGetter(ExecState* exec, const Identifier
                 JSLock::DropAllLocks dropAllLocks(exec);
                 value = getProperty(toRef(exec), thisRef, propertyNameRef.get(), &exception);
             }
-            exec->setException(toJS(exec, exception));
+            if (exception) {
+                exec->setException(toJS(exec, exception));
+                return jsUndefined();
+            }
             if (value)
                 return toJS(exec, value);
-            if (exception)
-                return jsUndefined();
         }
             
     return throwError(exec, ReferenceError, "hasProperty callback returned true for a property that doesn't exist.");
diff --git a/JavaScriptCore/API/tests/testapi.c b/JavaScriptCore/API/tests/testapi.c
index 2c17ecf..e7aba0f 100644
--- a/JavaScriptCore/API/tests/testapi.c
+++ b/JavaScriptCore/API/tests/testapi.c
@@ -166,6 +166,10 @@ static JSValueRef MyObject_getProperty(JSContextRef context, JSObjectRef object,
     if (JSStringIsEqualToUTF8CString(propertyName, "cantFind")) {
         return JSValueMakeUndefined(context);
     }
+    
+    if (JSStringIsEqualToUTF8CString(propertyName, "hasPropertyLie")) {
+        return 0;
+    }
 
     if (JSStringIsEqualToUTF8CString(propertyName, "throwOnGet")) {
         return JSEvaluateScript(context, JSStringCreateWithUTF8CString("throw 'an exception'"), object, JSStringCreateWithUTF8CString("test script"), 1, exception);
@@ -176,7 +180,7 @@ static JSValueRef MyObject_getProperty(JSContextRef context, JSObjectRef object,
         return JSValueMakeNumber(context, 1);
     }
     
-    return NULL;
+    return JSValueMakeNull(context);
 }
 
 static bool MyObject_setProperty(JSContextRef context, JSObjectRef object, JSStringRef propertyName, JSValueRef value, JSValueRef* exception)
@@ -299,7 +303,7 @@ static JSValueRef MyObject_convertToType(JSContextRef context, JSObjectRef objec
     }
 
     // string conversion -- forward to default object class
-    return NULL;
+    return JSValueMakeNull(context);
 }
 
 static JSStaticValue evilStaticValues[] = {
@@ -374,7 +378,7 @@ static JSValueRef EvilExceptionObject_convertToType(JSContextRef context, JSObje
         funcName = JSStringCreateWithUTF8CString("toStringExplicit");
         break;
     default:
-        return NULL;
+        return JSValueMakeNull(context);
         break;
     }
     
@@ -382,7 +386,7 @@ static JSValueRef EvilExceptionObject_convertToType(JSContextRef context, JSObje
     JSStringRelease(funcName);    
     JSObjectRef function = JSValueToObject(context, func, exception);
     if (!function)
-        return NULL;
+        return JSValueMakeNull(context);
     JSValueRef value = JSObjectCallAsFunction(context, function, object, 0, NULL, exception);
     if (!value) {
         JSStringRef errorString = JSStringCreateWithUTF8CString("convertToType failed"); 
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index d63e3ad..d7acc10 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,34 @@
+2009-11-17  Oliver Hunt  <oliver at apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Incorrect use of JavaScriptCore API in DumpRenderTree
+        https://bugs.webkit.org/show_bug.cgi?id=31577
+
+        Add assertions to the 'toJS' functions to catch mistakes like
+        this early.  Restructure existing code which blindly passed potentially
+        null values to toJS when forwarding exceptions so that a null check is
+        performed first.
+
+        * API/APICast.h:
+        (toJS):
+        (toJSForGC):
+        * API/JSCallbackObjectFunctions.h:
+        (JSC::::getOwnPropertySlot):
+        (JSC::::put):
+        (JSC::::deleteProperty):
+        (JSC::::construct):
+        (JSC::::hasInstance):
+        (JSC::::call):
+        (JSC::::toNumber):
+        (JSC::::toString):
+        (JSC::::staticValueGetter):
+        (JSC::::callbackGetter):
+        * API/tests/testapi.c: Fix errors in the API tester.
+        (MyObject_getProperty):
+        (MyObject_convertToType):
+        (EvilExceptionObject_convertToType):
+
 2009-11-16  Zoltan Herczeg  <zherczeg at inf.u-szeged.hu>
 
         Reviewed by Gavin Barraclough.
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index d5cb95c..ad76e98 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,18 @@
+2009-11-17  Oliver Hunt  <oliver at apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Incorrect use of JavaScriptCore API in DumpRenderTree
+        https://bugs.webkit.org/show_bug.cgi?id=31577
+
+        Return undefined rather than a literal null.
+
+        * DumpRenderTree/AccessibilityUIElement.cpp:
+        (setSelectedTextRangeCallback):
+        (incrementCallback):
+        (decrementCallback):
+        (showMenuCallback):
+
 2009-11-16  Chris Fleizach  <cfleizach at apple.com>
 
         Reviewed by Beth Dakin.
diff --git a/WebKitTools/DumpRenderTree/AccessibilityUIElement.cpp b/WebKitTools/DumpRenderTree/AccessibilityUIElement.cpp
index 2573512..a054a8c 100644
--- a/WebKitTools/DumpRenderTree/AccessibilityUIElement.cpp
+++ b/WebKitTools/DumpRenderTree/AccessibilityUIElement.cpp
@@ -272,25 +272,25 @@ static JSValueRef setSelectedTextRangeCallback(JSContextRef context, JSObjectRef
     }
     
     toAXElement(thisObject)->setSelectedTextRange(location, length);
-    return 0;
+    return JSValueMakeUndefined(context);
 }
 
 static JSValueRef incrementCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     toAXElement(thisObject)->increment();
-    return 0;
+    return JSValueMakeUndefined(context);
 }
 
 static JSValueRef decrementCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     toAXElement(thisObject)->decrement();
-    return 0;
+    return JSValueMakeUndefined(context);
 }
 
 static JSValueRef showMenuCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     toAXElement(thisObject)->showMenu();
-    return 0;
+    return JSValueMakeUndefined(context);
 }
 
 // Static Value Getters

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list