From 6f146a5d2ea893b13e4cfcd90243e66f7c9a0083 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Tue, 12 Jul 2011 15:55:01 +1000 Subject: [PATCH] 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 Reviewed-by: Aaron Kennedy Reviewed-by: Kent Hansen --- src/declarative/qml/v8/qv8listwrapper.cpp | 2 +- src/declarative/qml/v8/qv8qobjectwrapper.cpp | 4 +- src/declarative/qml/v8/qv8typewrapper.cpp | 18 ++--- src/declarative/qml/v8/qv8valuetypewrapper.cpp | 8 ++- src/declarative/qml/v8/qv8variantwrapper.cpp | 4 +- .../data/declarativeHasOwnProperty.qml | 72 ++++++++++++++++++++ .../tst_qdeclarativeecmascript.cpp | 50 ++++++++++++++ 7 files changed, 140 insertions(+), 18 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/declarativeHasOwnProperty.qml diff --git a/src/declarative/qml/v8/qv8listwrapper.cpp b/src/declarative/qml/v8/qv8listwrapper.cpp index 5f2d9fb..a8d1873 100644 --- a/src/declarative/qml/v8/qv8listwrapper.cpp +++ b/src/declarative/qml/v8/qv8listwrapper.cpp @@ -135,7 +135,7 @@ v8::Handle QV8ListWrapper::Getter(v8::Local property, { Q_UNUSED(property); Q_UNUSED(info); - return v8::Undefined(); + return v8::Handle(); } v8::Handle QV8ListWrapper::Setter(v8::Local property, diff --git a/src/declarative/qml/v8/qv8qobjectwrapper.cpp b/src/declarative/qml/v8/qv8qobjectwrapper.cpp index f97f427..e0d61df 100644 --- a/src/declarative/qml/v8/qv8qobjectwrapper.cpp +++ b/src/declarative/qml/v8/qv8qobjectwrapper.cpp @@ -600,7 +600,7 @@ v8::Handle QV8QObjectWrapper::Getter(v8::Local property, QV8QObjectResource *resource = v8_resource_check(info.This()); if (resource->object.isNull()) - return v8::Undefined(); + return v8::Handle(); QObject *object = resource->object; @@ -628,7 +628,7 @@ v8::Handle QV8QObjectWrapper::Getter(v8::Local property, } } - return v8::Undefined(); + return v8::Handle(); } v8::Handle QV8QObjectWrapper::Setter(v8::Local property, diff --git a/src/declarative/qml/v8/qv8typewrapper.cpp b/src/declarative/qml/v8/qv8typewrapper.cpp index fe30670..736ddc4 100644 --- a/src/declarative/qml/v8/qv8typewrapper.cpp +++ b/src/declarative/qml/v8/qv8typewrapper.cpp @@ -150,7 +150,7 @@ v8::Handle QV8TypeWrapper::Getter(v8::Local 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 QV8TypeWrapper::Getter(v8::Local 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 QV8TypeWrapper::Getter(v8::Local property, if (moduleApi->qobjectApi) { v8::Handle 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(); } } - // Fall through to undefined + // Fall through to return empty handle } else { Q_ASSERT(!"Unreachable"); } - return v8::Undefined(); + + return v8::Handle(); } v8::Handle QV8TypeWrapper::Setter(v8::Local property, diff --git a/src/declarative/qml/v8/qv8valuetypewrapper.cpp b/src/declarative/qml/v8/qv8valuetypewrapper.cpp index a556260..4d96d53 100644 --- a/src/declarative/qml/v8/qv8valuetypewrapper.cpp +++ b/src/declarative/qml/v8/qv8valuetypewrapper.cpp @@ -229,7 +229,7 @@ v8::Handle QV8ValueTypeWrapper::Getter(v8::Local property const v8::AccessorInfo &info) { QV8ValueTypeResource *r = v8_resource_cast(info.This()); - if (!r) return v8::Undefined(); + if (!r) return v8::Handle(); // 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 QV8ValueTypeWrapper::Getter(v8::Local property int index = r->type->metaObject()->indexOfProperty(propName.constData()); if (index == -1) - return v8::Undefined(); + return v8::Handle(); + if (r->objectType == QV8ValueTypeResource::Reference) { QV8ValueTypeReferenceResource *reference = static_cast(r); if (!reference->object) - return v8::Undefined(); + return v8::Handle(); + 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 --- 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 QV8VariantWrapper::Getter(v8::Local property, const v8::AccessorInfo &info) { - return v8::Undefined(); + return v8::Handle(); } v8::Handle QV8VariantWrapper::Setter(v8::Local property, v8::Local value, const v8::AccessorInfo &info) { - return v8::Undefined(); + return value; } v8::Handle QV8VariantWrapper::PreserveGetter(v8::Local property, diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/declarativeHasOwnProperty.qml b/tests/auto/declarative/qdeclarativeecmascript/data/declarativeHasOwnProperty.qml new file mode 100644 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 listProperty: [ Rectangle { width: 10; height: 10 } ] + property list 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 --- 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() == true); + QMetaObject::invokeMethod(object, "testHasOwnPropertyFailure"); + QVERIFY(object->property("result").value() == false); + + // now test other types in QML + QObject *child = object->findChild("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. -- 1.7.2.5