Merge pull request #6369 from magao/issue6367

Use Perl-compatible regexes for RSS rules. Closes #6367.

src/base/rss/rssdownloadrule.cpp

@@ -28,14 +28,17 @@
  * Contact : chris@qbittorrent.org
  */
 
-#include <QRegExp>
 #include <QDebug>
 #include <QDir>
+#include <QHash>
+#include <QRegExp>
+#include <QRegularExpression>
 #include <QString>
 #include <QStringList>
 
 #include "base/preferences.h"
 #include "base/utils/fs.h"
+#include "base/utils/string.h"
 #include "rssfeed.h"
 #include "rssarticle.h"
 #include "rssdownloadrule.h"
@@ -47,28 +50,48 @@ DownloadRule::DownloadRule()
     , m_useRegex(false)
     , m_apstate(USE_GLOBAL)
     , m_ignoreDays(0)
+    , m_cachedRegexes(new QHash<QString, QRegularExpression>)
+{
+}
+
+DownloadRule::~DownloadRule()
+{
+    delete m_cachedRegexes;
+}
+
+QRegularExpression DownloadRule::getRegex(const QString &expression, bool isRegex) const
 {
+    // Use a cache of regexes so we don't have to continually recompile - big performance increase.
+    // The cache is cleared whenever the regex/wildcard, must or must not contain fields or
+    // episode filter are modified.
+    Q_ASSERT(!expression.isEmpty());
+    QRegularExpression regex((*m_cachedRegexes)[expression]);
+
+    if (!regex.pattern().isEmpty())
+        return regex;
+
+    return (*m_cachedRegexes)[expression] = QRegularExpression(isRegex ? expression : Utils::String::wildcardToRegex(expression), QRegularExpression::CaseInsensitiveOption);
 }
 
 bool DownloadRule::matches(const QString &articleTitle, const QString &expression) const
 {
-    static QRegExp whitespace("\\s+");
+    static QRegularExpression whitespace("\\s+");
 
     if (expression.isEmpty()) {
         // A regex of the form "expr|" will always match, so do the same for wildcards
         return true;
     }
     else if (m_useRegex) {
-        QRegExp reg(expression, Qt::CaseInsensitive, QRegExp::RegExp);
+        QRegularExpression reg(getRegex(expression));
-        return reg.indexIn(articleTitle) > -1;
+        return reg.match(articleTitle).hasMatch();
     }
     else {
         // Only match if every wildcard token (separated by spaces) is present in the article name.
         // Order of wildcard tokens is unimportant (if order is important, they should have used *).
         foreach (const QString &wildcard, expression.split(whitespace, QString::SplitBehavior::SkipEmptyParts)) {
-            QRegExp reg(wildcard, Qt::CaseInsensitive, QRegExp::Wildcard);
+            QRegularExpression reg(getRegex(wildcard, false));
 
-            if (reg.indexIn(articleTitle) == -1)
+            if (!reg.match(articleTitle).hasMatch())
                 return false;
         }
     }
@@ -124,14 +147,15 @@ bool DownloadRule::matches(const QString &articleTitle) const
 
     if (!m_episodeFilter.isEmpty()) {
         qDebug() << "Checking episode filter:" << m_episodeFilter;
-        QRegExp f("(^\\d{1,4})x(.*;$)");
+        QRegularExpression f(getRegex("(^\\d{1,4})x(.*;$)"));
-        int pos = f.indexIn(m_episodeFilter);
+        QRegularExpressionMatch matcher = f.match(m_episodeFilter);
+        bool matched = matcher.hasMatch();
 
-        if (pos < 0)
+        if (!matched)
             return false;
 
-        QString s = f.cap(1);
+        QString s = matcher.captured(1);
-        QStringList eps = f.cap(2).split(";");
+        QStringList eps = matcher.captured(2).split(";");
         int sOurs = s.toInt();
 
         foreach (QString ep, eps) {
@@ -145,22 +169,24 @@ bool DownloadRule::matches(const QString &articleTitle) const
             if (ep.indexOf('-') != -1) { // Range detected
                 QString partialPattern1 = "\\bs0?(\\d{1,4})[ -_\\.]?e(0?\\d{1,4})(?:\\D|\\b)";
                 QString partialPattern2 = "\\b(\\d{1,4})x(0?\\d{1,4})(?:\\D|\\b)";
-                QRegExp reg(partialPattern1, Qt::CaseInsensitive);
+                QRegularExpression reg(getRegex(partialPattern1));
 
                 if (ep.endsWith('-')) { // Infinite range
                     int epOurs = ep.left(ep.size() - 1).toInt();
 
                     // Extract partial match from article and compare as digits
-                    pos = reg.indexIn(articleTitle);
+                    matcher = reg.match(articleTitle);
+                    matched = matcher.hasMatch();
 
-                    if (pos == -1) {
+                    if (!matched) {
-                        reg = QRegExp(partialPattern2, Qt::CaseInsensitive);
+                        reg = QRegularExpression(getRegex(partialPattern2));
-                        pos = reg.indexIn(articleTitle);
+                        matcher = reg.match(articleTitle);
+                        matched = matcher.hasMatch();
                     }
 
-                    if (pos != -1) {
+                    if (matched) {
-                        int sTheirs = reg.cap(1).toInt();
+                        int sTheirs = matcher.captured(1).toInt();
-                        int epTheirs = reg.cap(2).toInt();
+                        int epTheirs = matcher.captured(2).toInt();
                         if (((sTheirs == sOurs) && (epTheirs >= epOurs)) || (sTheirs > sOurs)) {
                             qDebug() << "Matched episode:" << ep;
                             qDebug() << "Matched article:" << articleTitle;
@@ -178,16 +204,18 @@ bool DownloadRule::matches(const QString &articleTitle) const
                     int epOursLast = range.last().toInt();
 
                     // Extract partial match from article and compare as digits
-                    pos = reg.indexIn(articleTitle);
+                    matcher = reg.match(articleTitle);
+                    matched = matcher.hasMatch();
 
-                    if (pos == -1) {
+                    if (!matched) {
-                        reg = QRegExp(partialPattern2, Qt::CaseInsensitive);
+                        reg = QRegularExpression(getRegex(partialPattern2));
-                        pos = reg.indexIn(articleTitle);
+                        matcher = reg.match(articleTitle);
+                        matched = matcher.hasMatch();
                     }
 
-                    if (pos != -1) {
+                    if (matched) {
-                        int sTheirs = reg.cap(1).toInt();
+                        int sTheirs = matcher.captured(1).toInt();
-                        int epTheirs = reg.cap(2).toInt();
+                        int epTheirs = matcher.captured(2).toInt();
                         if ((sTheirs == sOurs) && ((epOursFirst <= epTheirs) && (epOursLast >= epTheirs))) {
                             qDebug() << "Matched episode:" << ep;
                             qDebug() << "Matched article:" << articleTitle;
@@ -198,8 +226,8 @@ bool DownloadRule::matches(const QString &articleTitle) const
             }
             else { // Single number
                 QString expStr("\\b(?:s0?" + s + "[ -_\\.]?" + "e0?" + ep + "|" + s + "x" + "0?" + ep + ")(?:\\D|\\b)");
-                QRegExp reg(expStr, Qt::CaseInsensitive);
+                QRegularExpression reg(getRegex(expStr));
-                if (reg.indexIn(articleTitle) != -1) {
+                if (reg.match(articleTitle).hasMatch()) {
                     qDebug() << "Matched episode:" << ep;
                     qDebug() << "Matched article:" << articleTitle;
                     return true;
@@ -216,6 +244,8 @@ bool DownloadRule::matches(const QString &articleTitle) const
 
 void DownloadRule::setMustContain(const QString &tokens)
 {
+    m_cachedRegexes->clear();
+
     if (m_useRegex)
         m_mustContain = QStringList() << tokens;
     else
@@ -228,6 +258,8 @@ void DownloadRule::setMustContain(const QString &tokens)
 
 void DownloadRule::setMustNotContain(const QString &tokens)
 {
+    m_cachedRegexes->clear();
+
     if (m_useRegex)
         m_mustNotContain = QStringList() << tokens;
     else
@@ -377,6 +409,7 @@ bool DownloadRule::useRegex() const
 void DownloadRule::setUseRegex(bool enabled)
 {
     m_useRegex = enabled;
+    m_cachedRegexes->clear();
 }
 
 QString DownloadRule::episodeFilter() const
@@ -387,6 +420,7 @@ QString DownloadRule::episodeFilter() const
 void DownloadRule::setEpisodeFilter(const QString &e)
 {
     m_episodeFilter = e;
+    m_cachedRegexes->clear();
 }
 
 QStringList DownloadRule::findMatchingArticles(const FeedPtr &feed) const

src/base/rss/rssdownloadrule.h

@@ -31,10 +31,13 @@
 #ifndef RSSDOWNLOADRULE_H
 #define RSSDOWNLOADRULE_H
 
-#include <QStringList>
+#include <QDateTime>
 #include <QVariantHash>
 #include <QSharedPointer>
-#include <QDateTime>
+#include <QStringList>
+
+template <class T, class U> class QHash;
+class QRegularExpression;
 
 namespace Rss
 {
@@ -55,6 +58,7 @@ namespace Rss
         };
 
         DownloadRule();
+        ~DownloadRule();
 
         static DownloadRulePtr fromVariantHash(const QVariantHash &ruleHash);
         QVariantHash toVariantHash() const;
@@ -89,6 +93,7 @@ namespace Rss
 
     private:
         bool matches(const QString &articleTitle, const QString &expression) const;
+        QRegularExpression getRegex(const QString &expression, bool isRegex = true) const;
 
         QString m_name;
         QStringList m_mustContain;
@@ -102,6 +107,7 @@ namespace Rss
         AddPausedState m_apstate;
         QDateTime m_lastMatch;
         int m_ignoreDays;
+        mutable QHash<QString, QRegularExpression> *m_cachedRegexes;
     };
 }
 

src/base/utils/string.cpp

@@ -35,6 +35,7 @@
 #include <QCollator>
 #include <QtGlobal>
 #include <QLocale>
+#include <QRegExp>
 #ifdef Q_OS_MAC
 #include <QThreadStorage>
 #endif
@@ -178,3 +179,13 @@ bool Utils::String::slowEquals(const QByteArray &a, const QByteArray &b)
 
     return (diff == 0);
 }
+
+// This is marked as internal in QRegExp.cpp, but is exported. The alternative would be to
+// copy the code from QRegExp::wc2rx().
+QString qt_regexp_toCanonical(const QString &pattern, QRegExp::PatternSyntax patternSyntax);
+
+QString Utils::String::wildcardToRegex(const QString &pattern)
+{
+    return qt_regexp_toCanonical(pattern, QRegExp::Wildcard);
+}
+

src/base/utils/string.h

@@ -47,6 +47,8 @@ namespace Utils
 
         bool naturalCompareCaseSensitive(const QString &left, const QString &right);
         bool naturalCompareCaseInsensitive(const QString &left, const QString &right);
+
+        QString wildcardToRegex(const QString &pattern);
     }
 }
 

src/gui/rss/automatedrssdownloader.cpp

@@ -33,6 +33,7 @@
 #include <QFileDialog>
 #include <QMessageBox>
 #include <QMenu>
+#include <QRegularExpression>
 
 #include "base/bittorrent/session.h"
 #include "base/preferences.h"
@@ -73,8 +74,8 @@ AutomatedRssDownloader::AutomatedRssDownloader(const QWeakPointer<Rss::Manager>
     Q_ASSERT(ok);
     m_ruleList = manager.toStrongRef()->downloadRules();
     m_editableRuleList = new Rss::DownloadRuleList; // Read rule list from disk
-    m_episodeRegex = new QRegExp("^(^\\d{1,4}x(\\d{1,4}(-(\\d{1,4})?)?;){1,}){1,1}",
+    m_episodeRegex = new QRegularExpression("^(^\\d{1,4}x(\\d{1,4}(-(\\d{1,4})?)?;){1,}){1,1}",
-                                 Qt::CaseInsensitive);
+                                 QRegularExpression::CaseInsensitiveOption);
     QString tip = "<p>" + tr("Matches articles based on episode filter.") + "</p><p><b>" + tr("Example: ")
                   + "1x2;8-15;5;30-;</b>" + tr(" will match 2, 5, 8 through 15, 30 and onward episodes of season one", "example X will match") + "</p>";
     tip += "<p>" + tr("Episode filter rules: ") + "</p><ul><li>" + tr("Season number is a mandatory non-zero value") + "</li>"
@@ -672,7 +673,7 @@ void AutomatedRssDownloader::updateFieldsToolTips(bool regex)
 {
     QString tip;
     if (regex) {
-        tip = "<p>" + tr("Regex mode: use Perl-like regular expressions") + "</p>";
+        tip = "<p>" + tr("Regex mode: use Perl-compatible regular expressions") + "</p>";
     }
     else {
         tip = "<p>" + tr("Wildcard mode: you can use") + "<ul>"
@@ -698,17 +699,23 @@ void AutomatedRssDownloader::updateFieldsToolTips(bool regex)
 void AutomatedRssDownloader::updateMustLineValidity()
 {
     const QString text = ui->lineContains->text();
+    bool isRegex = ui->checkRegex->isChecked();
     bool valid = true;
+    QString error;
 
     if (!text.isEmpty()) {
         QStringList tokens;
-        if (ui->checkRegex->isChecked())
+        if (isRegex)
             tokens << text;
         else
-            tokens << text.split("|");
+            foreach (const QString &token, text.split("|"))
+                tokens << Utils::String::wildcardToRegex(token);
+
         foreach (const QString &token, tokens) {
-            QRegExp reg(token, Qt::CaseInsensitive, ui->checkRegex->isChecked() ? QRegExp::RegExp : QRegExp::Wildcard);
+            QRegularExpression reg(token, QRegularExpression::CaseInsensitiveOption);
             if (!reg.isValid()) {
+                if (isRegex)
+                    error = tr("Position %1: %2").arg(reg.patternErrorOffset()).arg(reg.errorString());
                 valid = false;
                 break;
             }
@@ -718,27 +725,35 @@ void AutomatedRssDownloader::updateMustLineValidity()
     if (valid) {
         ui->lineContains->setStyleSheet("");
         ui->lbl_must_stat->setPixmap(QPixmap());
+        ui->lbl_must_stat->setToolTip(QLatin1String(""));
     }
     else {
         ui->lineContains->setStyleSheet("QLineEdit { color: #ff0000; }");
         ui->lbl_must_stat->setPixmap(GuiIconProvider::instance()->getIcon("task-attention").pixmap(16, 16));
+        ui->lbl_must_stat->setToolTip(error);
     }
 }
 
 void AutomatedRssDownloader::updateMustNotLineValidity()
 {
     const QString text = ui->lineNotContains->text();
+    bool isRegex = ui->checkRegex->isChecked();
     bool valid = true;
+    QString error;
 
     if (!text.isEmpty()) {
         QStringList tokens;
-        if (ui->checkRegex->isChecked())
+        if (isRegex)
             tokens << text;
         else
-            tokens << text.split("|");
+            foreach (const QString &token, text.split("|"))
+                tokens << Utils::String::wildcardToRegex(token);
+
         foreach (const QString &token, tokens) {
-            QRegExp reg(token, Qt::CaseInsensitive, ui->checkRegex->isChecked() ? QRegExp::RegExp : QRegExp::Wildcard);
+            QRegularExpression reg(token, QRegularExpression::CaseInsensitiveOption);
             if (!reg.isValid()) {
+                if (isRegex)
+                    error = tr("Position %1: %2").arg(reg.patternErrorOffset()).arg(reg.errorString());
                 valid = false;
                 break;
             }
@@ -748,17 +763,19 @@ void AutomatedRssDownloader::updateMustNotLineValidity()
     if (valid) {
         ui->lineNotContains->setStyleSheet("");
         ui->lbl_mustnot_stat->setPixmap(QPixmap());
+        ui->lbl_mustnot_stat->setToolTip(QLatin1String(""));
     }
     else {
         ui->lineNotContains->setStyleSheet("QLineEdit { color: #ff0000; }");
         ui->lbl_mustnot_stat->setPixmap(GuiIconProvider::instance()->getIcon("task-attention").pixmap(16, 16));
+        ui->lbl_mustnot_stat->setToolTip(error);
     }
 }
 
 void AutomatedRssDownloader::updateEpisodeFilterValidity()
 {
     const QString text = ui->lineEFilter->text();
-    bool valid = text.isEmpty() || m_episodeRegex->indexIn(text) != -1;
+    bool valid = text.isEmpty() || m_episodeRegex->match(text).hasMatch();
 
     if (valid) {
         ui->lineEFilter->setStyleSheet("");

src/gui/rss/automatedrssdownloader.h

@@ -34,7 +34,6 @@
 #include <QDialog>
 #include <QHideEvent>
 #include <QPair>
-#include <QRegExpValidator>
 #include <QSet>
 #include <QShortcut>
 #include <QShowEvent>
@@ -58,6 +57,7 @@ namespace Rss
 
 QT_BEGIN_NAMESPACE
 class QListWidgetItem;
+class QRegularExpression;
 QT_END_NAMESPACE
 
 class AutomatedRssDownloader: public QDialog
@@ -113,7 +113,7 @@ private:
     QListWidgetItem *m_editedRule;
     Rss::DownloadRuleList *m_ruleList;
     Rss::DownloadRuleList *m_editableRuleList;
-    QRegExp *m_episodeRegex;
+    QRegularExpression *m_episodeRegex;
     QShortcut *editHotkey;
     QShortcut *deleteHotkey;
     QSet<QPair<QString, QString >> m_treeListEntries;