Refactor LinkParser (#4576)

CHANGELOG.md

@@ -26,6 +26,7 @@
 - Bugfix: Fixed a memory leak that occurred when loading message history. This was mostly noticeable with unstable internet connections where reconnections were frequent or long-running instances of Chatterino. (#4499)
 - Bugfix: Fixed Twitch channel-specific filters not being applied correctly. (#4529)
 - Bugfix: Fixed emote & badge tooltips not showing up when thumbnails were hidden. (#4509)
+- Bugfix: Fixed links with invalid IPv4 addresses being parsed. (#4576)
 - Dev: Disabling precompiled headers on Windows is now tested in CI. (#4472)
 - Dev: Themes are now stored as JSON files in `resources/themes`. (#4471, #4533)
 - Dev: Ignore unhandled BTTV user-events. (#4438)

src/common/LinkParser.cpp

@@ -1,19 +1,18 @@
+#define QT_NO_CAST_FROM_ASCII  // avoids unexpected implicit casts
 #include "common/LinkParser.hpp"
 
 #include <QFile>
-#include <QMap>
-#include <QRegularExpression>
 #include <QSet>
 #include <QString>
-#include <QStringRef>
+#include <QStringView>
 #include <QTextStream>
 
-namespace chatterino {
 namespace {
-    QSet<QString> &tlds()
+
-    {
+QSet<QString> &tlds()
+{
     static QSet<QString> tlds = [] {
-            QFile file(":/tlds.txt");
+        QFile file(QStringLiteral(":/tlds.txt"));
         file.open(QFile::ReadOnly);
         QTextStream stream(&file);
 
@@ -32,175 +31,202 @@ namespace {
             set.insert(line);
 
             if (safetyMax-- == 0)
+            {
                 break;
             }
+        }
 
         return set;
     }();
     return tlds;
-    }
+}
 
-    bool isValidHostname(QStringRef &host)
+bool isValidTld(QStringView tld)
+{
+    return tlds().contains(tld.toString().toLower());
+}
+
+bool isValidIpv4(QStringView host)
+{
+    // We don't care about the actual value,
+    // we only want to verify the ip.
+
+    char16_t sectionValue = 0;  // 0..256
+    uint8_t octetNumber = 0;    // 0..4
+    uint8_t sectionDigits = 0;  // 0..3
+    bool lastWasDot = true;
+
+    for (auto c : host)
     {
-        int index = host.lastIndexOf('.');
+        char16_t current = c.unicode();
-
+        if (current == '.')
-        return index != -1 &&
-               tlds().contains(host.mid(index + 1).toString().toLower());
-    }
-
-    bool isValidIpv4(QStringRef &host)
         {
-        static auto exp = QRegularExpression("^\\d{1,3}(?:\\.\\d{1,3}){3}$");
+            if (lastWasDot || octetNumber == 3)
-
-        return exp.match(host).hasMatch();
-    }
-
-#ifdef C_MATCH_IPV6_LINK
-    bool isValidIpv6(QStringRef &host)
             {
-        static auto exp = QRegularExpression("^\\[[a-fA-F0-9:%]+\\]$");
+                return false;
-
-        return exp.match(host).hasMatch();
             }
-#endif
+            lastWasDot = true;
+            octetNumber++;
+            sectionValue = 0;
+            sectionDigits = 0;
+            continue;
+        }
+        lastWasDot = false;
+
+        if (current > u'9' || current < u'0')
+        {
+            return false;
+        }
+
+        sectionValue = sectionValue * 10 + (current - u'0');
+        sectionDigits++;
+        if (sectionValue >= 256 || sectionDigits > 3)
+        {
+            return false;
+        }
+    }
+
+    return octetNumber == 3 && !lastWasDot;
+}
+
+/**
+ * @brief Checks if the string starts with a port number.
+ * 
+ * The value of the port number isn't checked. A port in this implementation
+ * can be in the range 0..100'000.
+ */
+bool startsWithPort(QStringView string)
+{
+    for (qsizetype i = 0; i < std::min<qsizetype>(5, string.length()); i++)
+    {
+        char16_t c = string[i].unicode();
+        if (i >= 1 && (c == u'/' || c == u'?' || c == u'#'))
+        {
+            return true;
+        }
+
+        if (!string[i].isDigit())
+        {
+            return false;
+        }
+    }
+    return true;
+}
+
 }  // namespace
 
+namespace chatterino {
+
 LinkParser::LinkParser(const QString &unparsedString)
 {
     ParsedLink result;
-
     // This is not implemented with a regex to increase performance.
-    // We keep removing parts of the url until there's either nothing left or we fail.
+    QStringView remaining(unparsedString);
-    QStringRef l(&unparsedString);
+    QStringView protocol(remaining);
 
-    bool hasHttp = false;
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+    QStringView wholeString(unparsedString);
+    const auto refFromView = [&](QStringView view) {
+        return QStringRef(&unparsedString,
+                          static_cast<int>(view.begin() - wholeString.begin()),
+                          static_cast<int>(view.size()));
+    };
+#endif
 
-    // Protocol `https?://`
+    // Check protocol for https?://
-    if (l.startsWith("https://", Qt::CaseInsensitive))
+    if (remaining.startsWith(QStringLiteral("http"), Qt::CaseInsensitive) &&
+        remaining.length() >= 4 + 3 + 1)  // 'http' + '://' + [any]
     {
-        hasHttp = true;
+        remaining = remaining.mid(4);  // 'http'
-        result.protocol = l.mid(0, 8);
+
-        l = l.mid(8);
+        if (remaining[0] == QChar(u's') || remaining[0] == QChar(u'S'))
-    }
-    else if (l.startsWith("http://", Qt::CaseInsensitive))
         {
-        hasHttp = true;
+            remaining = remaining.mid(1);
-        result.protocol = l.mid(0, 7);
-        l = l.mid(7);
         }
 
-    // Http basic auth `user:password`.
+        if (remaining.startsWith(QStringLiteral("://")))
-    // Not supported for security reasons (misleading links)
+        {
+            remaining = remaining.mid(3);
+#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
+            result.protocol = {protocol.begin(), remaining.begin()};
+#else
+            result.protocol =
+                refFromView({protocol.begin(), remaining.begin()});
+#endif
+        }
+    }
+
+    // Http basic auth `user:password` isn't supported for security reasons (misleading links)
 
     // Host `a.b.c.com`
-    QStringRef host = l;
+    QStringView host = remaining;
-    ParsedLink::StringView rest;
+    QStringView rest;
     bool lastWasDot = true;
-    bool inIpv6 = false;
+    int lastDotPos = -1;
-    bool hasMatch = false;
+    int nDots = 0;
 
-    for (int i = 0; i < l.size(); i++)
+    // Extract the host
+    for (int i = 0; i < remaining.size(); i++)
     {
-        if (l[i] == '.')
+        char16_t currentChar = remaining[i].unicode();
+        if (currentChar == u'.')
         {
-            if (lastWasDot == true)  // no double dots ..
+            if (lastWasDot)  // no double dots ..
-                goto error;
+            {
+                return;
+            }
+            lastDotPos = i;
             lastWasDot = true;
+            nDots++;
         }
         else
         {
             lastWasDot = false;
         }
 
-        if (l[i] == ':' && !inIpv6)
+        // found a port
+        if (currentChar == u':')
         {
-            host = l.mid(0, i);
+            host = remaining.mid(0, i);
-            rest = l.mid(i);
+            rest = remaining.mid(i);
-            l = l.mid(i + 1);
+            remaining = remaining.mid(i + 1);
-            goto parsePort;
+
-        }
+            if (!startsWithPort(remaining))
-        else if (l[i] == '/')
             {
-            host = l.mid(0, i);
+                return;
-            rest = l.mid(i);
-            l = l.mid(i + 1);
-            goto parsePath;
-        }
-        else if (l[i] == '?')
-        {
-            host = l.mid(0, i);
-            rest = l.mid(i);
-            l = l.mid(i + 1);
-            goto parseQuery;
-        }
-        else if (l[i] == '#')
-        {
-            host = l.mid(0, i);
-            rest = l.mid(i);
-            l = l.mid(i + 1);
-            goto parseAnchor;
             }
 
-        // ipv6
+            break;
-        if (l[i] == '[')
-        {
-            if (i == 0)
-                inIpv6 = true;
-            else
-                goto error;
-        }
-        else if (l[i] == ']')
-        {
-            inIpv6 = false;
-        }
         }
 
-    if (lastWasDot)
-        goto error;
-    else
-        goto done;
-
-parsePort:
-    // Port `:12345`
-    for (int i = 0; i < std::min<int>(5, l.size()); i++)
-    {
-        if (l[i] == '/')
-            goto parsePath;
-        else if (l[i] == '?')
-            goto parseQuery;
-        else if (l[i] == '#')
-            goto parseAnchor;
-
-        if (!l[i].isDigit())
-            goto error;
-    }
-
-    goto done;
-
-parsePath:
-parseQuery:
-parseAnchor:
         // we accept everything in the path/query/anchor
-
+        if (currentChar == u'/' || currentChar == u'?' || currentChar == u'#')
-done:
-    // check host
-    hasMatch = isValidHostname(host) || isValidIpv4(host)
-#ifdef C_MATCH_IPV6_LINK
-
-               || (hasHttp && isValidIpv6(host))
-#endif
-        ;
-
-    if (hasMatch)
         {
+            host = remaining.mid(0, i);
+            rest = remaining.mid(i);
+            break;
+        }
+    }
+
+    if (lastWasDot || lastDotPos <= 0)
+    {
+        return;
+    }
+
+    // check host/tld
+    if ((nDots == 3 && isValidIpv4(host)) ||
+        isValidTld(host.mid(lastDotPos + 1)))
+    {
+#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
         result.host = host;
         result.rest = rest;
+#else
+        result.host = refFromView(host);
+        result.rest = refFromView(rest);
+#endif
         result.source = unparsedString;
         this->result_ = std::move(result);
     }
-
-error:
-    return;
 }
 
 const std::optional<ParsedLink> &LinkParser::result() const

tests/src/LinkParser.cpp

@@ -80,11 +80,6 @@ TEST(LinkParser, parseIpv4Links)
         {"http://", "196.168.4.0", "#foo"},
         {"", "196.168.4.0", "/?#foo"},
         {"", "196.168.4.0", "#?/foo"},
-        {"", "256.255.255.255"},
-        {"http://", "256.255.255.255"},
-        {"", "255.256.255.255"},
-        {"", "255.255.256.255"},
-        {"", "255.255.255.256"},
         // test case-insensitiveness
         {"HTTP://", "196.168.4.0", "#Foo"},
         {"HTTPS://", "196.168.4.0", "#Foo"},
@@ -102,6 +97,8 @@ TEST(LinkParser, parseIpv4Links)
 TEST(LinkParser, doesntParseInvalidIpv4Links)
 {
     const QStringList inputs = {
+        // U+0660 - in category "number digits"
+        QStringLiteral("٠.٠.٠.٠"),
         "https://127.0.0.",
         "http://127.0.01",
         "127.0.0000.1",
@@ -112,6 +109,11 @@ TEST(LinkParser, doesntParseInvalidIpv4Links)
         "1.2.3",
         "htt://256.255.255.255",
         "aliens://256.255.255.255",
+        "256.255.255.255",
+        "http://256.255.255.255",
+        "255.256.255.255",
+        "255.255.256.255",
+        "255.255.255.256",
     };
 
     for (const auto &input : inputs)