From db4c8426225218c94a925401daed1a121796fa8c Mon Sep 17 00:00:00 2001 From: Hatem ElKharashy Date: Wed, 22 Apr 2026 08:57:44 +0300 Subject: [PATCH] Fix resolving paint servers going through stack overflow The parsing function resolvePaintServers() goes through the whole node structure and calls itself recursively to resolve paint servers which is not needed. Instead, store the styles that need to be resolved and go through them. This avoids causing a stack overflow. This branch still contained calls to the function structureNode->styleProperty(). Those were already replaced in 6220bd28997686804f613f824216ef64c1974679 which was not picked back to stable branches. To resolve the merge conflicts and also fix the behavior, we needed to insert lines which remove trailing hash marks from IDs, as structureNode->styleProperty() previously did. In the dev branch, those are not needed anymore because 56b58677bafb0941754f0d6c807d0fe87d87d5c1 refactored the handling of all IDs. Task-number: QTBUG-145916 Change-Id: Ie225bb51461f051b70b395f985b03006ccf9415c Reviewed-by: Robert Löhning (cherry picked from commit b702c4b3ac0fea5758452ef98de5d8b87ccb0c27) Reviewed-by: Hatem ElKharashy (cherry picked from commit 50e0a86bd6117a8cadfafc9e03f6baa633fa43b5) (cherry picked from commit 485d2450a785701e6bc7bfb4a4b678f6fc062b53) --- diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp index 0ac1d82..f113131 100644 --- a/src/svg/qsvghandler.cpp +++ b/src/svg/qsvghandler.cpp @@ -1053,7 +1053,7 @@ } else { QString id = idFromUrl(value); prop->setPaintStyleId(id); - prop->setPaintStyleResolved(false); + handler->pushUnresolvedStyle(prop); } } else if (attributes.fill != QLatin1String("none")) { QColor color; @@ -1223,7 +1223,7 @@ } else { QString id = idFromUrl(value); prop->setPaintStyleId(id); - prop->setPaintStyleResolved(false); + handler->pushUnresolvedStyle(prop); } } else if (attributes.stroke != QLatin1String("none")) { QColor color; @@ -4736,7 +4736,7 @@ break; } } - resolvePaintServers(m_doc); + resolvePaintServers(); resolveNodes(); if (detectCyclesAndWarn(m_doc)) { delete m_doc; @@ -4969,33 +4969,27 @@ return ((localName == QLatin1String("svg")) && (node != Doc)); } -void QSvgHandler::resolvePaintServers(QSvgNode *node, int nestedDepth) +void QSvgHandler::resolvePaintServers() { - if (!node || (node->type() != QSvgNode::Doc && node->type() != QSvgNode::Group - && node->type() != QSvgNode::Defs && node->type() != QSvgNode::Switch)) { - return; - } - - QSvgStructureNode *structureNode = static_cast(node); - - const QList ren = structureNode->renderers(); - for (auto it = ren.begin(); it != ren.end(); ++it) { - QSvgFillStyle *fill = static_cast((*it)->styleProperty(QSvgStyleProperty::FILL)); - if (fill && !fill->isPaintStyleResolved()) { + for (QSvgStyleProperty *prop : std::as_const(m_unresolvedStyles)) { + if (prop->type() == QSvgStyleProperty::FILL) { + QSvgFillStyle *fill = static_cast(prop); QString id = fill->paintStyleId(); - QSvgPaintStyleProperty *style = structureNode->styleProperty(id); + if (id.startsWith(QLatin1Char('#'))) + id.slice(1); + QSvgPaintStyleProperty *style = m_doc->namedStyle(id); if (style) { fill->setFillStyle(style); } else { qCWarning(lcSvgHandler, "%s", msgCouldNotResolveProperty(id, xml).constData()); fill->setBrush(Qt::NoBrush); } - } - - QSvgStrokeStyle *stroke = static_cast((*it)->styleProperty(QSvgStyleProperty::STROKE)); - if (stroke && !stroke->isPaintStyleResolved()) { + } else if (prop->type() == QSvgStyleProperty::STROKE) { + QSvgStrokeStyle *stroke = static_cast(prop); QString id = stroke->paintStyleId(); - QSvgPaintStyleProperty *style = structureNode->styleProperty(id); + if (id.startsWith(QLatin1Char('#'))) + id.slice(1); + QSvgPaintStyleProperty *style = m_doc->namedStyle(id); if (style) { stroke->setStyle(style); } else { @@ -5003,10 +4997,9 @@ stroke->setStroke(Qt::NoBrush); } } - - if (nestedDepth < 2048) - resolvePaintServers(*it, nestedDepth + 1); } + + m_unresolvedStyles.clear(); } void QSvgHandler::resolveNodes() @@ -5122,6 +5115,11 @@ return QColor(0, 0, 0); } +void QSvgHandler::pushUnresolvedStyle(QSvgStyleProperty *prop) +{ + m_unresolvedStyles.append(prop); +} + #ifndef QT_NO_CSSPARSER void QSvgHandler::setInStyle(bool b) diff --git a/src/svg/qsvghandler_p.h b/src/svg/qsvghandler_p.h index d98d7ca..5787bd5 100644 --- a/src/svg/qsvghandler_p.h +++ b/src/svg/qsvghandler_p.h @@ -82,6 +82,8 @@ void popColor(); QColor currentColor() const; + void pushUnresolvedStyle(QSvgStyleProperty *prop); + #ifndef QT_NO_CSSPARSER void setInStyle(bool b); bool inStyle() const; @@ -118,6 +120,7 @@ // - nodes which haven't been resolved yet. // - nodes to be checked for unsupported filter primitives. QList m_toBeResolved; + QList m_unresolvedStyles; enum CurrentNode { @@ -150,7 +153,7 @@ QCss::Parser m_cssParser; #endif void parse(); - void resolvePaintServers(QSvgNode *node, int nestedDepth = 0); + void resolvePaintServers(); void resolveNodes(); QPen m_defaultPen; diff --git a/src/svg/qsvgstyle.cpp b/src/svg/qsvgstyle.cpp index a49c4da..9b9a38d 100644 --- a/src/svg/qsvgstyle.cpp +++ b/src/svg/qsvgstyle.cpp @@ -102,7 +102,6 @@ , m_oldFillRule(Qt::WindingFill) , m_fillOpacity(1.0) , m_oldFillOpacity(0) - , m_paintStyleResolved(1) , m_fillRuleSet(0) , m_fillOpacitySet(0) , m_fillSet(0) @@ -269,7 +268,6 @@ , m_strokeDashOffset(0) , m_oldStrokeDashOffset(0) , m_style(0) - , m_paintStyleResolved(1) , m_vectorEffect(0) , m_oldVectorEffect(0) , m_strokeSet(0) diff --git a/src/svg/qsvgstyle_p.h b/src/svg/qsvgstyle_p.h index 95dc5ed..594fbd7 100644 --- a/src/svg/qsvgstyle_p.h +++ b/src/svg/qsvgstyle_p.h @@ -268,16 +268,6 @@ return m_paintStyleId; } - void setPaintStyleResolved(bool resolved) - { - m_paintStyleResolved = resolved; - } - - bool isPaintStyleResolved() const - { - return m_paintStyleResolved; - } - private: // fill v v 'inherit' | // fill-opacity v v 'inherit' | @@ -291,7 +281,6 @@ qreal m_oldFillOpacity; QString m_paintStyleId; - uint m_paintStyleResolved : 1; uint m_fillRuleSet : 1; uint m_fillOpacitySet : 1; @@ -498,16 +487,6 @@ return m_paintStyleId; } - void setPaintStyleResolved(bool resolved) - { - m_paintStyleResolved = resolved; - } - - bool isPaintStyleResolved() const - { - return m_paintStyleResolved; - } - QPen stroke() const { return m_stroke; @@ -531,7 +510,6 @@ QSvgRefCounter m_style; QString m_paintStyleId; - uint m_paintStyleResolved : 1; uint m_vectorEffect : 1; uint m_oldVectorEffect : 1; diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp index 20b38c8..56379cb 100644 --- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp @@ -167,7 +167,8 @@ void tst_QSvgRenderer::inexistentUrl() { - const char *src = ""; + const char *src = ""; + QTest::ignoreMessage(QtWarningMsg, ":1:66: Could not resolve property: inexistent"); QByteArray data(src); QSvgRenderer renderer(data); @@ -2165,7 +2166,7 @@ QTest::ignoreMessage(QtWarningMsg, ":2:68: Could not add child element to parent " "element because the types are incorrect."); - QTest::ignoreMessage(QtWarningMsg, ":4:28: Could not resolve property: #ptn"); + QTest::ignoreMessage(QtWarningMsg, ":4:28: Could not resolve property: ptn"); QSvgRenderer renderer(svg); QPainter painter(&image);