From 948599e7b71f59fd9c9c0f7f3d1987ec93a23490 Mon Sep 17 00:00:00 2001 From: Tatiana Borisova Date: Tue, 26 Nov 2024 21:18:40 +0100 Subject: [PATCH] QDomDocument::toByteArray() crashed in case of high XML nesting level The issue the combination of: - 300+ XML nesting level - Small stack size, by default on Windows (1 MB) - Unexpected and unexplained large stack frames with MSVC (3.5 kB) The described factors combination leads to the stack overflow on Windows + MSVC. To fix the problem, I got rid of the recursive call from QDomElementPrivate::save() and removed QDomNodePrivate::save() implementation. Instead of those I added the method that iterates through the tree not using recursion. [ChangeLog][QtXml] QDomDocument::toByteArray() now iterates the nodes of the document instead of recursing into subnodes. This avoids a stack-overflow crash that used to arise with deeply-nested document structures. Fixes: QTBUG-131151 Change-Id: Ib74aaef1422716f2aafcb89dfc8c05ef334e2a54 Reviewed-by: Thiago Macieira (cherry picked from commit 387633a6069a5e0e9b976971691b1b82725b6132) --- diff --git a/src/xml/dom/qdom.cpp b/src/xml/dom/qdom.cpp index 64f3203..834ab24 100644 --- a/src/xml/dom/qdom.cpp +++ b/src/xml/dom/qdom.cpp @@ -1349,16 +1349,40 @@ qNormalizeNode(this); } -/*! \internal - \a depth is used for indentation, it seems. - */ -void QDomNodePrivate::save(QTextStream& s, int depth, int indent) const +void QDomNodePrivate::saveSubTree(const QDomNodePrivate *n, QTextStream &s, + int depth, int indent) const { - const QDomNodePrivate* n = first; - while (n) { - n->save(s, depth, indent); - n = n->next; + if (!n) + return; + + const QDomNodePrivate *root = n->first; + n->save(s, depth, indent); + if (root) { + const int branchDepth = depth + 1; + int layerDepth = 0; + while (root) { + root->save(s, layerDepth + branchDepth, indent); + // A flattened (non-recursive) depth-first walk through the node tree. + if (root->first) { + layerDepth ++; + root = root->first; + continue; + } + root->afterSave(s, layerDepth + branchDepth, indent); + const QDomNodePrivate *prev = root; + root = root->next; + // Close QDomElementPrivate groups + while (!root && prev && (layerDepth > 0)) { + root = prev->parent(); + layerDepth --; + root->afterSave(s, layerDepth + branchDepth, indent); + prev = root; + root = root->next; + } + } + Q_ASSERT(layerDepth == 0); } + n->afterSave(s, depth, indent); } void QDomNodePrivate::setLocation(int lineNumber, int columnNumber) @@ -2146,7 +2170,7 @@ if (isDocument()) static_cast(impl)->saveDocument(stream, indent, encodingPolicy); else - IMPL->save(stream, 1, indent); + IMPL->saveSubTree(IMPL, stream, 1, indent); } /*! @@ -3064,11 +3088,11 @@ auto it2 = notations->map.constBegin(); for (; it2 != notations->map.constEnd(); ++it2) - it2.value()->save(s, 0, indent); + it2.value()->saveSubTree(it2.value(), s, 0, indent); auto it = entities->map.constBegin(); for (; it != entities->map.constEnd(); ++it) - it.value()->save(s, 0, indent); + it.value()->saveSubTree(it.value(), s, 0, indent); s << ']'; } @@ -4121,13 +4145,25 @@ if (indent != -1) s << Qt::endl; } - QDomNodePrivate::save(s, depth + 1, indent); if (!last->isText()) - s << QString(indent < 1 ? 0 : depth * indent, u' '); - - s << "'; } else { s << "/>"; } +} + +void QDomElementPrivate::afterSave(QTextStream &s, int depth, int indent) const +{ + if (last) { + QString qName(name); + + if (!prefix.isEmpty()) + qName = prefix + u':' + name; + + if (!last->isText()) + s << QString(indent < 1 ? 0 : depth * indent, u' '); + + s << "'; + } + if (!(next && next->isText())) { /* -1 disables new lines. */ if (indent != -1) @@ -5908,7 +5944,7 @@ type->save(s, 0, indent); doc = true; } - n->save(s, 0, indent); + n->saveSubTree(n, s, 0, indent); n = n->next; } } @@ -5935,8 +5971,8 @@ } // Now we serialize all the nodes after the faked XML declaration(the PI). - while(startNode) { - startNode->save(s, 0, indent); + while (startNode) { + startNode->saveSubTree(startNode, s, 0, indent); startNode = startNode->next; } } diff --git a/src/xml/dom/qdom_p.h b/src/xml/dom/qdom_p.h index 507852b..55f5b23 100644 --- a/src/xml/dom/qdom_p.h +++ b/src/xml/dom/qdom_p.h @@ -109,7 +109,9 @@ virtual QDomNode::NodeType nodeType() const { return QDomNode::BaseNode; } - virtual void save(QTextStream &, int, int) const; + void saveSubTree(const QDomNodePrivate *n, QTextStream &s, int depth, int indent) const; + virtual void save(QTextStream &, int, int) const {} + virtual void afterSave(QTextStream &, int, int) const {} void setLocation(int lineNumber, int columnNumber); @@ -328,6 +330,7 @@ QDomNode::NodeType nodeType() const override { return QDomNode::ElementNode; } QDomNodePrivate *cloneNode(bool deep = true) override; virtual void save(QTextStream &s, int, int) const override; + virtual void afterSave(QTextStream &s, int, int) const override; // Variables QDomNamedNodeMapPrivate *m_attr; diff --git a/tests/auto/xml/dom/qdom/testdata/deeply-nested.svg b/tests/auto/xml/dom/qdom/testdata/deeply-nested.svg new file mode 100644 index 0000000..85696a3 --- /dev/null +++ b/tests/auto/xml/dom/qdom/testdata/deeply-nested.svg @@ -0,0 +1,396 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/auto/xml/dom/qdom/tst_qdom.cpp b/tests/auto/xml/dom/qdom/tst_qdom.cpp index 3a6fc13..1501b12 100644 --- a/tests/auto/xml/dom/qdom/tst_qdom.cpp +++ b/tests/auto/xml/dom/qdom/tst_qdom.cpp @@ -114,6 +114,7 @@ void QTBUG49113_dontCrashWithNegativeIndex() const; void standalone(); void splitTextLeakMemory() const; + void noCrashOnDeepNesting() const; void cleanupTestCase() const; @@ -2337,5 +2338,21 @@ QCOMPARE(end.impl->ref.loadRelaxed(), 2); } +// The fix of QTBUG-131151 crash +void tst_QDom::noCrashOnDeepNesting() const +{ + const QString prefix = QFINDTESTDATA("testdata/"); + if (prefix.isEmpty()) + QFAIL("Cannot find testdata directory!"); + + QFile file(prefix + "/deeply-nested.svg"); + QVERIFY(file.open(QIODevice::ReadOnly)); + QDomDocument doc; + doc.setContent(&file); + QByteArray array = doc.toByteArray(); + QVERIFY(array.size()); + file.close(); +} + QTEST_MAIN(tst_QDom) #include "tst_qdom.moc"