From b107c0671d1a6c7ee442a5d99a0d54a21f6bb0f1 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 19 Sep 2017 13:03:54 +0800 Subject: [PATCH 1/3] WebAPI: fix root_folder default behavior Bug was introduced in 6b33db3ae3fe9d37c5f73737b537a64c77d2830e --- src/webui/webapplication.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp index 8c67b9fe7..cd86f4b63 100644 --- a/src/webui/webapplication.cpp +++ b/src/webui/webapplication.cpp @@ -394,7 +394,7 @@ void WebApplication::action_command_download() QStringList list = urls.split('\n'); bool skipChecking = request().posts["skip_checking"] == "true"; bool addPaused = request().posts["paused"] == "true"; - bool hasRootFolder = request().posts["root_folder"] == "true"; + const QString rootFolder = request().posts["root_folder"]; QString savepath = request().posts["savepath"]; QString category = request().posts["category"]; QString cookie = request().posts["cookie"]; @@ -423,9 +423,12 @@ void WebApplication::action_command_download() params.skipChecking = skipChecking; params.addPaused = TriStateBool(addPaused); - params.createSubfolder = TriStateBool(hasRootFolder); params.savePath = savepath; params.category = category; + if (rootFolder == "true") + params.createSubfolder = TriStateBool::True; + else if (rootFolder == "false") + params.createSubfolder = TriStateBool::False; bool partialSuccess = false; foreach (QString url, list) { @@ -448,7 +451,7 @@ void WebApplication::action_command_upload() CHECK_URI(0); bool skipChecking = request().posts["skip_checking"] == "true"; bool addPaused = request().posts["paused"] == "true"; - bool hasRootFolder = request().posts["root_folder"] == "true"; + const QString rootFolder = request().posts["root_folder"]; QString savepath = request().posts["savepath"]; QString category = request().posts["category"]; @@ -471,9 +474,13 @@ void WebApplication::action_command_upload() params.skipChecking = skipChecking; params.addPaused = TriStateBool(addPaused); - params.createSubfolder = TriStateBool(hasRootFolder); params.savePath = savepath; params.category = category; + if (rootFolder == "true") + params.createSubfolder = TriStateBool::True; + else if (rootFolder == "false") + params.createSubfolder = TriStateBool::False; + if (!BitTorrent::Session::instance()->addTorrent(torrentInfo, params)) { status(500, "Internal Server Error"); print(QObject::tr("Error: Could not add torrent to session."), Http::CONTENT_TYPE_TXT); From 72b0ba36ae8315533f5b97c40716dfaf8fadf617 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Mon, 14 Aug 2017 19:04:39 +0800 Subject: [PATCH 2/3] Refactor Merge statements Use case-insensitive contains() Add const Use value(), this avoids inserting empty values. Use range based for loop --- src/webui/webapplication.cpp | 110 ++++++++++++++++------------------- 1 file changed, 49 insertions(+), 61 deletions(-) diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp index cd86f4b63..728d47e47 100644 --- a/src/webui/webapplication.cpp +++ b/src/webui/webapplication.cpp @@ -390,48 +390,42 @@ void WebApplication::action_command_shutdown() void WebApplication::action_command_download() { CHECK_URI(0); - QString urls = request().posts["urls"]; - QStringList list = urls.split('\n'); - bool skipChecking = request().posts["skip_checking"] == "true"; - bool addPaused = request().posts["paused"] == "true"; - const QString rootFolder = request().posts["root_folder"]; - QString savepath = request().posts["savepath"]; - QString category = request().posts["category"]; - QString cookie = request().posts["cookie"]; + + const QString urls = request().posts.value("urls"); + const bool skipChecking = request().posts.value("skip_checking").contains("true", Qt::CaseInsensitive); + const bool addPaused = request().posts.value("paused").contains("true", Qt::CaseInsensitive); + const QString rootFolder = request().posts.value("root_folder"); + const QString savepath = request().posts.value("savepath").trimmed(); + const QString category = request().posts.value("category").trimmed(); + const QString cookie = request().posts.value("cookie"); + QList cookies; if (!cookie.isEmpty()) { - - QStringList cookiesStr = cookie.split("; "); - foreach (QString cookieStr, cookiesStr) { + const QStringList cookiesStr = cookie.split("; "); + for (QString cookieStr : cookiesStr) { cookieStr = cookieStr.trimmed(); int index = cookieStr.indexOf('='); if (index > 1) { QByteArray name = cookieStr.left(index).toLatin1(); QByteArray value = cookieStr.right(cookieStr.length() - index - 1).toLatin1(); - QNetworkCookie c(name, value); - cookies << c; + cookies += QNetworkCookie(name, value); } } } - savepath = savepath.trimmed(); - category = category.trimmed(); - BitTorrent::AddTorrentParams params; - // TODO: Check if destination actually exists params.skipChecking = skipChecking; - params.addPaused = TriStateBool(addPaused); params.savePath = savepath; params.category = category; - if (rootFolder == "true") + if (rootFolder.contains("true", Qt::CaseInsensitive)) params.createSubfolder = TriStateBool::True; - else if (rootFolder == "false") + else if (rootFolder.contains("false", Qt::CaseInsensitive)) params.createSubfolder = TriStateBool::False; bool partialSuccess = false; - foreach (QString url, list) { + for (QString url : urls.split('\n')) { url = url.trimmed(); if (!url.isEmpty()) { Net::DownloadManager::instance()->setCookiesFromUrl(cookies, QUrl::fromEncoded(url.toUtf8())); @@ -447,53 +441,47 @@ void WebApplication::action_command_download() void WebApplication::action_command_upload() { - qDebug() << Q_FUNC_INFO; CHECK_URI(0); - bool skipChecking = request().posts["skip_checking"] == "true"; - bool addPaused = request().posts["paused"] == "true"; - const QString rootFolder = request().posts["root_folder"]; - QString savepath = request().posts["savepath"]; - QString category = request().posts["category"]; - savepath = savepath.trimmed(); - category = category.trimmed(); + const bool skipChecking = request().posts.value("skip_checking").contains("true", Qt::CaseInsensitive); + const bool addPaused = request().posts.value("paused").contains("true", Qt::CaseInsensitive); + const QString rootFolder = request().posts.value("root_folder"); + const QString savepath = request().posts.value("savepath").trimmed(); + const QString category = request().posts.value("category").trimmed(); - foreach(const Http::UploadedFile& torrent, request().files) { - QString filePath = saveTmpFile(torrent.data); - - if (!filePath.isEmpty()) { - BitTorrent::TorrentInfo torrentInfo = BitTorrent::TorrentInfo::loadFromFile(filePath); - if (!torrentInfo.isValid()) { - status(415, "Unsupported Media Type"); - print(QObject::tr("Error: '%1' is not a valid torrent file.\n").arg(torrent.filename), Http::CONTENT_TYPE_TXT); - } - else { - BitTorrent::AddTorrentParams params; - - // TODO: Check if destination actually exists - params.skipChecking = skipChecking; - - params.addPaused = TriStateBool(addPaused); - params.savePath = savepath; - params.category = category; - if (rootFolder == "true") - params.createSubfolder = TriStateBool::True; - else if (rootFolder == "false") - params.createSubfolder = TriStateBool::False; - - if (!BitTorrent::Session::instance()->addTorrent(torrentInfo, params)) { - status(500, "Internal Server Error"); - print(QObject::tr("Error: Could not add torrent to session."), Http::CONTENT_TYPE_TXT); - } - } - // Clean up - Utils::Fs::forceRemove(filePath); - } - else { + for (const Http::UploadedFile &torrent : request().files) { + const QString filePath = saveTmpFile(torrent.data); + if (filePath.isEmpty()) { qWarning() << "I/O Error: Could not create temporary file"; status(500, "Internal Server Error"); print(QObject::tr("I/O Error: Could not create temporary file."), Http::CONTENT_TYPE_TXT); + continue; + } + + const BitTorrent::TorrentInfo torrentInfo = BitTorrent::TorrentInfo::loadFromFile(filePath); + if (!torrentInfo.isValid()) { + status(415, "Unsupported Media Type"); + print(QObject::tr("Error: '%1' is not a valid torrent file.\n").arg(torrent.filename), Http::CONTENT_TYPE_TXT); + } + else { + BitTorrent::AddTorrentParams params; + // TODO: Check if destination actually exists + params.skipChecking = skipChecking; + params.addPaused = TriStateBool(addPaused); + params.savePath = savepath; + params.category = category; + if (rootFolder.contains("true", Qt::CaseInsensitive)) + params.createSubfolder = TriStateBool::True; + else if (rootFolder.contains("false", Qt::CaseInsensitive)) + params.createSubfolder = TriStateBool::False; + + if (!BitTorrent::Session::instance()->addTorrent(torrentInfo, params)) { + status(500, "Internal Server Error"); + print(QObject::tr("Error: Could not add torrent to session."), Http::CONTENT_TYPE_TXT); + } } + // Clean up + Utils::Fs::forceRemove(filePath); } } From c5ddbcfb5b7d3cb3474466ba500ff7ffc370c111 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 19 Sep 2017 13:41:57 +0800 Subject: [PATCH 3/3] WebAPI: fix addPaused wrong default behavior Add helper function Sort include header --- src/webui/webapplication.cpp | 68 ++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 27 deletions(-) diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp index 728d47e47..d6c38d81a 100644 --- a/src/webui/webapplication.cpp +++ b/src/webui/webapplication.cpp @@ -35,22 +35,23 @@ #include #include +#include "base/bittorrent/session.h" +#include "base/bittorrent/torrenthandle.h" +#include "base/bittorrent/torrentinfo.h" +#include "base/bittorrent/trackerentry.h" #include "base/iconprovider.h" #include "base/logger.h" -#include "base/utils/misc.h" +#include "base/net/downloadmanager.h" +#include "base/preferences.h" +#include "base/tristatebool.h" #include "base/utils/fs.h" +#include "base/utils/misc.h" #include "base/utils/string.h" -#include "base/preferences.h" -#include "base/bittorrent/session.h" -#include "base/bittorrent/trackerentry.h" -#include "base/bittorrent/torrentinfo.h" -#include "base/bittorrent/torrenthandle.h" -#include "base/net/downloadmanager.h" #include "btjson.h" -#include "prefjson.h" #include "jsonutils.h" -#include "websessiondata.h" +#include "prefjson.h" #include "webapplication.h" +#include "websessiondata.h" static const int API_VERSION = 15; static const int API_VERSION_MIN = 15; @@ -67,9 +68,9 @@ const QString MAX_AGE_MONTH = "public, max-age=2592000"; #define ADD_ACTION(scope, action) actions[#scope][#action] = &WebApplication::action_##scope##_##action -QMap > WebApplication::initializeActions() +QMap> WebApplication::initializeActions() { - QMap > actions; + QMap> actions; ADD_ACTION(public, webui); ADD_ACTION(public, index); @@ -135,6 +136,8 @@ QMap > WebApplication::initialize return actions; } +namespace +{ #define CHECK_URI(ARGS_NUM) \ if (args_.size() != ARGS_NUM) { \ status(404, "Not Found"); \ @@ -154,6 +157,23 @@ QMap > WebApplication::initialize } \ } + bool parseBool(const QString &string, const bool defaultValue) + { + if (defaultValue) + return (string.compare("false", Qt::CaseInsensitive) == 0) ? false : true; + return (string.compare("true", Qt::CaseInsensitive) == 0) ? true : false; + } + + TriStateBool parseTristatebool(const QString &string) + { + if (string.compare("true", Qt::CaseInsensitive) == 0) + return TriStateBool::True; + if (string.compare("false", Qt::CaseInsensitive) == 0) + return TriStateBool::False; + return TriStateBool::Undefined; + } +} + void WebApplication::action_public_index() { QString path; @@ -392,9 +412,9 @@ void WebApplication::action_command_download() CHECK_URI(0); const QString urls = request().posts.value("urls"); - const bool skipChecking = request().posts.value("skip_checking").contains("true", Qt::CaseInsensitive); - const bool addPaused = request().posts.value("paused").contains("true", Qt::CaseInsensitive); - const QString rootFolder = request().posts.value("root_folder"); + const bool skipChecking = parseBool(request().posts.value("skip_checking"), false); + const TriStateBool addPaused = parseTristatebool(request().posts.value("paused")); + const TriStateBool rootFolder = parseTristatebool(request().posts.value("root_folder")); const QString savepath = request().posts.value("savepath").trimmed(); const QString category = request().posts.value("category").trimmed(); const QString cookie = request().posts.value("cookie"); @@ -416,13 +436,10 @@ void WebApplication::action_command_download() BitTorrent::AddTorrentParams params; // TODO: Check if destination actually exists params.skipChecking = skipChecking; - params.addPaused = TriStateBool(addPaused); + params.addPaused = addPaused; + params.createSubfolder = rootFolder; params.savePath = savepath; params.category = category; - if (rootFolder.contains("true", Qt::CaseInsensitive)) - params.createSubfolder = TriStateBool::True; - else if (rootFolder.contains("false", Qt::CaseInsensitive)) - params.createSubfolder = TriStateBool::False; bool partialSuccess = false; for (QString url : urls.split('\n')) { @@ -443,9 +460,9 @@ void WebApplication::action_command_upload() { CHECK_URI(0); - const bool skipChecking = request().posts.value("skip_checking").contains("true", Qt::CaseInsensitive); - const bool addPaused = request().posts.value("paused").contains("true", Qt::CaseInsensitive); - const QString rootFolder = request().posts.value("root_folder"); + const bool skipChecking = parseBool(request().posts.value("skip_checking"), false); + const TriStateBool addPaused = parseTristatebool(request().posts.value("paused")); + const TriStateBool rootFolder = parseTristatebool(request().posts.value("root_folder")); const QString savepath = request().posts.value("savepath").trimmed(); const QString category = request().posts.value("category").trimmed(); @@ -467,13 +484,10 @@ void WebApplication::action_command_upload() BitTorrent::AddTorrentParams params; // TODO: Check if destination actually exists params.skipChecking = skipChecking; - params.addPaused = TriStateBool(addPaused); + params.addPaused = addPaused; + params.createSubfolder = rootFolder; params.savePath = savepath; params.category = category; - if (rootFolder.contains("true", Qt::CaseInsensitive)) - params.createSubfolder = TriStateBool::True; - else if (rootFolder.contains("false", Qt::CaseInsensitive)) - params.createSubfolder = TriStateBool::False; if (!BitTorrent::Session::instance()->addTorrent(torrentInfo, params)) { status(500, "Internal Server Error");