Merge pull request #6834 from Chocobo1/cookie

[WebUI] Make cookie parsing robust

src/app/cmdoptions.cpp

@@ -44,6 +44,7 @@
 #endif
 
 #include "base/utils/misc.h"
+#include "base/utils/string.h"
 
 namespace
 {
@@ -86,7 +87,7 @@ namespace
         static QString padUsageText(const QString &usage)
         {
             QString res = QString(USAGE_INDENTATION, ' ') + usage;
-            
+
             if ((USAGE_TEXT_COLUMN - usage.length() - 4) > 0)
                 return res + QString(USAGE_TEXT_COLUMN - usage.length() - 4, ' ');
             else
@@ -153,7 +154,7 @@ namespace
         {
             QStringList parts = arg.split(QLatin1Char('='));
             if (parts.size() == 2)
-                return unquote(parts[1]);
+                return Utils::String::unquote(parts[1], QLatin1String("'\""));
             throw CommandLineParameterError(QObject::tr("Parameter '%1' must follow syntax '%1=%2'",
                                                         "e.g. Parameter '--webui-port' must follow syntax '--webui-port=value'")
                                             .arg(fullParameter()).arg(QLatin1String("<value>")));
@@ -162,7 +163,7 @@ namespace
         QString value(const QProcessEnvironment &env, const QString &defaultValue = QString()) const
         {
             QString val = env.value(envVarName());
-            return val.isEmpty() ? defaultValue : unquote(val);
+            return val.isEmpty() ? defaultValue : Utils::String::unquote(val, QLatin1String("'\""));
         }
 
         QString usage(const QString &valueName) const
@@ -175,19 +176,6 @@ namespace
         {
             return fullParameter() + QLatin1Char('=');
         }
-
-        static QString unquote(const QString &s)
-        {
-            auto isStringQuoted =
-                [](const QString &s, QChar quoteChar)
-                {
-                    return (s.startsWith(quoteChar) && s.endsWith(quoteChar));
-                };
-
-            if ((s.size() >= 2) && (isStringQuoted(s, QLatin1Char('\'')) || isStringQuoted(s, QLatin1Char('"'))))
-                return s.mid(1, s.size() - 2);
-            return s;
-        }
     };
 
     bool operator==(const QString &s, const StringOption &o)
@@ -290,7 +278,7 @@ namespace
         TriStateBool value(const QProcessEnvironment &env) const
         {
             QString val = env.value(envVarName(), "-1");
-            
+
             if (val.isEmpty()) {
                 return TriStateBool(m_defaultValue);
             }
@@ -374,7 +362,7 @@ QStringList QBtCommandLineParameters::paramList() const
     // specified by the user, and placing them at the beginning of the
     // string listr so that they will be processed before the list of
     // torrent paths or URLs.
-    
+
     if (!savePath.isEmpty())
         result.append(QString("@savePath=%1").arg(savePath));
 
@@ -384,7 +372,7 @@ QStringList QBtCommandLineParameters::paramList() const
     else if (addPaused == TriStateBool::False) {
         result.append(QLatin1String("@addPaused=0"));
     }
-    
+
     if (skipChecking)
         result.append(QLatin1String("@skipChecking"));
 
@@ -510,7 +498,7 @@ QString wrapText(const QString &text, int initialIndentation = USAGE_TEXT_COLUMN
     QStringList words = text.split(' ');
     QStringList lines = {words.first()};
     int currentLineMaxLength = wrapAtColumn - initialIndentation;
-    
+
     foreach (const QString &word, words.mid(1)) {
         if (lines.last().length() + word.length() + 1 < currentLineMaxLength) {
             lines.last().append(" " + word);
@@ -520,7 +508,7 @@ QString wrapText(const QString &text, int initialIndentation = USAGE_TEXT_COLUMN
             currentLineMaxLength = wrapAtColumn;
         }
     }
-    
+
     return lines.join("\n");
 }
 
@@ -559,7 +547,7 @@ QString makeUsage(const QString &prgName)
     stream << Option::padUsageText(QObject::tr("files or urls"))
            << wrapText(QObject::tr("Downloads the torrents passed by the user")) << '\n'
            << '\n';
-           
+
     stream << wrapText(QObject::tr("Options when adding new torrents:"), 0) << '\n';
     stream << SAVE_PATH_OPTION.usage(QObject::tr("path")) << wrapText(QObject::tr("Torrent save path")) << '\n';
     stream << PAUSED_OPTION.usage() << wrapText(QObject::tr("Add torrents as started or paused")) << '\n';

src/base/utils/string.h

@@ -30,10 +30,10 @@
 #ifndef UTILS_STRING_H
 #define UTILS_STRING_H
 
-#include <string>
+#include <QString>
 
 class QByteArray;
-class QString;
+class QLatin1String;
 
 namespace Utils
 {
@@ -49,6 +49,19 @@ namespace Utils
         bool naturalCompareCaseInsensitive(const QString &left, const QString &right);
 
         QString wildcardToRegex(const QString &pattern);
+
+        template <typename T>
+        T unquote(const T &str, const QString &quotes = QLatin1String("\""))
+        {
+            if (str.length() < 2) return str;
+
+            for (auto const quote : quotes) {
+                if (str.startsWith(quote) && str.endsWith(quote))
+                    return str.mid(1, str.length() - 2);
+            }
+
+            return str;
+        }
     }
 }
 

src/webui/abstractwebapplication.cpp

@@ -40,6 +40,7 @@
 #include "base/preferences.h"
 #include "base/utils/fs.h"
 #include "base/utils/random.h"
+#include "base/utils/string.h"
 #include "websessiondata.h"
 
 // UnbanTimer
@@ -147,24 +148,13 @@ void AbstractWebApplication::removeInactiveSessions()
 
 bool AbstractWebApplication::sessionInitialize()
 {
-    static const QString SID_START = QLatin1String(C_SID) + QLatin1String("=");
-
     if (session_ == 0)
     {
-        QString cookie = request_.headers.value("cookie");
-        //qDebug() << Q_FUNC_INFO << "cookie: " << cookie;
-
-        QString sessionId;
-        int pos = cookie.indexOf(SID_START);
-        if (pos >= 0) {
-            pos += SID_START.length();
-            int end = cookie.indexOf(QRegExp("[,;]"), pos);
-            sessionId = cookie.mid(pos, end >= 0 ? end - pos : end);
-        }
+        const QString sessionId = parseCookie(request_).value(C_SID);
 
         // TODO: Additional session check
 
-        if (!sessionId.isNull()) {
+        if (!sessionId.isEmpty()) {
             if (sessions_.contains(sessionId)) {
                 session_ = sessions_[sessionId];
                 session_->updateTimestamp();
@@ -386,3 +376,23 @@ const QStringMap AbstractWebApplication::CONTENT_TYPE_BY_EXT = {
     { "png", Http::CONTENT_TYPE_PNG },
     { "js", Http::CONTENT_TYPE_JS }
 };
+
+QStringMap AbstractWebApplication::parseCookie(const Http::Request &request) const
+{
+    // [rfc6265] 4.2.1. Syntax
+    QStringMap ret;
+    const QString cookieStr = request.headers.value(QLatin1String("cookie"));
+    const QVector<QStringRef> cookies = cookieStr.splitRef(';', QString::SkipEmptyParts);
+
+    for (const auto &cookie : cookies) {
+        const int idx = cookie.indexOf('=');
+        if (idx < 0)
+            continue;
+
+        const QString name = cookie.left(idx).trimmed().toString();
+        const QString value = Utils::String::unquote(cookie.mid(idx + 1).trimmed())
+                                  .toString();
+        ret.insert(name, value);
+    }
+    return ret;
+}

src/webui/abstractwebapplication.h

@@ -100,6 +100,8 @@ private:
     QString generateSid();
     bool sessionInitialize();
 
+    QStringMap parseCookie(const Http::Request &request) const;
+
     static void translateDocument(QString &data);
 
     static const QStringMap CONTENT_TYPE_BY_EXT;