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

@@ -30,6 +30,7 @@
 
 #include <libtorrent/session.hpp>
 
+#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<quint16> ports)
 {
-    if (m_mappedPorts.contains(port))
-        return;
-
-    if (isEnabled())
-        m_mappedPorts.insert(port, m_provider->add_port_mapping(lt::session::tcp, port, port));
-    else
-        m_mappedPorts.insert(port, {});
-}
-
-void PortForwarderImpl::deletePort(const quint16 port)
-{
-    const auto iter = m_mappedPorts.find(port);
-    if (iter == m_mappedPorts.end())
-        return;
-
-    if (isEnabled())
+    PortMapping &portMapping = m_portProfiles[profile];
+    Algorithm::removeIf(portMapping, [this, &ports](const quint16 port, const std::vector<lt::port_mapping_t> &handles)
     {
-        for (const lt::port_mapping_t &portMapping : *iter)
-            m_provider->delete_port_mapping(portMapping);
+        // keep existing forwardings
+        const bool isAlreadyMapped = ports.remove(port);
+        if (isAlreadyMapped)
+            return false;
+
+        // remove outdated forwardings
+        for (const lt::port_mapping_t &handle : handles)
+            m_provider->delete_port_mapping(handle);
+        m_forwardedPorts.remove(port);
+
+        return true;
+    });
+
+    // add new forwardings
+    for (const quint16 port : ports)
+    {
+        // 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());
+        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);
+            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);
 }

src/base/bittorrent/portforwarderimpl.h

@@ -34,6 +34,7 @@
 #include <libtorrent/portmap.hpp>
 
 #include <QHash>
+#include <QSet>
 
 #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<quint16> ports) override;
+    void removePorts(const QString &profile) override;
 
 private:
     void start();
@@ -59,5 +60,8 @@ private:
 
     CachedSettingValue<bool> m_storeActive;
     lt::session *const m_provider = nullptr;
-    QHash<quint16, std::vector<lt::port_mapping_t>> m_mappedPorts;
+
+    using PortMapping = QHash<quint16, std::vector<lt::port_mapping_t>>;  // <port, handles>
+    QHash<QString, PortMapping> m_portProfiles;
+    QSet<quint16> m_forwardedPorts;
 };

src/base/net/portforwarder.h

@@ -29,6 +29,9 @@
 #pragma once
 
 #include <QObject>
+#include <QSet>
+
+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<quint16> ports) = 0;
+        virtual void removePorts(const QString &profile) = 0;
 
     private:
         static PortForwarder *m_instance;

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;

src/webui/webui.h

@@ -66,5 +66,4 @@ private:
     QPointer<Http::Server> m_httpServer;
     QPointer<Net::DNSUpdater> m_dnsUpdater;
     QPointer<WebApplication> m_webapp;
-    quint16 m_port = 0;
 };