Don't crash if QScriptClass property getter returns an invalid value

It's possible that a class claims to have a property of a given name
(i.e. queryProperty() returns true), but returns an invalid value
for that property. In that case we should silently convert the value
to undefined, otherwise the value may wreak havoc in JS.

This is a regression from Qt 4.5 (pre-JavaScriptCore-based), which
had this check.

Task-number: QTBUG-15079
Reviewed-by: Jedrzej Nowacki

2 files changed, 33 insertions, 0 deletions

diff --git a/src/script/bridge/qscriptclassobject.cpp b/src/script/bridge/qscriptclassobject.cpp
index dd229f1..228e755 100644
--- a/src/script/bridge/qscriptclassobject.cpp
+++ b/src/script/bridge/qscriptclassobject.cpp
@@ -78,6 +78,12 @@ bool ClassObjectDelegate::getOwnPropertySlot(QScriptObject* object,
         scriptObject, scriptName, QScriptClass::HandlesReadAccess, &id);
     if (flags & QScriptClass::HandlesReadAccess) {
         QScriptValue value = m_scriptClass->property(scriptObject, scriptName, id);
+        if (!value.isValid()) {
+            // The class claims to have the property, but returned an invalid
+            // value. Silently convert to undefined to avoid the invalid value
+            // "escaping" into JS.
+            value = QScriptValue(QScriptValue::UndefinedValue);
+        }
         slot.setValue(engine->scriptValueToJSCValue(value));
         return true;
     }
@@ -119,6 +125,12 @@ bool ClassObjectDelegate::getOwnPropertyDescriptor(QScriptObject *object,
         attribs |= pflags & QScriptValue::UserRange;
         // Rather than calling the getter, we could return an access descriptor here.
         QScriptValue value = m_scriptClass->property(scriptObject, scriptName, id);
+        if (!value.isValid()) {
+            // The class claims to have the property, but returned an invalid
+            // value. Silently convert to undefined to avoid the invalid value
+            // "escaping" into JS.
+            value = QScriptValue(QScriptValue::UndefinedValue);
+        }
         descriptor.setDescriptor(engine->scriptValueToJSCValue(value), attribs);
         return true;
     }
diff --git a/tests/auto/qscriptclass/tst_qscriptclass.cpp b/tests/auto/qscriptclass/tst_qscriptclass.cpp
index b4dbe73..da6c76f 100644
--- a/tests/auto/qscriptclass/tst_qscriptclass.cpp
+++ b/tests/auto/qscriptclass/tst_qscriptclass.cpp
@@ -66,6 +66,7 @@ public:
 private slots:
     void newInstance();
     void getAndSetProperty();
+    void getProperty_invalidValue();
     void enumerate();
     void extension();
 };
@@ -741,6 +742,26 @@ void tst_QScriptClass::getAndSetProperty()
     QVERIFY(!obj1.property(bar).isValid());
 }
 
+void tst_QScriptClass::getProperty_invalidValue()
+{
+    QScriptEngine eng;
+    TestClass cls(&eng);
+    cls.addCustomProperty(eng.toStringHandle("foo"), QScriptClass::HandlesReadAccess,
+                          /*id=*/0, QScriptValue::ReadOnly, QScriptValue());
+    QScriptValue obj = eng.newObject(&cls);
+
+    QVERIFY(obj.property("foo").isUndefined());
+
+    eng.globalObject().setProperty("obj", obj);
+    QVERIFY(eng.evaluate("obj.hasOwnProperty('foo'))").toBool());
+    // The JS environment expects that a valid value is returned,
+    // otherwise we could crash.
+    QVERIFY(eng.evaluate("obj.foo").isUndefined());
+    QVERIFY(eng.evaluate("obj.foo + ''").isString());
+    QVERIFY(eng.evaluate("Object.getOwnPropertyDescriptor(obj, 'foo').value").isUndefined());
+    QVERIFY(eng.evaluate("Object.getOwnPropertyDescriptor(obj, 'foo').value +''").isString());
+}
+
 void tst_QScriptClass::enumerate()
 {
     QScriptEngine eng;