From 99b7663fa962ff6960da1426eeeb6700bf061a89 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sat, 5 Nov 2022 11:33:21 +0800 Subject: [PATCH] Revise interface of port forwarder This eases the usage of port forwarder as the caller code doesn't need to store previous used port and now can rely on port forwarder doing all the hard work. PR #17967. --- src/base/bittorrent/portforwarderimpl.cpp | 74 +++++++++++++++-------- src/base/bittorrent/portforwarderimpl.h | 10 ++- src/base/net/portforwarder.h | 7 ++- src/webui/webui.cpp | 28 ++++----- src/webui/webui.h | 1 - 5 files changed, 73 insertions(+), 47 deletions(-) diff --git a/src/base/bittorrent/portforwarderimpl.cpp b/src/base/bittorrent/portforwarderimpl.cpp index cff7dedac..a6b9e3275 100644 --- a/src/base/bittorrent/portforwarderimpl.cpp +++ b/src/base/bittorrent/portforwarderimpl.cpp @@ -30,6 +30,7 @@ #include +#include "base/algorithm.h" #include "base/logger.h" PortForwarderImpl::PortForwarderImpl(lt::session *provider, QObject *parent) @@ -63,30 +64,45 @@ void PortForwarderImpl::setEnabled(const bool enabled) m_storeActive = enabled; } -void PortForwarderImpl::addPort(const quint16 port) +void PortForwarderImpl::setPorts(const QString &profile, QSet ports) { - if (m_mappedPorts.contains(port)) - return; + PortMapping &portMapping = m_portProfiles[profile]; + Algorithm::removeIf(portMapping, [this, &ports](const quint16 port, const std::vector &handles) + { + // keep existing forwardings + const bool isAlreadyMapped = ports.remove(port); + if (isAlreadyMapped) + return false; - if (isEnabled()) - m_mappedPorts.insert(port, m_provider->add_port_mapping(lt::session::tcp, port, port)); - else - m_mappedPorts.insert(port, {}); -} + // remove outdated forwardings + for (const lt::port_mapping_t &handle : handles) + m_provider->delete_port_mapping(handle); + m_forwardedPorts.remove(port); -void PortForwarderImpl::deletePort(const quint16 port) -{ - const auto iter = m_mappedPorts.find(port); - if (iter == m_mappedPorts.end()) - return; + return true; + }); - if (isEnabled()) + // add new forwardings + for (const quint16 port : ports) { - for (const lt::port_mapping_t &portMapping : *iter) - m_provider->delete_port_mapping(portMapping); + // port already forwarded/taken by other profile, don't do anything + if (m_forwardedPorts.contains(port)) + continue; + + if (isEnabled()) + portMapping.insert(port, m_provider->add_port_mapping(lt::session::tcp, port, port)); + else + portMapping.insert(port, {}); + m_forwardedPorts.insert(port); } - m_mappedPorts.erase(iter); + if (portMapping.isEmpty()) + m_portProfiles.remove(profile); +} + +void PortForwarderImpl::removePorts(const QString &profile) +{ + setPorts(profile, {}); } void PortForwarderImpl::start() @@ -96,12 +112,16 @@ void PortForwarderImpl::start() settingsPack.set_bool(lt::settings_pack::enable_natpmp, true); m_provider->apply_settings(settingsPack); - for (auto iter = m_mappedPorts.begin(); iter != m_mappedPorts.end(); ++iter) + for (auto profileIter = m_portProfiles.begin(); profileIter != m_portProfiles.end(); ++profileIter) { - Q_ASSERT(iter.value().empty()); - - const quint16 port = iter.key(); - iter.value() = m_provider->add_port_mapping(lt::session::tcp, port, port); + PortMapping &portMapping = profileIter.value(); + for (auto iter = portMapping.begin(); iter != portMapping.end(); ++iter) + { + Q_ASSERT(iter.value().empty()); + + const quint16 port = iter.key(); + iter.value() = m_provider->add_port_mapping(lt::session::tcp, port, port); + } } LogMsg(tr("UPnP/NAT-PMP support: ON"), Log::INFO); @@ -114,9 +134,13 @@ void PortForwarderImpl::stop() settingsPack.set_bool(lt::settings_pack::enable_natpmp, false); m_provider->apply_settings(settingsPack); - // don't clear m_mappedPorts so a later `start()` call can restore the port forwarding - for (auto iter = m_mappedPorts.begin(); iter != m_mappedPorts.end(); ++iter) - iter.value().clear(); + // don't clear m_portProfiles so a later `start()` call can restore the port forwardings + for (auto profileIter = m_portProfiles.begin(); profileIter != m_portProfiles.end(); ++profileIter) + { + PortMapping &portMapping = profileIter.value(); + for (auto iter = portMapping.begin(); iter != portMapping.end(); ++iter) + iter.value().clear(); + } LogMsg(tr("UPnP/NAT-PMP support: OFF"), Log::INFO); } diff --git a/src/base/bittorrent/portforwarderimpl.h b/src/base/bittorrent/portforwarderimpl.h index dee74ed1c..902b1002c 100644 --- a/src/base/bittorrent/portforwarderimpl.h +++ b/src/base/bittorrent/portforwarderimpl.h @@ -34,6 +34,7 @@ #include #include +#include #include "base/net/portforwarder.h" #include "base/settingvalue.h" @@ -50,8 +51,8 @@ public: bool isEnabled() const override; void setEnabled(bool enabled) override; - void addPort(quint16 port) override; - void deletePort(quint16 port) override; + void setPorts(const QString &profile, QSet ports) override; + void removePorts(const QString &profile) override; private: void start(); @@ -59,5 +60,8 @@ private: CachedSettingValue m_storeActive; lt::session *const m_provider = nullptr; - QHash> m_mappedPorts; + + using PortMapping = QHash>; // + QHash m_portProfiles; + QSet m_forwardedPorts; }; diff --git a/src/base/net/portforwarder.h b/src/base/net/portforwarder.h index f23ca2197..6f835d223 100644 --- a/src/base/net/portforwarder.h +++ b/src/base/net/portforwarder.h @@ -29,6 +29,9 @@ #pragma once #include +#include + +class QString; namespace Net { @@ -45,8 +48,8 @@ namespace Net virtual bool isEnabled() const = 0; virtual void setEnabled(bool enabled) = 0; - virtual void addPort(quint16 port) = 0; - virtual void deletePort(quint16 port) = 0; + virtual void setPorts(const QString &profile, QSet ports) = 0; + virtual void removePorts(const QString &profile) = 0; private: static PortForwarder *m_instance; diff --git a/src/webui/webui.cpp b/src/webui/webui.cpp index 5698e65e1..dc1b43596 100644 --- a/src/webui/webui.cpp +++ b/src/webui/webui.cpp @@ -50,25 +50,21 @@ void WebUI::configure() { m_isErrored = false; // clear previous error state - Preferences *const pref = Preferences::instance(); - - const quint16 oldPort = m_port; - m_port = pref->getWebUiPort(); + const QString portForwardingProfile = u"webui"_qs; + const Preferences *pref = Preferences::instance(); + const quint16 port = pref->getWebUiPort(); if (pref->isWebUiEnabled()) { - // UPnP/NAT-PMP + // Port forwarding + auto *portForwarder = Net::PortForwarder::instance(); if (pref->useUPnPForWebUIPort()) { - if (m_port != oldPort) - { - Net::PortForwarder::instance()->deletePort(oldPort); - Net::PortForwarder::instance()->addPort(m_port); - } + portForwarder->setPorts(portForwardingProfile, {port}); } else { - Net::PortForwarder::instance()->deletePort(oldPort); + portForwarder->removePorts(portForwardingProfile); } // http server @@ -81,7 +77,7 @@ void WebUI::configure() else { if ((m_httpServer->serverAddress().toString() != serverAddressString) - || (m_httpServer->serverPort() != m_port)) + || (m_httpServer->serverPort() != port)) m_httpServer->close(); } @@ -112,15 +108,15 @@ void WebUI::configure() { const auto address = ((serverAddressString == u"*") || serverAddressString.isEmpty()) ? QHostAddress::Any : QHostAddress(serverAddressString); - bool success = m_httpServer->listen(address, m_port); + bool success = m_httpServer->listen(address, port); if (success) { - LogMsg(tr("Web UI: Now listening on IP: %1, port: %2").arg(serverAddressString).arg(m_port)); + LogMsg(tr("Web UI: Now listening on IP: %1, port: %2").arg(serverAddressString).arg(port)); } else { const QString errorMsg = tr("Web UI: Unable to bind to IP: %1, port: %2. Reason: %3") - .arg(serverAddressString).arg(m_port).arg(m_httpServer->errorString()); + .arg(serverAddressString).arg(port).arg(m_httpServer->errorString()); LogMsg(errorMsg, Log::CRITICAL); qCritical() << errorMsg; @@ -144,7 +140,7 @@ void WebUI::configure() } else { - Net::PortForwarder::instance()->deletePort(oldPort); + Net::PortForwarder::instance()->removePorts(portForwardingProfile); delete m_httpServer; delete m_webapp; diff --git a/src/webui/webui.h b/src/webui/webui.h index f3fd37422..e2c63b828 100644 --- a/src/webui/webui.h +++ b/src/webui/webui.h @@ -66,5 +66,4 @@ private: QPointer m_httpServer; QPointer m_dnsUpdater; QPointer m_webapp; - quint16 m_port = 0; };