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

Author: ludviggunne

gui/test/data/xmlfiles/xmlreport_v2.xml

@@ -2,10 +2,10 @@
 <results version="2">
     <cppcheck version="1.47"/>
     <errors>
-        <error id="unreadVariable" severity="style" msg="Variable &amp;#039;a&amp;#039; is assigned a value that is never used" verbose="Variable &amp;#039;a&amp;#039; is assigned a value that is never used">
+        <error id="unreadVariable" severity="style" msg="Variable 'a' is assigned a value that is never used" verbose="Variable 'a' is assigned a value that is never used">
             <location file="test.cxx" line="11"/>
         </error>
-        <error id="unreadVariable" severity="style" msg="Variable &amp;#039;b&amp;#039; is assigned a value that is never used" verbose="Variable &amp;#039;b&amp;#039; is assigned a value that is never used">
+        <error id="unreadVariable" severity="style" msg="Variable 'b' is assigned a value that is never used" verbose="Variable 'b' is assigned a value that is never used">
             <location file="test.cxx" line="28"/>
         </error>
         <error id="memleak" severity="error" msg="Memory leak: b" verbose="Memory leak: b">
@@ -15,7 +15,7 @@
             <location file="test.cxx" line="32"/>
             <location file="test.cxx" line="16"/>
         </error>
-        <error id="obsoleteFunctionsgets" severity="style" msg="Found obsolescent function &amp;#039;gets&amp;#039;. It is recommended to use the function &amp;#039;fgets&amp;#039; instead" verbose="Found obsolescent function &amp;#039;gets&amp;#039;. With gets you&amp;#039;ll get buffer overruns if the input data too big for the buffer. It is recommended to use the function &amp;#039;fgets&amp;#039; instead.">
+        <error id="obsoleteFunctionsgets" severity="style" msg="Found obsolescent function 'gets'. It is recommended to use the function 'fgets' instead" verbose="Found obsolescent function 'gets'. With gets you'll get buffer overruns if the input data too big for the buffer. It is recommended to use the function 'fgets' instead.">
             <location file="test.cxx" line="31"/>
         </error>
         <error id="postfixOperator" severity="performance" msg="Prefer prefix ++/-- operators for non-primitive types." verbose="Prefix ++/-- operators should be preferred for non-primitive types. Pre-increment/decrement can be more efficient than post-increment/decrement. Post-increment/decrement usually involves keeping a copy of the previous value around and adds a little extra code.">

gui/test/resultstree/testresultstree.cpp

@@ -106,12 +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;
-}
 XmlReport::XmlReport(const QString& filename) : Report(filename) {}
 // NOLINTNEXTLINE(performance-unnecessary-value-param)
 void ThreadResult::finishCheck(CheckThread::Details /*unused*/) {

gui/test/xmlreportv2/testxmlreportv2.cpp

@@ -21,7 +21,9 @@
 #include "erroritem.h"
 #include "xmlreportv2.h"
 
+#include <QFile>
 #include <QList>
+#include <QTemporaryDir>
 #include <QtTest>
 
 void TestXmlReportV2::readXml() const
@@ -53,4 +55,39 @@ void TestXmlReportV2::readXml() const
     QCOMPARE(item2.message, QString("Mismatching allocation and deallocation: k"));
 }
 
+void TestXmlReportV2::writeXmlEscaping() const
+{
+    QTemporaryDir dir;
+    const QString path = dir.path() + "/results.xml";
+
+    ErrorItem item;
+    item.errorId = "id";
+    item.severity = Severity::style;
+    item.summary = "\"x\" & <y> &quot;";
+    item.message = item.summary;
+    QErrorPathItem loc;
+    loc.file = "a&b.cpp";
+    loc.line = 1;
+    item.errorPath << loc;
+    {
+        XmlReportV2 report(path, QString());
+        QVERIFY(report.create());
+        report.writeHeader();
+        report.writeError(item);
+        report.writeFooter();
+    }
+
+    QFile file(path);
+    QVERIFY(file.open(QIODevice::ReadOnly));
+    QVERIFY(file.readAll().contains("msg=\"&quot;x&quot; &amp; &lt;y&gt; &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[0].file, loc.file);
+}
+
 QTEST_MAIN(TestXmlReportV2)

gui/test/xmlreportv2/testxmlreportv2.h

@@ -23,4 +23,5 @@ class TestXmlReportV2 : public QObject {
 
 private slots:
     void readXml() const;
+    void writeXmlEscaping() const;
 };

gui/xmlreport.cpp

@@ -32,28 +32,6 @@ XmlReport::XmlReport(const QString &filename) :
     Report(filename)
 {}
 
-QString XmlReport::quoteMessage(const QString &message)
-{
-    QString quotedMessage(message);
-    quotedMessage.replace("&", "&");
-    quotedMessage.replace("\"", """);
-    quotedMessage.replace("'", "'");
-    quotedMessage.replace("<", "&lt;");
-    quotedMessage.replace(">", "&gt;");
-    return quotedMessage;
-}
-
-QString XmlReport::unquoteMessage(const QString &message)
-{
-    QString quotedMessage(message);
-    quotedMessage.replace("&amp;", "&");
-    quotedMessage.replace("&quot;", "\"");
-    quotedMessage.replace("&#039;", "'");
-    quotedMessage.replace("&lt;", "<");
-    quotedMessage.replace("&gt;", ">");
-    return quotedMessage;
-}
-
 int XmlReport::determineVersion(const QString &filename)
 {
     QFile file;

gui/xmlreport.h

@@ -42,20 +42,6 @@ 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.
-     */
-    static QString unquoteMessage(const QString &message);
-
     /**
      * @brief Get the XML report format version from the file.
      * @param filename Filename of the report file.

gui/xmlreportv2.cpp

@@ -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);
+    mXmlWriter->writeAttribute(VerboseAttribute, error.message);
     if (error.inconclusive)
         mXmlWriter->writeAttribute(InconclusiveAttribute, "true");
     if (!error.remark.isEmpty())
@@ -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())
@@ -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();
     }
@@ -228,10 +226,8 @@ ErrorItem XmlReportV2::readError(const QXmlStreamReader *reader)
         QXmlStreamAttributes attribs = reader->attributes();
         item.errorId = attribs.value(QString(), IdAttribute).toString();
         item.severity = GuiSeverity::fromString(attribs.value(QString(), SeverityAttribute).toString());
-        const QString summary = attribs.value(QString(), MsgAttribute).toString();
-        item.summary = XmlReport::unquoteMessage(summary);
-        const QString message = attribs.value(QString(), VerboseAttribute).toString();
-        item.message = XmlReport::unquoteMessage(message);
+        item.summary = attribs.value(QString(), MsgAttribute).toString();
+        item.message = attribs.value(QString(), VerboseAttribute).toString();
         if (attribs.hasAttribute(QString(), InconclusiveAttribute))
             item.inconclusive = true;
         if (attribs.hasAttribute(QString(), RemarkAttribute))
@@ -258,14 +254,14 @@ ErrorItem XmlReportV2::readError(const QXmlStreamReader *reader)
                 QXmlStreamAttributes attribs = mXmlReader->attributes();
                 QString file0 = attribs.value(QString(), IncludedFromFilenameAttribute).toString();
                 if (!file0.isEmpty())
-                    item.file0 = XmlReport::unquoteMessage(file0);
+                    item.file0 = file0;
                 QErrorPathItem loc;
-                loc.file = XmlReport::unquoteMessage(attribs.value(QString(), FilenameAttribute).toString());
+                loc.file = attribs.value(QString(), FilenameAttribute).toString();
                 loc.line = attribs.value(QString(), LineAttribute).toString().toUInt();
                 if (attribs.hasAttribute(QString(), ColumnAttribute))
                     loc.column = attribs.value(QString(), ColumnAttribute).toString().toInt();
                 if (attribs.hasAttribute(QString(), InfoAttribute))
-                    loc.info = XmlReport::unquoteMessage(attribs.value(QString(), InfoAttribute).toString());
+                    loc.info = attribs.value(QString(), InfoAttribute).toString();
                 item.errorPath.push_front(loc);
             }
             break;