Skip to content

Fix #15107: Exporting XML via GUI escapes special characters twice. #8943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ludviggunne wants to merge 2 commits into
base: main
Choose a base branch
from
+59 −30
Open

Fix #15107: Exporting XML via GUI escapes special characters twice. #8943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83cbae4
test
ludviggunne Oct 15, 2026
54b2dde
fix
ludviggunne Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions gui/test/resultstree/testresultstree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,6 @@ Application& ApplicationList::getApplication(const int /*unused*/) {
const Application& ApplicationList::getApplication(const int index) const {
return mApplications.at(index);
}
QString XmlReport::quoteMessage(const QString &message) {
return message;
}
QString XmlReport::unquoteMessage(const QString &message) {
return message;
}
Expand Down
50 changes: 50 additions & 0 deletions gui/test/xmlreportv2/testxmlreportv2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
#include "erroritem.h"
#include "xmlreportv2.h"

#include <QFile>
#include <QList>
#include <QTemporaryDir>
#include <QtTest>

void TestXmlReportV2::readXml() const
Expand Down Expand Up @@ -53,4 +55,52 @@ void TestXmlReportV2::readXml() const
QCOMPARE(item2.message, QString("Mismatching allocation and deallocation: k"));
}

void TestXmlReportV2::writeXmlEscaping() const
{
QTemporaryDir tmpdir;
const QString path = tmpdir.path() + "/results.xml";

ErrorItem item;
item.errorId = "unusedVariable";
item.severity = Severity::style;
item.summary = "Unused variable: \"x\" & <y>";
item.message = item.summary;

@dmcppcheck dmcppcheck Oct 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

The round-trip check passes only because the string has no literal entity. With e.g. item.summary = "x &lt; y", the QCOMPARE(errors[0].summary, item.summary) below would fail, because unquoteMessage turns the text that was read into x < y.


QErrorPathItem loc;
loc.file = "test.cpp";
loc.line = 1;
loc.column = 1;
loc.info = item.summary;
item.errorPath << loc;
loc.line = 2;
item.errorPath << loc;

{
XmlReportV2 report(path, QString());
QVERIFY(report.create());
report.writeHeader();
report.writeError(item);
report.writeFooter();
}

// Each special character must be escaped exactly once
QFile file(path);
QVERIFY(file.open(QIODevice::ReadOnly));

const QString xml = QString::fromUtf8(file.readAll());
QVERIFY(xml.contains("msg=\"Unused variable: &quot;x&quot; &amp; &lt;y&gt;\""));
QVERIFY(xml.contains("info=\"Unused variable: &quot;x&quot; &amp; &lt;y&gt;\""));
QVERIFY(!xml.contains("&amp;quot;"));

XmlReportV2 report(path, QString());
QVERIFY(report.open());

const QList<ErrorItem> errors = report.read();
QCOMPARE(errors.size(), 1);
QCOMPARE(errors[0].summary, item.summary);
QCOMPARE(errors[0].message, item.message);
QCOMPARE(errors[0].errorPath.size(), 2);
QCOMPARE(errors[0].errorPath[0].info, item.summary);
}

QTEST_MAIN(TestXmlReportV2)
1 change: 1 addition & 0 deletions gui/test/xmlreportv2/testxmlreportv2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ class TestXmlReportV2 : public QObject {

private slots:
void readXml() const;
void writeXmlEscaping() const;
};
11 changes: 0 additions & 11 deletions gui/xmlreport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,6 @@ XmlReport::XmlReport(const QString &filename) :
Report(filename)
{}

QString XmlReport::quoteMessage(const QString &message)
{
QString quotedMessage(message);
quotedMessage.replace("&", "&amp;");
quotedMessage.replace("\"", "&quot;");
quotedMessage.replace("'", "&#039;");
quotedMessage.replace("<", "&lt;");
quotedMessage.replace(">", "&gt;");
return quotedMessage;
}

QString XmlReport::unquoteMessage(const QString &message)
{
QString quotedMessage(message);
Expand Down
12 changes: 3 additions & 9 deletions gui/xmlreport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,11 @@ class XmlReport : public Report {
*/
virtual QList<ErrorItem> read() = 0;

/**
* @brief Quote the message.
* @param message Message to quote.
* @return quoted message.
*/
static QString quoteMessage(const QString &message);

/**
* @brief Unquote the message.
* @param message Message to quote.
* @return quoted message.
* For older reports where special characters have been escaped twice (#15107).

@dmcppcheck dmcppcheck Oct 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

This comment says the function is for older reports, but readError() calls it on every file. That includes files written after this PR and the XML that the CLI writes, which are escaped only once.

* @param message Message to unquote.
* @return unquoted message.
*/
static QString unquoteMessage(const QString &message);

Expand Down
12 changes: 5 additions & 7 deletions gui/xmlreportv2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,8 @@ void XmlReportV2::writeError(const ErrorItem &error)

// Don't localize severity so we can read these files
mXmlWriter->writeAttribute(SeverityAttribute, GuiSeverity::toString(error.severity));
const QString summary = XmlReport::quoteMessage(error.summary);
mXmlWriter->writeAttribute(MsgAttribute, summary);
const QString message = XmlReport::quoteMessage(error.message);
mXmlWriter->writeAttribute(VerboseAttribute, message);
mXmlWriter->writeAttribute(MsgAttribute, error.summary);

@dmcppcheck dmcppcheck Oct 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

Now that the attribute is escaped only once, the reader at line 230 still calls XmlReport::unquoteMessage() on the value that QXmlStreamReader has already decoded. Literal entities in a message are lost on save/load. For example, cppcheck reports Condition 'strcmp(s,"&lt;")==0' is always true; it is written as &amp;lt;, read back as &lt; and then unquoted to <. Before this PR the round trip was correct. The same applies to file, file0 and info (lines 259/261/266). Fallback matching in ErrorItem::same() uses message and errorPath, so updateFromOldReport() on lastResults.xml can also stop matching these items. Old double-escaped files should be detected, for example from a marker or version in the header, and only those should be unquoted.

mXmlWriter->writeAttribute(VerboseAttribute, error.message);
if (error.inconclusive)
mXmlWriter->writeAttribute(InconclusiveAttribute, "true");
if (!error.remark.isEmpty())
Expand All @@ -145,7 +143,7 @@ void XmlReportV2::writeError(const ErrorItem &error)
if (error.hash > 0)
mXmlWriter->writeAttribute(HashAttribute, QString::number(error.hash));
if (!error.file0.isEmpty())
mXmlWriter->writeAttribute(IncludedFromFilenameAttribute, quoteMessage(error.file0));
mXmlWriter->writeAttribute(IncludedFromFilenameAttribute, error.file0);
if (!error.sinceDate.isEmpty())
mXmlWriter->writeAttribute(SinceDateAttribute, error.sinceDate);
if (!error.tags.isEmpty())
Expand All @@ -155,12 +153,12 @@ void XmlReportV2::writeError(const ErrorItem &error)
mXmlWriter->writeStartElement(LocationElementName);

QString file = QDir::toNativeSeparators(error.errorPath[i].file);
mXmlWriter->writeAttribute(FilenameAttribute, XmlReport::quoteMessage(file));
mXmlWriter->writeAttribute(FilenameAttribute, file);
mXmlWriter->writeAttribute(LineAttribute, QString::number(error.errorPath[i].line));
if (error.errorPath[i].column > 0)
mXmlWriter->writeAttribute(ColumnAttribute, QString::number(error.errorPath[i].column));
if (error.errorPath.count() > 1)
mXmlWriter->writeAttribute(InfoAttribute, XmlReport::quoteMessage(error.errorPath[i].info));
mXmlWriter->writeAttribute(InfoAttribute, error.errorPath[i].info);

mXmlWriter->writeEndElement();
}
Expand Down
Loading