From b369dc6021e83e70b95d04f886cfda4dfcf56c45 Mon Sep 17 00:00:00 2001
From: Oliver Eftevaag <oliver.eftevaag@qt.io>
Date: Wed, 15 Dec 2021 15:10:00 +0100
Subject: [PATCH] QTextHtmlParser: fix prefix lookahead and html comments

The hasPrefix() function would only use the second 'lookahead' parameter
to check if there was more unparsed text after the current character.
When it's obvious from the codebase that it should actually look ahead
of the current character being processed, and compare againt that future
character.

Html comments were also not handled quite right. Partially because of
the broken hasPrefix() function, but also because it would advance the
current index tracker by 3 instead of 2. Remember that the beginning of
an html comment is <!-- meaning that there are only supposed to be 2
dashes required, not 3. The result would be that something like this
<!----> would not automatically close, because the current index tracker
would jump over the first 3 dashes when it begins a comment, and the
remaining unprocessed string would be ->

Also, because of the broken lookahead in hasPrefix(), a comment could
actually be started with just <!- not requiring a second dash at all.

Pick-to: 6.3 6.2 5.15
Fixes: QTBUG-99147
Change-Id: I8f4d4a1107eaf2dae16d16b7b860525d45a1c474
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
---
 src/gui/text/qtexthtmlparser.cpp              |  6 +--
 src/gui/text/qtexthtmlparser_p.h              |  4 +-
 .../text/qtextdocument/tst_qtextdocument.cpp  | 41 +++++++++++++++++++
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/src/gui/text/qtexthtmlparser.cpp b/src/gui/text/qtexthtmlparser.cpp
index 85877bfe475..f97b8a672a3 100644
--- a/src/gui/text/qtexthtmlparser.cpp
+++ b/src/gui/text/qtexthtmlparser.cpp
@@ -785,8 +785,8 @@ void QTextHtmlParser::parseCloseTag()
 void QTextHtmlParser::parseExclamationTag()
 {
     ++pos;
-    if (hasPrefix(QLatin1Char('-'),1) && hasPrefix(QLatin1Char('-'),2)) {
-        pos += 3;
+    if (hasPrefix(QLatin1Char('-')) && hasPrefix(QLatin1Char('-'), 1)) {
+        pos += 2;
         // eat comments
         int end = txt.indexOf(QLatin1String("-->"), pos);
         pos = (end >= 0 ? end + 3 : len);
@@ -882,7 +882,7 @@ QString QTextHtmlParser::parseWord()
         while (pos < len) {
             QChar c = txt.at(pos++);
             if (c == QLatin1Char('>')
-                || (c == QLatin1Char('/') && hasPrefix(QLatin1Char('>'), 1))
+                || (c == QLatin1Char('/') && hasPrefix(QLatin1Char('>')))
                 || c == QLatin1Char('<')
                 || c == QLatin1Char('=')
                 || c.isSpace()) {
diff --git a/src/gui/text/qtexthtmlparser_p.h b/src/gui/text/qtexthtmlparser_p.h
index 06bbc032a83..a96bf3244f7 100644
--- a/src/gui/text/qtexthtmlparser_p.h
+++ b/src/gui/text/qtexthtmlparser_p.h
@@ -333,7 +333,9 @@ protected:
     void applyAttributes(const QStringList &attributes);
     void eatSpace();
     inline bool hasPrefix(QChar c, int lookahead = 0) const
-        {return pos + lookahead < len && txt.at(pos) == c; }
+    {
+        return pos + lookahead < len && txt.at(pos + lookahead) == c;
+    }
     int margin(int i, int mar) const;
 
     bool nodeIsChildOf(int i, QTextHTMLElements id) const;
diff --git a/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp b/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp
index a38defa6562..bb2f3750aa0 100644
--- a/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp
+++ b/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp
@@ -202,6 +202,9 @@ private slots:
     void contentsChangeIndices_data();
     void contentsChangeIndices();
 
+    void insertHtmlWithComments_data();
+    void insertHtmlWithComments();
+
 private:
     void backgroundImage_checkExpectedHtml(const QTextDocument &doc);
     void buildRegExpData();
@@ -3917,6 +3920,44 @@ void tst_QTextDocument::contentsChangeIndices()
     QCOMPARE(changeAdded - changeRemoved, 1);
 }
 
+void tst_QTextDocument::insertHtmlWithComments_data()
+{
+    QTest::addColumn<QString>("html");
+    QTest::addColumn<QStringList>("expectedBlocks");
+
+    QTest::newRow("commentless") << "<p>first</p><p>second</p><p>third</p>"
+                                 << QStringList { "first", "second", "third" };
+    QTest::newRow("normal") << "<p>first</p><!--<p>second</p>--><p>third</p>"
+                            << QStringList { "first", "third" };
+    QTest::newRow("nonClosing") << "<p>first</p><!--<p>second</p><p>third</p>"
+                                << QStringList { "first" };
+    QTest::newRow("immediatelyClosing") << "<p>first</p><!----><p>second</p><p>third</p>"
+                                        << QStringList { "first", "second", "third" };
+    QTest::newRow("fake") << "<p>first</p><!-<p>second</p><p>third</p>"
+                          << QStringList { "first", "second", "third" };
+    QTest::newRow("endingNonExistant") << "<p>first</p>--><p>second</p><p>third</p>"
+                                       << QStringList { "first", "-->", "second", "third" };
+}
+
+void tst_QTextDocument::insertHtmlWithComments()
+{
+    QFETCH(QString, html);
+    QFETCH(QStringList, expectedBlocks);
+
+    QTextDocument doc;
+    doc.setHtml(html);
+
+    QCOMPARE(doc.blockCount(), expectedBlocks.count());
+
+    QStringList blockContent;
+    auto currentBlock = doc.begin();
+    while (currentBlock != doc.end()) {
+        blockContent.append(currentBlock.text());
+        currentBlock = currentBlock.next();
+    }
+
+    QCOMPARE(blockContent, expectedBlocks);
+}
 
 QTEST_MAIN(tst_QTextDocument)
 #include "tst_qtextdocument.moc"