Prevent "dynamic" values from getting assigned to command properties

Values such as artifact objects are not plain data and we therefore must
not attempt to make deep copies of them. Catch attempts to assign them
to command properties.

Fixes: QBS-1404
Change-Id: I1fa02720a3543cd8e2c734ed9437d31b5a573d57
Reviewed-by: Pier Luigi Fiorini <pierluigi.fiorini@liri.io>

5 files changed, 29 insertions, 8 deletions

diff --git a/src/lib/corelib/buildgraph/rulecommands.cpp b/src/lib/corelib/buildgraph/rulecommands.cpp
index 4bc005ccb..a15047290 100644
--- a/src/lib/corelib/buildgraph/rulecommands.cpp
+++ b/src/lib/corelib/buildgraph/rulecommands.cpp
@@ -149,7 +149,9 @@ void AbstractCommand::applyCommandProperties(const QScriptValue *scriptValue)
         if (m_predefinedProperties.contains(it.name()))
             continue;
         const QVariant value = it.value().toVariant();
-        if (QMetaType::Type(value.type()) == QMetaType::QObjectStar) {
+        if (QMetaType::Type(value.type()) == QMetaType::QObjectStar
+                || it.value().scriptClass()
+                || it.value().data().isValid()) {
             throw ErrorInfo(Tr::tr("Property '%1' has a type unsuitable for storing in a command "
                                    "object.").arg(it.name()), m_codeLocation);
         }
diff --git a/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs b/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs
index f3c10138a..7ed5f5d96 100644
--- a/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs
+++ b/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs
@@ -45,8 +45,8 @@ Product {
             var cmd = new JavaScriptCommand();
             var output = outputs["mytype.final"][0];
             cmd.description = "generating " + output.fileName;
-            cmd.output = output;
-            cmd.sourceCode = function() { File.copy(input.filePath, output.filePath); };
+            cmd.outputFilePath = output.filePath;
+            cmd.sourceCode = function() { File.copy(input.filePath, outputFilePath); };
             return [cmd];
         }
     }
diff --git a/tests/auto/blackbox/testdata/invalid-command-property/invalid-command-property.qbs b/tests/auto/blackbox/testdata/invalid-command-property/invalid-command-property.qbs
index 28bcca827..b08fcd4a3 100644
--- a/tests/auto/blackbox/testdata/invalid-command-property/invalid-command-property.qbs
+++ b/tests/auto/blackbox/testdata/invalid-command-property/invalid-command-property.qbs
@@ -1,6 +1,8 @@
 import qbs.TextFile
 
 Product {
+    name: "p"
+    property string errorType
     type: ["output"]
     Group {
         files: ["input.txt"]
@@ -15,11 +17,15 @@ Product {
         prepare: {
             var cmd = new JavaScriptCommand();
             cmd.description = "Creating output";
-            cmd.textFile = new TextFile(input.filePath, TextFile.ReadOnly);
-            cmd.sourceCode = function() {
-                var content = textFile.readAll();
-                textFile.close();
-            }
+            if (product.errorType === "qobject")
+                cmd.dummy = new TextFile(input.filePath, TextFile.ReadOnly);
+            else if (product.errorType === "input")
+                cmd.dummy = input;
+            else if (product.errorType === "artifact")
+                cmd.dummy = product.artifacts.qbs[0];
+            else
+                throw "invalid error type " + product.errorType;
+            cmd.sourceCode = function() { }
             return [cmd];
         }
     }
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 5dc858585..3d15281e2 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -3831,9 +3831,21 @@ void TestBlackbox::installTree()
     QVERIFY(QFile::exists(installRoot + "content/subdir2/baz.txt"));
 }
 
+void TestBlackbox::invalidCommandProperty_data()
+{
+    QTest::addColumn<QString>("errorType");
+
+    QTest::newRow("assigning QObject") << QString("qobject");
+    QTest::newRow("assigning input artifact") << QString("input");
+    QTest::newRow("assigning other artifact") << QString("artifact");
+}
+
 void TestBlackbox::invalidCommandProperty()
 {
     QDir::setCurrent(testDataDir + "/invalid-command-property");
+    QFETCH(QString, errorType);
+    QCOMPARE(runQbs(QbsRunParameters("resolve", QStringList("products.p.errorType:" + errorType))),
+             0);
     QbsRunParameters params;
     params.expectFailure = true;
     QVERIFY(runQbs(params) != 0);
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index 92ba338ab..3df507bdc 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -144,6 +144,7 @@ private slots:
     void installPackage();
     void installRootFromProjectFile();
     void installTree();
+    void invalidCommandProperty_data();
     void invalidCommandProperty();
     void invalidExtensionInstantiation();
     void invalidExtensionInstantiation_data();