Skip to content

Commit

Permalink
QDomDocument::toByteArray() crashed in case of high XML nesting level
Browse files Browse the repository at this point in the history
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
Pick-to: 6.8
Change-Id: Ib74aaef1422716f2aafcb89dfc8c05ef334e2a54
Reviewed-by: Thiago Macieira <[email protected]>
  • Loading branch information
qt-tatiana committed Dec 5, 2024
1 parent 7b00975 commit 387633a
Show file tree
Hide file tree
Showing 4 changed files with 470 additions and 18 deletions.
70 changes: 53 additions & 17 deletions src/xml/dom/qdom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1349,16 +1349,40 @@ void QDomNodePrivate::normalize()
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)
Expand Down Expand Up @@ -2146,7 +2170,7 @@ void QDomNode::save(QTextStream& stream, int indent, EncodingPolicy encodingPoli
if (isDocument())
static_cast<const QDomDocumentPrivate *>(impl)->saveDocument(stream, indent, encodingPolicy);
else
IMPL->save(stream, 1, indent);
IMPL->saveSubTree(IMPL, stream, 1, indent);
}

/*!
Expand Down Expand Up @@ -3064,11 +3088,11 @@ void QDomDocumentTypePrivate::save(QTextStream& s, int, int indent) const

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 << ']';
}
Expand Down Expand Up @@ -4121,13 +4145,25 @@ void QDomElementPrivate::save(QTextStream& s, int depth, int indent) const
if (indent != -1)
s << Qt::endl;
}
QDomNodePrivate::save(s, depth + 1, indent); if (!last->isText())
} 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 << "</" << qName << '>';
} else {
s << "/>";
}

if (!(next && next->isText())) {
/* -1 disables new lines. */
if (indent != -1)
Expand Down Expand Up @@ -5908,7 +5944,7 @@ void QDomDocumentPrivate::saveDocument(QTextStream& s, const int indent, QDomNod
type->save(s, 0, indent);
doc = true;
}
n->save(s, 0, indent);
n->saveSubTree(n, s, 0, indent);
n = n->next;
}
}
Expand All @@ -5935,8 +5971,8 @@ void QDomDocumentPrivate::saveDocument(QTextStream& s, const int indent, QDomNod
}

// 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;
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/xml/dom/qdom_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ class QDomNodePrivate

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);

Expand Down Expand Up @@ -328,6 +330,7 @@ class QDomElementPrivate : public QDomNodePrivate
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;
Expand Down
Loading

0 comments on commit 387633a

Please sign in to comment.