You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
353 lines
14 KiB
353 lines
14 KiB
3 weeks ago
|
From 3ed063d2ff6b418e8487bea0d839fd2f768c91b6 Mon Sep 17 00:00:00 2001
|
||
|
From: Sami Shalayel <sami.shalayel@qt.io>
|
||
|
Date: Thu, 21 Mar 2024 14:43:19 +0100
|
||
|
Subject: [PATCH] dom: avoid asserts during dom construction
|
||
|
|
||
|
Add missing safeguards to avoid crashing when constructing the Dom, also
|
||
|
add some tests.
|
||
|
|
||
|
Fixes: QTBUG-123871
|
||
|
Change-Id: Ie5da7c3b7bbf61d49d755ec6c338e2011532e89b
|
||
|
(cherry picked from commit 4c4605be79e564921699a065df58333d3ee10d59)
|
||
|
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
|
||
|
---
|
||
|
|
||
|
diff --git a/src/qmldom/qqmldomastcreator.cpp b/src/qmldom/qqmldomastcreator.cpp
|
||
|
index 1f5db5a..d8e7590 100644
|
||
|
--- a/src/qmldom/qqmldomastcreator.cpp
|
||
|
+++ b/src/qmldom/qqmldomastcreator.cpp
|
||
|
@@ -495,8 +495,10 @@
|
||
|
{
|
||
|
if (auto &lastEl = currentNode(); lastEl.kind == DomType::Binding) {
|
||
|
Binding &b = std::get<Binding>(lastEl.value);
|
||
|
- if (m_enableScriptExpressions && scriptNodeStack.size() != 1)
|
||
|
+ if (m_enableScriptExpressions
|
||
|
+ && (scriptNodeStack.size() != 1 || scriptNodeStack.last().isList())) {
|
||
|
Q_SCRIPTELEMENT_DISABLE();
|
||
|
+ }
|
||
|
if (m_enableScriptExpressions) {
|
||
|
b.scriptExpressionValue()->setScriptElement(finalizeScriptExpression(
|
||
|
currentScriptNodeEl().takeVariant(), Path().field(Fields::scriptElement),
|
||
|
@@ -1077,7 +1079,7 @@
|
||
|
if (!m_enableScriptExpressions)
|
||
|
return false;
|
||
|
|
||
|
- if (scriptNodeStack.empty()) {
|
||
|
+ if (scriptNodeStack.empty() || scriptNodeStack.last().isList()) {
|
||
|
Q_SCRIPTELEMENT_DISABLE();
|
||
|
return false;
|
||
|
}
|
||
|
@@ -1106,7 +1108,7 @@
|
||
|
for (auto it = list; it; it = it->next) {
|
||
|
if (it->elision) {
|
||
|
Node::accept(it->elision, this);
|
||
|
- if (scriptNodeStack.empty()) {
|
||
|
+ if (scriptNodeStack.empty() || !scriptNodeStack.last().isList()) {
|
||
|
Q_SCRIPTELEMENT_DISABLE();
|
||
|
return false;
|
||
|
}
|
||
|
@@ -1115,7 +1117,7 @@
|
||
|
}
|
||
|
if (it->element) {
|
||
|
Node::accept(it->element, this);
|
||
|
- if (scriptNodeStack.empty()) {
|
||
|
+ if (scriptNodeStack.empty() || scriptNodeStack.last().isList()) {
|
||
|
Q_SCRIPTELEMENT_DISABLE();
|
||
|
return false;
|
||
|
}
|
||
|
@@ -1142,7 +1144,7 @@
|
||
|
Node::accept(it->property, this);
|
||
|
if (!m_enableScriptExpressions)
|
||
|
return false;
|
||
|
- if (scriptNodeStack.empty()) {
|
||
|
+ if (scriptNodeStack.empty() || scriptNodeStack.last().isList()) {
|
||
|
Q_SCRIPTELEMENT_DISABLE();
|
||
|
return false;
|
||
|
}
|
||
|
@@ -1346,7 +1348,7 @@
|
||
|
auto current = makeScriptList(list);
|
||
|
|
||
|
for (auto it = list; it; it = it->next) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current.append(scriptNodeStack.takeLast().takeVariant());
|
||
|
}
|
||
|
|
||
|
@@ -1364,15 +1366,15 @@
|
||
|
|
||
|
void QQmlDomAstCreator::endVisit(AST::BinaryExpression *exp)
|
||
|
{
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.size() < 2);
|
||
|
-
|
||
|
if (!m_enableScriptExpressions)
|
||
|
return;
|
||
|
|
||
|
auto current = makeScriptElement<ScriptElements::BinaryExpression>(exp);
|
||
|
current->addLocation(OperatorTokenRegion, exp->operatorToken);
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setRight(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setLeft(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
|
||
|
@@ -1426,19 +1428,19 @@
|
||
|
current->addLocation(FileLocationRegion::RightParenthesisRegion, forStatement->rparenToken);
|
||
|
|
||
|
if (forStatement->statement) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setBody(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode(std::nullopt);
|
||
|
}
|
||
|
|
||
|
if (forStatement->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setExpression(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode(std::nullopt);
|
||
|
}
|
||
|
|
||
|
if (forStatement->condition) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setCondition(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode(std::nullopt);
|
||
|
}
|
||
|
@@ -1458,7 +1460,7 @@
|
||
|
}
|
||
|
|
||
|
if (forStatement->initialiser) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setInitializer(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode(std::nullopt);
|
||
|
}
|
||
|
@@ -1590,7 +1592,7 @@
|
||
|
Node::accept(it->declaration, this);
|
||
|
if (!m_enableScriptExpressions)
|
||
|
return false;
|
||
|
- if (scriptNodeStack.empty()) {
|
||
|
+ if (scriptNodeStack.empty() || scriptNodeStack.last().isList()) {
|
||
|
Q_SCRIPTELEMENT_DISABLE();
|
||
|
return false;
|
||
|
}
|
||
|
@@ -1697,18 +1699,18 @@
|
||
|
current->addLocation(ElseKeywordRegion, ifStatement->elseToken);
|
||
|
|
||
|
if (ifStatement->ko) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setAlternative(scriptNodeStack.last().takeVariant());
|
||
|
scriptNodeStack.removeLast();
|
||
|
}
|
||
|
|
||
|
if (ifStatement->ok) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setConsequence(scriptNodeStack.last().takeVariant());
|
||
|
scriptNodeStack.removeLast();
|
||
|
}
|
||
|
if (ifStatement->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setCondition(scriptNodeStack.last().takeVariant());
|
||
|
scriptNodeStack.removeLast();
|
||
|
}
|
||
|
@@ -1733,7 +1735,7 @@
|
||
|
current->addLocation(ReturnKeywordRegion, returnStatement->returnToken);
|
||
|
|
||
|
if (returnStatement->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setExpression(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -1759,7 +1761,7 @@
|
||
|
current->addLocation(FileLocationRegion::OperatorTokenRegion, expression->dotToken);
|
||
|
|
||
|
if (expression->base) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setLeft(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -1790,7 +1792,7 @@
|
||
|
current->addLocation(FileLocationRegion::OperatorTokenRegion, expression->lbracketToken);
|
||
|
|
||
|
if (expression->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
// if scriptNodeStack.last() is fieldmember expression, add expression to it instead of
|
||
|
// creating new one
|
||
|
current->setRight(currentScriptNodeEl().takeVariant());
|
||
|
@@ -1798,7 +1800,7 @@
|
||
|
}
|
||
|
|
||
|
if (expression->base) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->setLeft(currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -1834,7 +1836,7 @@
|
||
|
}
|
||
|
|
||
|
if (exp->base) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::callee, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -1924,7 +1926,7 @@
|
||
|
return;
|
||
|
|
||
|
if (exp->name) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::name, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -2038,7 +2040,7 @@
|
||
|
}
|
||
|
|
||
|
if (exp->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::expression, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -2062,7 +2064,7 @@
|
||
|
auto current = makeScriptList(list);
|
||
|
|
||
|
for (auto it = list; it; it = it->next) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current.append(scriptNodeStack.takeLast().takeVariant());
|
||
|
}
|
||
|
|
||
|
@@ -2094,7 +2096,7 @@
|
||
|
}
|
||
|
|
||
|
if (exp->defaultClause) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::defaultClause, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -2125,12 +2127,12 @@
|
||
|
current->addLocation(FileLocationRegion::RightParenthesisRegion, exp->rparenToken);
|
||
|
|
||
|
if (exp->block) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::caseBlock, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
if (exp->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::expression, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -2157,13 +2159,13 @@
|
||
|
current->addLocation(FileLocationRegion::RightParenthesisRegion, exp->rparenToken);
|
||
|
|
||
|
if (exp->statement) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::body, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
|
||
|
if (exp->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::expression, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -2191,13 +2193,13 @@
|
||
|
current->addLocation(FileLocationRegion::RightParenthesisRegion, exp->rparenToken);
|
||
|
|
||
|
if (exp->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::expression, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
|
||
|
if (exp->statement) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::body, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
@@ -2224,18 +2226,18 @@
|
||
|
current->addLocation(FileLocationRegion::RightParenthesisRegion, exp->rparenToken);
|
||
|
|
||
|
if (exp->statement) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::body, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
if (exp->expression) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::expression, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
|
||
|
if (exp->lhs) {
|
||
|
- Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty());
|
||
|
+ Q_SCRIPTELEMENT_EXIT_IF(scriptNodeStack.isEmpty() || scriptNodeStack.last().isList());
|
||
|
current->insertChild(Fields::bindingElement, currentScriptNodeEl().takeVariant());
|
||
|
removeCurrentScriptNode({});
|
||
|
}
|
||
|
diff --git a/tests/auto/qmldom/domdata/domitem/crashes/lambda.qml b/tests/auto/qmldom/domdata/domitem/crashes/lambda.qml
|
||
|
new file mode 100644
|
||
|
index 0000000..85f696f
|
||
|
--- /dev/null
|
||
|
+++ b/tests/auto/qmldom/domdata/domitem/crashes/lambda.qml
|
||
|
@@ -0,0 +1,7 @@
|
||
|
+import QtQuick.Controls
|
||
|
+
|
||
|
+Action {
|
||
|
+ onTriggered: foo(Bla.Bar, function() {
|
||
|
+ console.log("Hello")
|
||
|
+ })
|
||
|
+}
|
||
|
diff --git a/tests/auto/qmldom/domdata/domitem/crashes/templateStrings.qml b/tests/auto/qmldom/domdata/domitem/crashes/templateStrings.qml
|
||
|
new file mode 100644
|
||
|
index 0000000..feb5646
|
||
|
--- /dev/null
|
||
|
+++ b/tests/auto/qmldom/domdata/domitem/crashes/templateStrings.qml
|
||
|
@@ -0,0 +1,10 @@
|
||
|
+import QtQuick
|
||
|
+
|
||
|
+
|
||
|
+Item {
|
||
|
+ property string verbatim1: 'A "verbatim" string!'
|
||
|
+ property string verbatim2: "A 'verbatim' string\u2757"
|
||
|
+ property string verbatim3: `400 + 300 is ${400 + 300}.
|
||
|
+
|
||
|
+mutliline`
|
||
|
+}
|
||
|
diff --git a/tests/auto/qmldom/domitem/tst_qmldomitem.h b/tests/auto/qmldom/domitem/tst_qmldomitem.h
|
||
|
index 184dc8e..3673871 100644
|
||
|
--- a/tests/auto/qmldom/domitem/tst_qmldomitem.h
|
||
|
+++ b/tests/auto/qmldom/domitem/tst_qmldomitem.h
|
||
|
@@ -2813,6 +2813,12 @@
|
||
|
|
||
|
QTest::addRow("inactiveVisitorMarkerCrash")
|
||
|
<< baseDir + u"/inactiveVisitorMarkerCrash.qml"_s;
|
||
|
+
|
||
|
+ QTest::addRow("templateStrings")
|
||
|
+ << baseDir + u"/crashes/templateStrings.qml"_s;
|
||
|
+
|
||
|
+ QTest::addRow("lambda")
|
||
|
+ << baseDir + u"/crashes/lambda.qml"_s;
|
||
|
}
|
||
|
void crashes()
|
||
|
{
|