Merge pull request #17453 from Chocobo1/i18n

Fix empty string parameter was omitted

src/app/application.cpp

@@ -83,6 +83,7 @@
 #include "base/utils/compare.h"
 #include "base/utils/fs.h"
 #include "base/utils/misc.h"
+#include "base/utils/string.h"
 #include "base/version.h"
 #include "applicationinstancemanager.h"
 #include "filelogger.h"
@@ -353,64 +354,74 @@ void Application::processMessage(const QString &message)
 
 void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const
 {
-    QString program = Preferences::instance()->getAutoRunProgram().trimmed();
+    // Cannot give users shell environment by default, as doing so could
+    // enable command injection via torrent name and other arguments
+    // (especially when some automated download mechanism has been setup).
+    // See: https://github.com/qbittorrent/qBittorrent/issues/10925
 
-    for (int i = (program.length() - 2); i >= 0; --i)
+    const auto replaceVariables = [torrent](QString str) -> QString
     {
-        if (program[i] != u'%')
+        for (int i = (str.length() - 2); i >= 0; --i)
-            continue;
-
-        const ushort specifier = program[i + 1].unicode();
-        switch (specifier)
         {
-        case u'C':
+            if (str[i] != u'%')
-            program.replace(i, 2, QString::number(torrent->filesCount()));
+                continue;
-            break;
+
-        case u'D':
+            const ushort specifier = str[i + 1].unicode();
-            program.replace(i, 2, torrent->savePath().toString());
+            switch (specifier)
-            break;
+            {
-        case u'F':
+            case u'C':
-            program.replace(i, 2, torrent->contentPath().toString());
+                str.replace(i, 2, QString::number(torrent->filesCount()));
-            break;
+                break;
-        case u'G':
+            case u'D':
-            program.replace(i, 2, torrent->tags().join(u","_qs));
+                str.replace(i, 2, torrent->savePath().toString());
-            break;
+                break;
-        case u'I':
+            case u'F':
-            program.replace(i, 2, (torrent->infoHash().v1().isValid() ? torrent->infoHash().v1().toString() : u"-"_qs));
+                str.replace(i, 2, torrent->contentPath().toString());
-            break;
+                break;
-        case u'J':
+            case u'G':
-            program.replace(i, 2, (torrent->infoHash().v2().isValid() ? torrent->infoHash().v2().toString() : u"-"_qs));
+                str.replace(i, 2, torrent->tags().join(u","_qs));
-            break;
+                break;
-        case u'K':
+            case u'I':
-            program.replace(i, 2, torrent->id().toString());
+                str.replace(i, 2, (torrent->infoHash().v1().isValid() ? torrent->infoHash().v1().toString() : u"-"_qs));
-            break;
+                break;
-        case u'L':
+            case u'J':
-            program.replace(i, 2, torrent->category());
+                str.replace(i, 2, (torrent->infoHash().v2().isValid() ? torrent->infoHash().v2().toString() : u"-"_qs));
-            break;
+                break;
-        case u'N':
+            case u'K':
-            program.replace(i, 2, torrent->name());
+                str.replace(i, 2, torrent->id().toString());
-            break;
+                break;
-        case u'R':
+            case u'L':
-            program.replace(i, 2, torrent->rootPath().toString());
+                str.replace(i, 2, torrent->category());
-            break;
+                break;
-        case u'T':
+            case u'N':
-            program.replace(i, 2, torrent->currentTracker());
+                str.replace(i, 2, torrent->name());
-            break;
+                break;
-        case u'Z':
+            case u'R':
-            program.replace(i, 2, QString::number(torrent->totalSize()));
+                str.replace(i, 2, torrent->rootPath().toString());
-            break;
+                break;
-        default:
+            case u'T':
-            // do nothing
+                str.replace(i, 2, torrent->currentTracker());
-            break;
+                break;
+            case u'Z':
+                str.replace(i, 2, QString::number(torrent->totalSize()));
+                break;
+            default:
+                // do nothing
+                break;
+            }
+
+            // decrement `i` to avoid unwanted replacement, example pattern: "%%N"
+            --i;
         }
 
-        // decrement `i` to avoid unwanted replacement, example pattern: "%%N"
+        return str;
-        --i;
+    };
-    }
 
-    LogMsg(tr("Torrent: %1, running external program, command: %2").arg(torrent->name(), program));
+    const QString logMsg = tr("Running external program. Torrent: \"%1\". Command: `%2`");
 
+    // The processing sequenece is different for Windows and other OS, this is intentional
 #if defined(Q_OS_WIN)
+    const QString program = replaceVariables(Preferences::instance()->getAutoRunProgram().trimmed());
     const std::wstring programWStr = program.toStdWString();
 
     // Need to split arguments manually because QProcess::startDetached(QString)
@@ -419,10 +430,15 @@ void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const
     int argCount = 0;
     std::unique_ptr<LPWSTR[], decltype(&::LocalFree)> args {::CommandLineToArgvW(programWStr.c_str(), &argCount), ::LocalFree};
 
+    if (argCount <= 0)
+        return;
+
     QStringList argList;
     for (int i = 1; i < argCount; ++i)
         argList += QString::fromWCharArray(args[i]);
 
+    LogMsg(logMsg.arg(torrent->name(), program));
+
     QProcess proc;
     proc.setProgram(QString::fromWCharArray(args[0]));
     proc.setArguments(argList);
@@ -449,14 +465,16 @@ void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const
     });
     proc.startDetached();
 #else // Q_OS_WIN
-    // Cannot give users shell environment by default, as doing so could
+    QStringList args = Utils::String::splitCommand(Preferences::instance()->getAutoRunProgram().trimmed());
-    // enable command injection via torrent name and other arguments
+
-    // (especially when some automated download mechanism has been setup).
-    // See: https://github.com/qbittorrent/qBittorrent/issues/10925
-    QStringList args = QProcess::splitCommand(program);
     if (args.isEmpty())
         return;
 
+    for (QString &arg : args)
+        arg = replaceVariables(arg);
+
+    LogMsg(logMsg.arg(torrent->name(), args.join(u' ')));
+
     const QString command = args.takeFirst();
     QProcess::startDetached(command, args);
 #endif

src/app/cmdoptions.cpp

@@ -527,7 +527,7 @@ QString makeUsage(const QString &prgName)
     const QString indentation {USAGE_INDENTATION, u' '};
 
     const QString text = QObject::tr("Usage:") + u'\n'
-        + indentation + prgName + u" [options] [(<filename> | <url>)...]" + u'\n'
+        + indentation + prgName + u' ' + QObject::tr("[options] [(<filename> | <url>)...]") + u'\n'
 
         + QObject::tr("Options:") + u'\n'
 #if !defined(Q_OS_WIN) || defined(DISABLE_GUI)

src/base/utils/string.cpp

@@ -33,6 +33,7 @@
 
 #include <QLocale>
 #include <QRegularExpression>
+#include <QStringList>
 #include <QVector>
 
 #if (QT_VERSION < QT_VERSION_CHECK(6, 0, 0))
@@ -76,6 +77,42 @@ QString Utils::String::wildcardToRegexPattern(const QString &pattern)
 }
 #endif
 
+QStringList Utils::String::splitCommand(const QString &command)
+{
+    QStringList ret;
+    ret.reserve(32);
+
+    bool inQuotes = false;
+    QString tmp;
+    for (const QChar c : command)
+    {
+        if (c == u' ')
+        {
+            if (!inQuotes)
+            {
+                if (!tmp.isEmpty())
+                {
+                    ret.append(tmp);
+                    tmp.clear();
+                }
+
+                continue;
+            }
+        }
+        else if (c == u'"')
+        {
+            inQuotes = !inQuotes;
+        }
+
+        tmp.append(c);
+    }
+
+    if (!tmp.isEmpty())
+        ret.append(tmp);
+
+    return ret;
+}
+
 std::optional<bool> Utils::String::parseBool(const QString &string)
 {
     if (string.compare(u"true", Qt::CaseInsensitive) == 0)

src/base/utils/string.h

@@ -61,6 +61,8 @@ namespace Utils::String
     std::optional<int> parseInt(const QString &string);
     std::optional<double> parseDouble(const QString &string);
 
+    QStringList splitCommand(const QString &command);
+
     QString join(const QList<QStringView> &strings, QStringView separator);
 
     QString fromDouble(double n, int precision);

test/CMakeLists.txt

@@ -14,6 +14,7 @@ set(testFiles
     testorderedset.cpp
     testutilscompare.cpp
     testutilsgzip.cpp
+    testutilsstring.cpp
 )
 foreach(testFile ${testFiles})
     get_filename_component(testFilename "${testFile}" NAME_WLE)

test/testutilsstring.cpp

@@ -0,0 +1,59 @@
+/*
+ * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2022  Mike Tzou (Chocobo1)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * In addition, as a special exception, the copyright holders give permission to
+ * link this program with the OpenSSL project's "OpenSSL" library (or with
+ * modified versions of it that use the same license as the "OpenSSL" library),
+ * and distribute the linked executables. You must obey the GNU General Public
+ * License in all respects for all of the code used other than "OpenSSL".  If you
+ * modify file(s), you may extend this exception to your version of the file(s),
+ * but you are not obligated to do so. If you do not wish to do so, delete this
+ * exception statement from your version.
+ */
+
+#include <QTest>
+
+#include "base/global.h"
+#include "base/utils/string.h"
+
+class TestUtilsString final : public QObject
+{
+    Q_OBJECT
+    Q_DISABLE_COPY_MOVE(TestUtilsString)
+
+public:
+    TestUtilsString() = default;
+
+private slots:
+    void testSplitCommand() const
+    {
+        QCOMPARE(Utils::String::splitCommand({}), {});
+        QCOMPARE(Utils::String::splitCommand(u""_qs), {});
+        QCOMPARE(Utils::String::splitCommand(u"  "_qs), {});
+        QCOMPARE(Utils::String::splitCommand(uR"("")"_qs), {uR"("")"_qs});
+        QCOMPARE(Utils::String::splitCommand(uR"(" ")"_qs), {uR"(" ")"_qs});
+        QCOMPARE(Utils::String::splitCommand(u"\"\"\""_qs), {u"\"\"\""_qs});
+        QCOMPARE(Utils::String::splitCommand(uR"(" """)"_qs), {uR"(" """)"_qs});
+        QCOMPARE(Utils::String::splitCommand(u" app a b c  "_qs), QStringList({u"app"_qs, u"a"_qs, u"b"_qs, u"c"_qs}));
+        QCOMPARE(Utils::String::splitCommand(u"   cmd.exe /d --arg2 \"arg3\" \"\" arg5 \"\"arg6 \"arg7 "_qs)
+            , QStringList({u"cmd.exe"_qs, u"/d"_qs, u"--arg2"_qs, u"\"arg3\""_qs, u"\"\""_qs, u"arg5"_qs, u"\"\"arg6"_qs, u"\"arg7 "_qs}));
+    }
+};
+
+QTEST_APPLESS_MAIN(TestUtilsString)
+#include "testutilsstring.moc"