Ensure that the prototype chain is checked in property Get

This commit ensures that the prototype chain is checked during
property Get operations on QObjects and other types in QML by
returning an empty handle from the property Get interceptor if
no valid property with the given name is found.

Task-number: QTBUG-20336
Change-Id: I670ee211c74c0fdcb68c3f91b29fcc0ea8b55d6f
Reviewed-on: http://codereview.qt.nokia.com/1477
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>
Reviewed-by: Kent Hansen <kent.hansen@nokia.com>

diff --git a/src/declarative/qml/v8/qv8listwrapper.cpp b/src/declarative/qml/v8/qv8listwrapper.cpp
index 5f2d9fb..a8d1873 100644 (file)
--- a/src/declarative/qml/v8/qv8listwrapper.cpp
+++ b/src/declarative/qml/v8/qv8listwrapper.cpp
@@ -135,7 +135,7 @@ v8::Handle<v8::Value> QV8ListWrapper::Getter(v8::Local<v8::String> property,
 {
     Q_UNUSED(property);
     Q_UNUSED(info);
-    return v8::Undefined();
+    return v8::Handle<v8::Value>();
 }
 
 v8::Handle<v8::Value> QV8ListWrapper::Setter(v8::Local<v8::String> property, 

diff --git a/src/declarative/qml/v8/qv8qobjectwrapper.cpp b/src/declarative/qml/v8/qv8qobjectwrapper.cpp
index f97f427..e0d61df 100644 (file)
--- a/src/declarative/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/declarative/qml/v8/qv8qobjectwrapper.cpp
@@ -600,7 +600,7 @@ v8::Handle<v8::Value> QV8QObjectWrapper::Getter(v8::Local<v8::String> property,
     QV8QObjectResource *resource = v8_resource_check<QV8QObjectResource>(info.This());
 
     if (resource->object.isNull()) 
-        return v8::Undefined();
+        return v8::Handle<v8::Value>();
 
     QObject *object = resource->object;
 
@@ -628,7 +628,7 @@ v8::Handle<v8::Value> QV8QObjectWrapper::Getter(v8::Local<v8::String> property,
         }
     } 
 
-    return v8::Undefined();
+    return v8::Handle<v8::Value>();
 }
 
 v8::Handle<v8::Value> QV8QObjectWrapper::Setter(v8::Local<v8::String> property, 

diff --git a/src/declarative/qml/v8/qv8typewrapper.cpp b/src/declarative/qml/v8/qv8typewrapper.cpp
index fe30670..736ddc4 100644 (file)
--- a/src/declarative/qml/v8/qv8typewrapper.cpp
+++ b/src/declarative/qml/v8/qv8typewrapper.cpp
@@ -150,7 +150,7 @@ v8::Handle<v8::Value> QV8TypeWrapper::Getter(v8::Local<v8::String> property,
                 }
             } 
 
-            // Fall through to undefined
+            // Fall through to return empty handle
 
         } else if (resource->object) {
             QObject *ao = qmlAttachedPropertiesObjectById(type->attachedPropertiesId(), object);
@@ -158,10 +158,10 @@ v8::Handle<v8::Value> QV8TypeWrapper::Getter(v8::Local<v8::String> property,
                 return v8engine->qobjectWrapper()->getProperty(ao, propertystring, 
                                                                QV8QObjectWrapper::IgnoreRevision);
 
-            // Fall through to undefined
+            // Fall through to return empty handle
         }
 
-        // Fall through to undefined
+        // Fall through to return empty handle
 
     } else if (resource->typeNamespace) {
 
@@ -185,21 +185,19 @@ v8::Handle<v8::Value> QV8TypeWrapper::Getter(v8::Local<v8::String> property,
 
             if (moduleApi->qobjectApi) {
                 v8::Handle<v8::Value> rv = v8engine->qobjectWrapper()->getProperty(moduleApi->qobjectApi, propertystring, QV8QObjectWrapper::IgnoreRevision);
-                if (rv.IsEmpty())
-                    return v8::Undefined();
-                else 
-                    return rv;
+                return rv;
             } else {
-                return v8::Undefined();
+                return v8::Handle<v8::Value>();
             }
         }
 
-        // Fall through to undefined
+        // Fall through to return empty handle
 
     } else {
         Q_ASSERT(!"Unreachable");
     }
-    return v8::Undefined(); 
+
+    return v8::Handle<v8::Value>();
 }
 
 v8::Handle<v8::Value> QV8TypeWrapper::Setter(v8::Local<v8::String> property, 

diff --git a/src/declarative/qml/v8/qv8valuetypewrapper.cpp b/src/declarative/qml/v8/qv8valuetypewrapper.cpp
index a556260..4d96d53 100644 (file)
--- a/src/declarative/qml/v8/qv8valuetypewrapper.cpp
+++ b/src/declarative/qml/v8/qv8valuetypewrapper.cpp
@@ -229,7 +229,7 @@ v8::Handle<v8::Value> QV8ValueTypeWrapper::Getter(v8::Local<v8::String> property
                                                   const v8::AccessorInfo &info)
 {
     QV8ValueTypeResource *r =  v8_resource_cast<QV8ValueTypeResource>(info.This());
-    if (!r) return v8::Undefined();
+    if (!r) return v8::Handle<v8::Value>();
 
     // XXX This is horribly inefficient.  Sadly people seem to have taken a liking to 
     // value type properties, so we should probably try and optimize it a little.
@@ -242,13 +242,15 @@ v8::Handle<v8::Value> QV8ValueTypeWrapper::Getter(v8::Local<v8::String> property
 
     int index = r->type->metaObject()->indexOfProperty(propName.constData());
     if (index == -1)
-        return v8::Undefined();
+        return v8::Handle<v8::Value>();
+
 
     if (r->objectType == QV8ValueTypeResource::Reference) {
         QV8ValueTypeReferenceResource *reference = static_cast<QV8ValueTypeReferenceResource *>(r);
 
         if (!reference->object)
-            return v8::Undefined();
+            return v8::Handle<v8::Value>();
+
 
         r->type->read(reference->object, reference->property);
     } else {

diff --git a/src/declarative/qml/v8/qv8variantwrapper.cpp b/src/declarative/qml/v8/qv8variantwrapper.cpp
index d4097d7..907f927 100644 (file)
--- a/src/declarative/qml/v8/qv8variantwrapper.cpp
+++ b/src/declarative/qml/v8/qv8variantwrapper.cpp
@@ -162,14 +162,14 @@ QVariant QV8VariantWrapper::toVariant(QV8ObjectResource *r)
 v8::Handle<v8::Value> QV8VariantWrapper::Getter(v8::Local<v8::String> property, 
                                                 const v8::AccessorInfo &info)
 {
-    return v8::Undefined();
+    return v8::Handle<v8::Value>();
 }
 
 v8::Handle<v8::Value> QV8VariantWrapper::Setter(v8::Local<v8::String> property, 
                                                 v8::Local<v8::Value> value,
                                                 const v8::AccessorInfo &info)
 {
-    return v8::Undefined();
+    return value;
 }
 
 v8::Handle<v8::Value> QV8VariantWrapper::PreserveGetter(v8::Local<v8::String> property, 

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/declarativeHasOwnProperty.qml b/tests/auto/declarative/qdeclarativeecmascript/data/declarativeHasOwnProperty.qml
new file mode 100644 (file)
index 0000000..12598b3
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/declarativeHasOwnProperty.qml
@@ -0,0 +1,72 @@
+import QtQuick 2.0
+import Qt.test 1.0
+import Qt.test.qobjectApi 1.0 as QtTestQObjectApi
+
+Item {
+    id: obj
+    objectName: "objName"
+    property int someIntProperty: 10
+    property bool result: false
+
+    function testHasOwnPropertySuccess()
+    {
+        obj.result = obj.hasOwnProperty("someIntProperty");
+    }
+
+    function testHasOwnPropertyFailure()
+    {
+        obj.result = obj.hasOwnProperty("someNonexistentProperty");
+    }
+
+    MyTypeObject {
+        id: typeObj
+        objectName: "typeObj"
+        pointProperty: Qt.point(34, 29)
+        variantProperty: Qt.vector3d(1, 2, 3)
+        stringProperty: "test string"
+        property list<Rectangle> listProperty: [ Rectangle { width: 10; height: 10 } ]
+        property list<Rectangle> emptyListProperty
+
+        property bool valueTypeHasOwnProperty
+        property bool valueTypeHasOwnProperty2
+        property bool variantTypeHasOwnProperty
+        property bool stringTypeHasOwnProperty
+        property bool listTypeHasOwnProperty
+        property bool listAtValidHasOwnProperty
+        property bool emptyListTypeHasOwnProperty
+        property bool enumTypeHasOwnProperty
+        property bool typenameHasOwnProperty
+        property bool typenameHasOwnProperty2
+        property bool moduleApiTypeHasOwnProperty
+        property bool moduleApiPropertyTypeHasOwnProperty
+        function testHasOwnPropertySuccess() {
+            valueTypeHasOwnProperty = !typeObj.pointProperty.hasOwnProperty("nonexistentpropertyname");
+            valueTypeHasOwnProperty2 = typeObj.pointProperty.hasOwnProperty("x"); // should be true
+            variantTypeHasOwnProperty = !typeObj.variantProperty.hasOwnProperty("nonexistentpropertyname");
+            stringTypeHasOwnProperty = !typeObj.stringProperty.hasOwnProperty("nonexistentpropertyname");
+            listTypeHasOwnProperty = !typeObj.listProperty.hasOwnProperty("nonexistentpropertyname");
+            listAtValidHasOwnProperty = !typeObj.listProperty[0].hasOwnProperty("nonexistentpropertyname");
+            emptyListTypeHasOwnProperty = !typeObj.emptyListProperty.hasOwnProperty("nonexistentpropertyname");
+            enumTypeHasOwnProperty = !MyTypeObject.EnumVal1.hasOwnProperty("nonexistentpropertyname");
+            typenameHasOwnProperty = !MyTypeObject.hasOwnProperty("nonexistentpropertyname");
+            typenameHasOwnProperty2 = MyTypeObject.hasOwnProperty("EnumVal1"); // should be true.
+            moduleApiTypeHasOwnProperty = !QtTestQObjectApi.hasOwnProperty("nonexistentpropertyname");
+            moduleApiPropertyTypeHasOwnProperty = !QtTestQObjectApi.qobjectTestProperty.hasOwnProperty("nonexistentpropertyname");
+        }
+
+        property bool enumNonValueHasOwnProperty
+        function testHasOwnPropertyFailureOne() {
+            enumNonValueHasOwnProperty = !MyTypeObject.NonexistentEnumVal.hasOwnProperty("nonexistentpropertyname");
+        }
+
+        property bool moduleApiNonPropertyHasOwnProperty
+        function testHasOwnPropertyFailureTwo() {
+            moduleApiNonPropertyHasOwnProperty = !QtTestQObjectApi.someNonexistentProperty.hasOwnProperty("nonexistentpropertyname");
+        }
+
+        property bool listAtInvalidHasOwnProperty
+        function testHasOwnPropertyFailureThree() {
+            listAtInvalidHasOwnProperty = !typeObj.listProperty[5].hasOwnProperty("nonexistentpropertyname");
+        }
+    }
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index 6c629a8..c5f7ed1 100644 (file)
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -113,6 +113,7 @@ private slots:
     void dynamicCreation();
     void dynamicDestruction();
     void objectToString();
+    void objectHasOwnProperty();
     void selfDeletingBinding();
     void extendedObjectPropertyLookup();
     void scriptErrors();
@@ -1141,6 +1142,55 @@ void tst_qdeclarativeecmascript::objectToString()
 }
 
 /*
+  tests that id.hasOwnProperty() works
+*/
+void tst_qdeclarativeecmascript::objectHasOwnProperty()
+{
+    QUrl url = TEST_FILE("declarativeHasOwnProperty.qml");
+    QString warning1 = url.toString() + ":59: TypeError: Cannot call method 'hasOwnProperty' of undefined";
+    QString warning2 = url.toString() + ":64: TypeError: Cannot call method 'hasOwnProperty' of undefined";
+    QString warning3 = url.toString() + ":69: TypeError: Cannot call method 'hasOwnProperty' of undefined";
+
+    QDeclarativeComponent component(&engine, url);
+    QObject *object = component.create();
+    QVERIFY(object != 0);
+
+    // test QObjects in QML
+    QMetaObject::invokeMethod(object, "testHasOwnPropertySuccess");
+    QVERIFY(object->property("result").value<bool>() == true);
+    QMetaObject::invokeMethod(object, "testHasOwnPropertyFailure");
+    QVERIFY(object->property("result").value<bool>() == false);
+
+    // now test other types in QML
+    QObject *child = object->findChild<QObject*>("typeObj");
+    QVERIFY(child != 0);
+    QMetaObject::invokeMethod(child, "testHasOwnPropertySuccess");
+    QCOMPARE(child->property("valueTypeHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("valueTypeHasOwnProperty2").toBool(), true);
+    QCOMPARE(child->property("variantTypeHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("stringTypeHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("listTypeHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("emptyListTypeHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("enumTypeHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("typenameHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("typenameHasOwnProperty2").toBool(), true);
+    QCOMPARE(child->property("moduleApiTypeHasOwnProperty").toBool(), true);
+    QCOMPARE(child->property("moduleApiPropertyTypeHasOwnProperty").toBool(), true);
+
+    QTest::ignoreMessage(QtWarningMsg, warning1.toLatin1().constData());
+    QMetaObject::invokeMethod(child, "testHasOwnPropertyFailureOne");
+    QCOMPARE(child->property("enumNonValueHasOwnProperty").toBool(), false);
+    QTest::ignoreMessage(QtWarningMsg, warning2.toLatin1().constData());
+    QMetaObject::invokeMethod(child, "testHasOwnPropertyFailureTwo");
+    QCOMPARE(child->property("moduleApiNonPropertyHasOwnProperty").toBool(), false);
+    QTest::ignoreMessage(QtWarningMsg, warning3.toLatin1().constData());
+    QMetaObject::invokeMethod(child, "testHasOwnPropertyFailureThree");
+    QCOMPARE(child->property("listAtInvalidHasOwnProperty").toBool(), false);
+
+    delete object;
+}
+
+/*
 Tests bindings that indirectly cause their own deletion work.
 
 This test is best run under valgrind to ensure no invalid memory access occur.