Source/WebCore: [V8] Crash in npObjectGetProperty() in V8NPObject.cpp
authorjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 May 2012 23:37:39 +0000 (23:37 +0000)
committerjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 May 2012 23:37:39 +0000 (23:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=86131

Reviewed by Adam Barth.

Tests: plugins/npruntime/delete-plugin-within-getProperty.html
       plugins/npruntime/delete-plugin-within-hasProperty-return-false.html
       plugins/npruntime/delete-plugin-within-hasProperty-return-true.html
       plugins/npruntime/delete-plugin-within-invoke.html
       plugins/npruntime/delete-plugin-within-setProperty.html

* bindings/v8/NPV8Object.cpp:
(_NPN_EvaluateHelper):
* bindings/v8/V8NPObject.cpp: Check NPN_IsAlive in a bunch of places we're not currently.
(WebCore::npObjectInvokeImpl):
(WebCore::npObjectGetProperty):
(WebCore::npObjectSetProperty):

Tools: Add end-of-life test cases for https://bugs.webkit.org/show_bug.cgi?id=86131.

Reviewed by Adam Barth.

* DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:
(callDeletePlugin):
(pluginHasProperty):
(pluginHasMethod):
(pluginGetProperty):
(pluginSetProperty):
(pluginInvoke):

LayoutTests: Test for https://bugs.webkit.org/show_bug.cgi?id=86131.

Reviewed by Adam Barth.

* plugins/npruntime/delete-plugin-within-getProperty-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-getProperty.html: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-false.html: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-true.html: Added.
* plugins/npruntime/delete-plugin-within-invoke-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-invoke.html: Added.
* plugins/npruntime/delete-plugin-within-setProperty-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-setProperty.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@117012 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/plugins/npruntime/delete-plugin-within-getProperty-expected.txt [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-getProperty.html [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false.html [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true.html [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-invoke-expected.txt [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-invoke.html [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-setProperty-expected.txt [new file with mode: 0644]
LayoutTests/plugins/npruntime/delete-plugin-within-setProperty.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/bindings/v8/NPV8Object.cpp
Source/WebCore/bindings/v8/V8NPObject.cpp
Tools/ChangeLog
Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp

index bb19e6d..d53384f 100644 (file)
@@ -1,3 +1,20 @@
+2012-05-14  Nate Chapin  <japhet@chromium.org>
+
+        Test for https://bugs.webkit.org/show_bug.cgi?id=86131.
+
+        Reviewed by Adam Barth.
+
+        * plugins/npruntime/delete-plugin-within-getProperty-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-getProperty.html: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-false.html: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-true.html: Added.
+        * plugins/npruntime/delete-plugin-within-invoke-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-invoke.html: Added.
+        * plugins/npruntime/delete-plugin-within-setProperty-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-setProperty.html: Added.
+
 2012-05-14  Dirk Pranke  <dpranke@chromium.org>
 
         Rebaseline http/tests/misc/will-send-request-returns-null-on-redirect on chromium.
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty-expected.txt b/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty-expected.txt
new file mode 100644 (file)
index 0000000..8b13789
--- /dev/null
@@ -0,0 +1 @@
+
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty.html b/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty.html
new file mode 100644 (file)
index 0000000..fbe1dba
--- /dev/null
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    var test = plg.deletePluginInGetProperty;
+</script>
+</body>
+</html>
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt b/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt
new file mode 100644 (file)
index 0000000..f4e3639
--- /dev/null
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 13: Uncaught ReferenceError: NPObject deleted
+
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false.html b/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false.html
new file mode 100644 (file)
index 0000000..9139f23
--- /dev/null
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    plg.deletePluginReturnFalse;
+</script>
+</body>
+</html>
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt b/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt
new file mode 100644 (file)
index 0000000..f4e3639
--- /dev/null
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 13: Uncaught ReferenceError: NPObject deleted
+
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true.html b/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true.html
new file mode 100644 (file)
index 0000000..6d3b825
--- /dev/null
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+    
+    plg.deletePluginReturnTrue;
+</script>
+</body>
+</html>
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-invoke-expected.txt b/LayoutTests/plugins/npruntime/delete-plugin-within-invoke-expected.txt
new file mode 100644 (file)
index 0000000..8b13789
--- /dev/null
@@ -0,0 +1 @@
+
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-invoke.html b/LayoutTests/plugins/npruntime/delete-plugin-within-invoke.html
new file mode 100644 (file)
index 0000000..eeb173b
--- /dev/null
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    plg.testDeleteWithinInvoke();
+</script>
+</body>
+</html>
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty-expected.txt b/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty-expected.txt
new file mode 100644 (file)
index 0000000..f4e3639
--- /dev/null
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 13: Uncaught ReferenceError: NPObject deleted
+
diff --git a/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty.html b/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty.html
new file mode 100644 (file)
index 0000000..85090fa
--- /dev/null
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    plg.deletePluginReturnTrue = true;
+</script>
+</body>
+</html>
index 9d2c4ee..2e6d92d 100644 (file)
@@ -1,3 +1,23 @@
+2012-05-14  Nate Chapin  <japhet@chromium.org>
+
+        [V8] Crash in npObjectGetProperty() in V8NPObject.cpp
+        https://bugs.webkit.org/show_bug.cgi?id=86131
+
+        Reviewed by Adam Barth.
+
+        Tests: plugins/npruntime/delete-plugin-within-getProperty.html
+               plugins/npruntime/delete-plugin-within-hasProperty-return-false.html
+               plugins/npruntime/delete-plugin-within-hasProperty-return-true.html
+               plugins/npruntime/delete-plugin-within-invoke.html
+               plugins/npruntime/delete-plugin-within-setProperty.html
+
+        * bindings/v8/NPV8Object.cpp:
+        (_NPN_EvaluateHelper):
+        * bindings/v8/V8NPObject.cpp: Check NPN_IsAlive in a bunch of places we're not currently.
+        (WebCore::npObjectInvokeImpl):
+        (WebCore::npObjectGetProperty):
+        (WebCore::npObjectSetProperty):
+
 2012-05-14  Brent Fulgham  <bfulgham@webkit.org>
 
         [WinCairo] Unreviewed build correction.
index 5c33593..0d7cff6 100644 (file)
@@ -336,7 +336,8 @@ bool _NPN_EvaluateHelper(NPP npp, bool popupsAllowed, NPObject* npObject, NPStri
     if (v8result.IsEmpty())
         return false;
 
-    convertV8ObjectToNPVariant(v8result, npObject, result);
+    if (_NPN_IsAlive(npObject))
+        convertV8ObjectToNPVariant(v8result, npObject, result);
     return true;
 }
 
index f6dd1f8..0d1c03c 100644 (file)
@@ -139,7 +139,9 @@ static v8::Handle<v8::Value> npObjectInvokeImpl(const v8::Arguments& args, Invok
         _NPN_ReleaseVariantValue(&npArgs[i]);
 
     // Unwrap return values.
-    v8::Handle<v8::Value> returnValue = convertNPVariantToV8Object(&result, npObject);
+    v8::Handle<v8::Value> returnValue;
+    if (_NPN_IsAlive(npObject))
+        returnValue = convertNPVariantToV8Object(&result, npObject);
     _NPN_ReleaseVariantValue(&result);
 
     return returnValue;
@@ -191,21 +193,30 @@ static v8::Handle<v8::Value> npObjectGetProperty(v8::Local<v8::Object> self, NPI
         return throwError("NPObject deleted", V8Proxy::ReferenceError);
 
 
-    if (npObject->_class->hasProperty && npObject->_class->hasProperty(npObject, identifier)
-        && npObject->_class->getProperty) {
+    if (npObject->_class->hasProperty && npObject->_class->getProperty && npObject->_class->hasProperty(npObject, identifier)) {
+        if (!_NPN_IsAlive(npObject))
+            return throwError("NPObject deleted", V8Proxy::ReferenceError);
 
         NPVariant result;
         VOID_TO_NPVARIANT(result);
         if (!npObject->_class->getProperty(npObject, identifier, &result))
             return v8::Handle<v8::Value>();
 
-        v8::Handle<v8::Value> returnValue = convertNPVariantToV8Object(&result, npObject);
+        v8::Handle<v8::Value> returnValue;
+        if (_NPN_IsAlive(npObject))
+            returnValue = convertNPVariantToV8Object(&result, npObject);
         _NPN_ReleaseVariantValue(&result);
         return returnValue;
 
     }
 
+    if (!_NPN_IsAlive(npObject))
+        return throwError("NPObject deleted", V8Proxy::ReferenceError);
+
     if (key->IsString() && npObject->_class->hasMethod && npObject->_class->hasMethod(npObject, identifier)) {
+        if (!_NPN_IsAlive(npObject))
+            return throwError("NPObject deleted", V8Proxy::ReferenceError);
+
         PrivateIdentifier* id = static_cast<PrivateIdentifier*>(identifier);
         v8::Persistent<v8::FunctionTemplate> functionTemplate = staticTemplateMap().get(id);
         // Cache templates using identifier as the key.
@@ -266,8 +277,9 @@ static v8::Handle<v8::Value> npObjectSetProperty(v8::Local<v8::Object> self, NPI
         return value;  // Intercepted, but an exception was thrown.
     }
 
-    if (npObject->_class->hasProperty && npObject->_class->hasProperty(npObject, identifier)
-        && npObject->_class->setProperty) {
+    if (npObject->_class->hasProperty && npObject->_class->setProperty && npObject->_class->hasProperty(npObject, identifier)) {
+        if (!_NPN_IsAlive(npObject))
+            return throwError("NPObject deleted", V8Proxy::ReferenceError);
 
         NPVariant npValue;
         VOID_TO_NPVARIANT(npValue);
index d47b4a5..e154ebb 100644 (file)
@@ -1,3 +1,17 @@
+2012-05-14  Nate Chapin  <japhet@chromium.org>
+
+        Add end-of-life test cases for https://bugs.webkit.org/show_bug.cgi?id=86131.
+
+        Reviewed by Adam Barth.
+
+        * DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:
+        (callDeletePlugin):
+        (pluginHasProperty):
+        (pluginHasMethod):
+        (pluginGetProperty):
+        (pluginSetProperty):
+        (pluginInvoke):
+
 2012-05-14  Dirk Pranke  <dpranke@chromium.org>
 
         Re-enable "drt mode" on chromium-mac-leopard
index 5442180..4ce9592 100644 (file)
@@ -145,6 +145,7 @@ enum {
     ID_LAST_SET_WINDOW_ARGUMENTS,
     ID_PROPERTY_WINDOWED_PLUGIN,
     ID_PROPERTY_TEST_OBJECT_COUNT,
+    ID_PROPERTY_DELETE_IN_GET_PROPERTY,
     NUM_PROPERTY_IDENTIFIERS
 };
 
@@ -161,6 +162,7 @@ static const NPUTF8 *pluginPropertyIdentifierNames[NUM_PROPERTY_IDENTIFIERS] = {
     "lastSetWindowArguments",
     "windowedPlugin",
     "testObjectCount",
+    "deletePluginInGetProperty"
 };
 
 enum {
@@ -202,6 +204,7 @@ enum {
     ID_NORMALIZE,
     ID_INVALIDATE_RECT,
     ID_OBJECTS_ARE_SAME,
+    ID_TEST_DELETE_WITHIN_INVOKE,
     NUM_METHOD_IDENTIFIERS
 };
 
@@ -244,7 +247,8 @@ static const NPUTF8 *pluginMethodIdentifierNames[NUM_METHOD_IDENTIFIERS] = {
     "resizeTo",
     "normalize",
     "invalidateRect",
-    "objectsAreSame"
+    "objectsAreSame",
+    "testDeleteWithinInvoke"
 };
 
 static NPUTF8* createCStringFromNPVariant(const NPVariant* variant)
@@ -262,8 +266,30 @@ static void initializeIdentifiers(void)
     browser->getstringidentifiers(pluginMethodIdentifierNames, NUM_METHOD_IDENTIFIERS, pluginMethodIdentifiers);
 }
 
+static bool callDeletePlugin(NPObject* obj, NPIdentifier name, NPIdentifier identifierToMatch)
+{
+    if (name != identifierToMatch)
+        return false;
+
+    PluginObject* plugin = reinterpret_cast<PluginObject*>(obj);
+    NPObject* windowScriptObject;
+    browser->getvalue(plugin->npp, NPNVWindowNPObject, &windowScriptObject);
+
+    NPIdentifier callbackIdentifier = browser->getstringidentifier("deletePlugin");
+    NPVariant browserResult;
+    if (browser->invoke(plugin->npp, windowScriptObject, callbackIdentifier, 0, 0, &browserResult))
+        browser->releasevariantvalue(&browserResult);
+    return true;
+}
+
 static bool pluginHasProperty(NPObject *obj, NPIdentifier name)
 {
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginReturnTrue")))
+        return true;
+
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginReturnFalse")))
+        return false;
+
     for (int i = 0; i < NUM_PROPERTY_IDENTIFIERS; i++)
         if (name == pluginPropertyIdentifiers[i])
             return true;
@@ -272,6 +298,9 @@ static bool pluginHasProperty(NPObject *obj, NPIdentifier name)
 
 static bool pluginHasMethod(NPObject *obj, NPIdentifier name)
 {
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginInHasMethod")))
+        return true;
+
     for (int i = 0; i < NUM_METHOD_IDENTIFIERS; i++)
         if (name == pluginMethodIdentifiers[i])
             return true;
@@ -331,12 +360,25 @@ static bool pluginGetProperty(NPObject* obj, NPIdentifier name, NPVariant* resul
         return true;
     }
 
+    if (name == pluginPropertyIdentifiers[ID_PROPERTY_DELETE_IN_GET_PROPERTY]) {
+        browser->retainobject(obj);
+        callDeletePlugin(obj, name, pluginPropertyIdentifiers[ID_PROPERTY_DELETE_IN_GET_PROPERTY]);
+        NPObject* testObject = plugin->testObject;
+        browser->retainobject(testObject);
+        OBJECT_TO_NPVARIANT(testObject, *result);
+        browser->releaseobject(obj);
+        return true;
+    }
+
     return false;
 }
 
 static bool pluginSetProperty(NPObject* obj, NPIdentifier name, const NPVariant* variant)
 {
     PluginObject* plugin = reinterpret_cast<PluginObject*>(obj);
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginReturnTrue")))
+        return true;
+
     if (name == pluginPropertyIdentifiers[ID_PROPERTY_EVENT_LOGGING]) {
         plugin->eventLogging = NPVARIANT_TO_BOOLEAN(*variant);
         return true;
@@ -1123,7 +1165,12 @@ static bool pluginInvoke(NPObject* header, NPIdentifier name, const NPVariant* a
         return invalidateRect(plugin, args, argCount, result);
     if (name == pluginMethodIdentifiers[ID_OBJECTS_ARE_SAME])
         return objectsAreSame(plugin, args, argCount, result);
-
+    if (name == pluginMethodIdentifiers[ID_TEST_DELETE_WITHIN_INVOKE]) {
+        NPObject* newObject = browser->createobject(plugin->npp, &pluginClass);
+        OBJECT_TO_NPVARIANT(newObject, *result);
+        callDeletePlugin(header, name, pluginMethodIdentifiers[ID_TEST_DELETE_WITHIN_INVOKE]);
+        return true;
+    }
     return false;
 }