From 5f6395256650bd1a58e0617a2d53e85d6c5bfe6c Mon Sep 17 00:00:00 2001 From: Morteza Namvar <morteza.namvar@savoirfairelinux.com> Date: Tue, 4 Jul 2023 17:08:58 -0400 Subject: [PATCH] refactor and cleanup Change-Id: Ia59c83978c26cebe060a301ec37bacd805d36ef5 --- CMakeLists.txt | 11 +- include/upnp/mapping.h | 1 + {src => include}/upnp/upnp_context.h | 36 ++- {src => include}/upnp/upnp_thread_util.h | 14 +- src/connectionmanager.cpp | 1 + src/fileutils.cpp | 6 +- src/ip_utils.cpp | 24 +- src/security/diffie-hellman.cpp | 20 +- src/upnp/protocol/igd.cpp | 10 +- src/upnp/protocol/igd.h | 3 +- src/upnp/protocol/mapping.cpp | 14 +- src/upnp/protocol/natpmp/nat_pmp.cpp | 144 +++++----- src/upnp/protocol/pupnp/pupnp.cpp | 328 +++++++++++------------ src/upnp/protocol/upnp_protocol.h | 22 -- src/upnp/upnp_context.cpp | 249 ++++++++--------- src/upnp/upnp_control.cpp | 13 +- 16 files changed, 461 insertions(+), 435 deletions(-) rename {src => include}/upnp/upnp_context.h (89%) rename {src => include}/upnp/upnp_thread_util.h (62%) diff --git a/CMakeLists.txt b/CMakeLists.txt index a94fa65..98a55d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -29,6 +29,12 @@ list (APPEND dhtnet_SOURCES src/security/tls_session.cpp src/security/certstore.cpp src/security/threadloop.cpp + src/upnp/upnp_context.cpp + src/upnp/upnp_control.cpp + src/upnp/protocol/mapping.cpp + src/upnp/upnp_context.cpp + src/upnp/upnp_control.cpp + src/upnp/protocol/igd.cpp ) list (APPEND dhtnet_HEADERS @@ -40,10 +46,13 @@ list (APPEND dhtnet_HEADERS include/fileutils.h include/string_utils.h include/ip_utils.h + include/mapping.h + include/upnp_context.h + include/upnp_control.h ) add_library(dhtnet ${dhtnet_SOURCES}) -target_link_libraries(dhtnet PUBLIC PkgConfig::opendht fmt::fmt msgpack-cxx) +target_link_libraries(dhtnet PUBLIC PkgConfig::opendht PkgConfig::pjproject fmt::fmt msgpackc) target_include_directories(dhtnet PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include> diff --git a/include/upnp/mapping.h b/include/upnp/mapping.h index 3ce16ac..a781022 100644 --- a/include/upnp/mapping.h +++ b/include/upnp/mapping.h @@ -28,6 +28,7 @@ #include <chrono> #include <functional> #include <mutex> +#include <memory> namespace jami { namespace upnp { diff --git a/src/upnp/upnp_context.h b/include/upnp/upnp_context.h similarity index 89% rename from src/upnp/upnp_context.h rename to include/upnp/upnp_context.h index 8d4e8c4..a85db51 100644 --- a/src/upnp/upnp_context.h +++ b/include/upnp/upnp_context.h @@ -22,17 +22,20 @@ #pragma once -#include "protocol/upnp_protocol.h" +/*#include "upnp_protocol.h" #if HAVE_LIBNATPMP #include "protocol/natpmp/nat_pmp.h" #endif #if HAVE_LIBUPNP #include "protocol/pupnp/pupnp.h" #endif -#include "protocol/igd.h" +#include "igd.h"*/ #include "ip_utils.h" +#include "upnp_thread_util.h" +#include "upnp/mapping.h" + #include <opendht/rng.h> #include <asio/steady_timer.hpp> @@ -44,9 +47,9 @@ #include <chrono> #include <random> #include <atomic> -#include <cstdlib> +#include <condition_variable> -#include "upnp_thread_util.h" +#include <cstdlib> using random_device = dht::crypto::random_device; @@ -59,6 +62,31 @@ class IpAddr; namespace jami { namespace upnp { +class UPnPProtocol; +class IGD; + +enum class UpnpIgdEvent { ADDED, REMOVED, INVALID_STATE }; + +// Interface used to report mapping event from the protocol implementations. +// This interface is meant to be implemented only by UPnPConext class. Sincce +// this class is a singleton, it's assumed that it out-lives the protocol +// implementations. In other words, the observer is always assumed to point to a +// valid instance. +class UpnpMappingObserver +{ +public: + UpnpMappingObserver() {}; + virtual ~UpnpMappingObserver() {}; + + virtual void onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) = 0; + virtual void onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; + virtual void onMappingRequestFailed(const Mapping& map) = 0; +#if HAVE_LIBNATPMP + virtual void onMappingRenewed(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; +#endif + virtual void onMappingRemoved(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; +}; + class UPnPContext : public UpnpMappingObserver, protected UpnpThreadUtil { private: diff --git a/src/upnp/upnp_thread_util.h b/include/upnp/upnp_thread_util.h similarity index 62% rename from src/upnp/upnp_thread_util.h rename to include/upnp/upnp_thread_util.h index 10d454a..6c7f97b 100644 --- a/src/upnp/upnp_thread_util.h +++ b/include/upnp/upnp_thread_util.h @@ -1,13 +1,16 @@ #pragma once #include <thread> +#include <memory> +#include <asio/io_context.hpp> // This macro is used to validate that a code is executed from the expected // thread. It's useful to detect unexpected race on data members. #define CHECK_VALID_THREAD() \ if (not isValidThread()) \ - JAMI_ERR() << "The calling thread " << getCurrentThread() \ - << " is not the expected thread: " << threadId_; + printf("The calling thread %d is not the expected thread: %d", getCurrentThread(), threadId_); + /*JAMI_ERR() << "The calling thread " << getCurrentThread() \ + << " is not the expected thread: " << threadId_;*/ namespace jami { namespace upnp { @@ -21,13 +24,16 @@ protected: // Upnp context execution queue (same as manager's scheduler) // Helpers to run tasks on upnp context queue. - static ScheduledExecutor* getScheduler() { return &Manager::instance().scheduler(); } + //static ScheduledExecutor* getScheduler() { return &Manager::instance().scheduler(); } + template<typename Callback> static void runOnUpnpContextQueue(Callback&& cb) { - getScheduler()->run([cb = std::forward<Callback>(cb)]() mutable { cb(); }); + //getScheduler()->run([cb = std::forward<Callback>(cb)]() mutable { cb(); }); + //ioContext->post(std::move(cb)); } + std::shared_ptr<asio::io_context> ioContext; std::thread::id threadId_; }; diff --git a/src/connectionmanager.cpp b/src/connectionmanager.cpp index 17206d3..5313096 100644 --- a/src/connectionmanager.cpp +++ b/src/connectionmanager.cpp @@ -34,6 +34,7 @@ #include <condition_variable> #include <set> #include <charconv> +#include <fstream> namespace jami { static constexpr std::chrono::seconds DHT_MSG_TIMEOUT {30}; diff --git a/src/fileutils.cpp b/src/fileutils.cpp index 8655f75..f3633ed 100644 --- a/src/fileutils.cpp +++ b/src/fileutils.cpp @@ -595,13 +595,13 @@ eraseFile_win32(const std::string& path, bool dosync) HANDLE h = CreateFileA(path.c_str(), GENERIC_WRITE, 0, 0, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0); if (h == INVALID_HANDLE_VALUE) { - JAMI_WARN("Can not open file %s for erasing.", path.c_str()); + // JAMI_WARN("Can not open file %s for erasing.", path.c_str()); return false; } LARGE_INTEGER size; if (!GetFileSizeEx(h, &size)) { - JAMI_WARN("Can not erase file %s: GetFileSizeEx() failed.", path.c_str()); + // JAMI_WARN("Can not erase file %s: GetFileSizeEx() failed.", path.c_str()); CloseHandle(h); return false; } @@ -618,7 +618,7 @@ eraseFile_win32(const std::string& path, bool dosync) try { buffer = new char[ERASE_BLOCK]; } catch (std::bad_alloc& ba) { - JAMI_WARN("Can not allocate buffer for erasing %s.", path.c_str()); + // JAMI_WARN("Can not allocate buffer for erasing %s.", path.c_str()); CloseHandle(h); return false; } diff --git a/src/ip_utils.cpp b/src/ip_utils.cpp index 494dc9b..0b2ded5 100644 --- a/src/ip_utils.cpp +++ b/src/ip_utils.cpp @@ -203,7 +203,7 @@ ip_utils::getLocalGateway() { char localHostBuf[INET_ADDRSTRLEN]; if (ip_utils::getHostName(localHostBuf, INET_ADDRSTRLEN) < 0) { - JAMI_WARN("Couldn't find local host"); + // JAMI_WARN("Couldn't find local host"); return {}; } else { return IpAddr(ip_utils::getGateway(localHostBuf, ip_utils::subnet_mask::prefix_24bit)); @@ -227,10 +227,10 @@ ip_utils::getAddrList(std::string_view name, pj_uint16_t family) const pj_str_t pjname(sip_utils::CONST_PJ_STR(name)); auto status = pj_getaddrinfo(family, &pjname, &addr_num, res); if (status != PJ_SUCCESS) { - JAMI_ERR("Error resolving %.*s : %s", - (int) name.size(), - name.data(), - sip_utils::sip_strerror(status).c_str()); + // JAMI_ERR("Error resolving %.*s : %s", + // (int) name.size(), + // name.data(), + // sip_utils::sip_strerror(status).c_str()); return ipList; } @@ -268,14 +268,14 @@ ip_utils::getLocalAddr(pj_uint16_t family) if (status == PJ_SUCCESS) { return ip_addr; } - JAMI_WARN("Could not get preferred address familly (%s)", - (family == pj_AF_INET6()) ? "IPv6" : "IPv4"); + // JAMI_WARN("Could not get preferred address familly (%s)", + // (family == pj_AF_INET6()) ? "IPv6" : "IPv4"); family = (family == pj_AF_INET()) ? pj_AF_INET6() : pj_AF_INET(); status = pj_gethostip(family, ip_addr.pjPtr()); if (status == PJ_SUCCESS) { return ip_addr; } - JAMI_ERR("Could not get local IP"); + // JAMI_ERR("Could not get local IP"); return ip_addr; } @@ -292,14 +292,14 @@ ip_utils::getInterfaceAddr(const std::string& interface, pj_uint16_t family) int fd = socket(unix_family, SOCK_DGRAM, 0); if (fd < 0) { - JAMI_ERR("Could not open socket: %m"); + // JAMI_ERR("Could not open socket: %m"); return addr; } if (unix_family == AF_INET6) { int val = family != pj_AF_UNSPEC(); if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*) &val, sizeof(val)) < 0) { - JAMI_ERR("Could not setsockopt: %m"); + // JAMI_ERR("Could not setsockopt: %m"); close(fd); return addr; } @@ -329,7 +329,7 @@ ip_utils::getInterfaceAddr(const std::string& interface, pj_uint16_t family) DWORD dwRetval = getaddrinfo(interface.c_str(), "0", &hints, &result); if (dwRetval != 0) { - JAMI_ERR("getaddrinfo failed with error: %lu", dwRetval); + // JAMI_ERR("getaddrinfo failed with error: %lu", dwRetval); return addr; } @@ -375,7 +375,7 @@ ip_utils::getAllIpInterfaceByName() } #else - JAMI_ERR("Not implemented yet. (iphlpapi.h problem)"); + // JAMI_ERR("Not implemented yet. (iphlpapi.h problem)"); #endif return ifaceList; } diff --git a/src/security/diffie-hellman.cpp b/src/security/diffie-hellman.cpp index bc0a854..dfe1d75 100644 --- a/src/security/diffie-hellman.cpp +++ b/src/security/diffie-hellman.cpp @@ -67,7 +67,7 @@ std::vector<uint8_t> DhParams::serialize() const { if (!params_) { - JAMI_WARN("serialize() called on an empty DhParams"); + // JAMI_WARN("serialize() called on an empty DhParams"); return {}; } gnutls_datum_t out; @@ -85,25 +85,25 @@ DhParams::generate() auto bits = gnutls_sec_param_to_pk_bits(GNUTLS_PK_DH, /* GNUTLS_SEC_PARAM_HIGH */ GNUTLS_SEC_PARAM_HIGH); - JAMI_DBG("Generating DH params with %u bits", bits); + // JAMI_DBG("Generating DH params with %u bits", bits); auto start = clock::now(); gnutls_dh_params_t new_params_; int ret = gnutls_dh_params_init(&new_params_); if (ret != GNUTLS_E_SUCCESS) { - JAMI_ERR("Error initializing DH params: %s", gnutls_strerror(ret)); + // JAMI_ERR("Error initializing DH params: %s", gnutls_strerror(ret)); return {}; } DhParams params {new_params_}; ret = gnutls_dh_params_generate2(params.get(), bits); if (ret != GNUTLS_E_SUCCESS) { - JAMI_ERR("Error generating DH params: %s", gnutls_strerror(ret)); + // JAMI_ERR("Error generating DH params: %s", gnutls_strerror(ret)); return {}; } std::chrono::duration<double> time_span = clock::now() - start; - JAMI_DBG("Generated DH params with %u bits in %lfs", bits, time_span.count()); + // JAMI_DBG("Generated DH params with %u bits in %lfs", bits, time_span.count()); return params; } @@ -117,20 +117,20 @@ DhParams::loadDhParams(const std::string& path) if (duration >= std::chrono::hours(24 * 3)) // file is valid only 3 days throw std::runtime_error("file too old"); - JAMI_DBG("Loading DhParams from file '%s'", path.c_str()); + // JAMI_DBG("Loading DhParams from file '%s'", path.c_str()); return {fileutils::loadFile(path)}; } catch (const std::exception& e) { - JAMI_DBG("Failed to load DhParams file '%s': %s", path.c_str(), e.what()); + // JAMI_DBG("Failed to load DhParams file '%s': %s", path.c_str(), e.what()); if (auto params = tls::DhParams::generate()) { try { fileutils::saveFile(path, params.serialize(), 0600); - JAMI_DBG("Saved DhParams to file '%s'", path.c_str()); + // JAMI_DBG("Saved DhParams to file '%s'", path.c_str()); } catch (const std::exception& ex) { - JAMI_WARN("Failed to save DhParams in file '%s': %s", path.c_str(), ex.what()); + // JAMI_WARN("Failed to save DhParams in file '%s': %s", path.c_str(), ex.what()); } return params; } - JAMI_ERR("Can't generate DH params."); + // JAMI_ERR("Can't generate DH params."); return {}; } } diff --git a/src/upnp/protocol/igd.cpp b/src/upnp/protocol/igd.cpp index 0e2ac90..862e908 100644 --- a/src/upnp/protocol/igd.cpp +++ b/src/upnp/protocol/igd.cpp @@ -21,7 +21,7 @@ */ #include "igd.h" -#include "logger.h" +//#include "logger.h" namespace jami { namespace upnp { @@ -45,7 +45,7 @@ IGD::setValid(bool valid) // Reset errors counter. errorsCounter_ = 0; } else { - JAMI_WARN("IGD %s [%s] was disabled", toString().c_str(), getProtocolName()); + // JAMI_WARN("IGD %s [%s] was disabled", toString().c_str(), getProtocolName()); } } @@ -56,9 +56,9 @@ IGD::incrementErrorsCounter() return false; if (++errorsCounter_ >= MAX_ERRORS_COUNT) { - JAMI_WARN("IGD %s [%s] has too many errors, it will be disabled", - toString().c_str(), - getProtocolName()); + // JAMI_WARN("IGD %s [%s] has too many errors, it will be disabled", + // toString().c_str(), + // getProtocolName()); setValid(false); return false; } diff --git a/src/upnp/protocol/igd.h b/src/upnp/protocol/igd.h index 33810f8..4ed9ed3 100644 --- a/src/upnp/protocol/igd.h +++ b/src/upnp/protocol/igd.h @@ -21,9 +21,10 @@ #pragma once #include <mutex> +#include <atomic> #include "ip_utils.h" -#include "mapping.h" +#include "upnp/mapping.h" #ifdef _MSC_VER typedef uint16_t in_port_t; diff --git a/src/upnp/protocol/mapping.cpp b/src/upnp/protocol/mapping.cpp index 8a25864..bb09441 100644 --- a/src/upnp/protocol/mapping.cpp +++ b/src/upnp/protocol/mapping.cpp @@ -20,8 +20,8 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "mapping.h" -#include "logger.h" +#include "upnp/mapping.h" +// #include "logger.h" #include "igd.h" namespace jami { @@ -70,7 +70,7 @@ void Mapping::updateFrom(const Mapping& other) { if (type_ != other.type_) { - JAMI_ERR("The source and destination types must match"); + // JAMI_ERR("The source and destination types must match"); return; } @@ -84,10 +84,10 @@ Mapping::updateFrom(const Mapping& other) void Mapping::setAvailable(bool val) { - JAMI_DBG("Changing mapping %s state from %s to %s", - toString().c_str(), - available_ ? "AVAILABLE" : "UNAVAILABLE", - val ? "AVAILABLE" : "UNAVAILABLE"); + // JAMI_DBG("Changing mapping %s state from %s to %s", + // toString().c_str(), + // available_ ? "AVAILABLE" : "UNAVAILABLE", + // val ? "AVAILABLE" : "UNAVAILABLE"); std::lock_guard<std::mutex> lock(mutex_); available_ = val; diff --git a/src/upnp/protocol/natpmp/nat_pmp.cpp b/src/upnp/protocol/natpmp/nat_pmp.cpp index 21f11ee..68dfee0 100644 --- a/src/upnp/protocol/natpmp/nat_pmp.cpp +++ b/src/upnp/protocol/natpmp/nat_pmp.cpp @@ -28,7 +28,7 @@ namespace upnp { NatPmp::NatPmp() { - JAMI_DBG("NAT-PMP: Instance [%p] created", this); + // JAMI_DBG("NAT-PMP: Instance [%p] created", this); runOnNatPmpQueue([this] { threadId_ = getCurrentThread(); igd_ = std::make_shared<PMPIGD>(); @@ -37,7 +37,7 @@ NatPmp::NatPmp() NatPmp::~NatPmp() { - JAMI_DBG("NAT-PMP: Instance [%p] destroyed", this); + // JAMI_DBG("NAT-PMP: Instance [%p] destroyed", this); } void @@ -61,7 +61,7 @@ NatPmp::initNatPmp() // Local address must be valid. if (not getHostAddress() or getHostAddress().isLoopback()) { - JAMI_WARN("NAT-PMP: Does not have a valid local address!"); + // JAMI_WARN("NAT-PMP: Does not have a valid local address!"); return; } @@ -75,18 +75,18 @@ NatPmp::initNatPmp() igd_->setPublicIp(IpAddr()); igd_->setUID(""); - JAMI_DBG("NAT-PMP: Trying to initialize IGD"); + // JAMI_DBG("NAT-PMP: Trying to initialize IGD"); int err = initnatpmp(&natpmpHdl_, 0, 0); if (err < 0) { - JAMI_WARN("NAT-PMP: Initializing IGD using default gateway failed!"); + // JAMI_WARN("NAT-PMP: Initializing IGD using default gateway failed!"); const auto& localGw = ip_utils::getLocalGateway(); if (not localGw) { - JAMI_WARN("NAT-PMP: Couldn't find valid gateway on local host"); + // JAMI_WARN("NAT-PMP: Couldn't find valid gateway on local host"); err = NATPMP_ERR_CANNOTGETGATEWAY; } else { - JAMI_WARN("NAT-PMP: Trying to initialize using detected gateway %s", + // JAMI_WARN("NAT-PMP: Trying to initialize using detected gateway %s", localGw.toString().c_str()); struct in_addr inaddr; @@ -96,14 +96,14 @@ NatPmp::initNatPmp() } if (err < 0) { - JAMI_ERR("NAT-PMP: Can't initialize libnatpmp -> %s", getNatPmpErrorStr(err)); + // JAMI_ERR("NAT-PMP: Can't initialize libnatpmp -> %s", getNatPmpErrorStr(err)); return; } char addrbuf[INET_ADDRSTRLEN]; inet_ntop(AF_INET, &natpmpHdl_.gateway, addrbuf, sizeof(addrbuf)); IpAddr igdAddr(addrbuf); - JAMI_DBG("NAT-PMP: Initialized on gateway %s", igdAddr.toString().c_str()); + // JAMI_DBG("NAT-PMP: Initialized on gateway %s", igdAddr.toString().c_str()); // Set the local (gateway) address. igd_->setLocalIp(igdAddr); @@ -133,7 +133,7 @@ NatPmp::setObserver(UpnpMappingObserver* obs) return; } - JAMI_DBG("NAT-PMP: Setting observer to %p", obs); + // JAMI_DBG("NAT-PMP: Setting observer to %p", obs); observer_ = obs; } @@ -164,9 +164,9 @@ NatPmp::terminate() }); if (cv.wait_for(lk, std::chrono::seconds(10), [this] { return shutdownComplete_; })) { - JAMI_DBG("NAT-PMP: Shutdown completed"); + // JAMI_DBG("NAT-PMP: Shutdown completed"); } else { - JAMI_ERR("NAT-PMP: Shutdown timed-out"); + // JAMI_ERR("NAT-PMP: Shutdown timed-out"); } } @@ -229,7 +229,7 @@ NatPmp::searchForIgd() // Schedule a retry in case init failed. if (not initialized_) { if (igdSearchCounter_++ < MAX_RESTART_SEARCH_RETRIES) { - JAMI_DBG("NAT-PMP: Start search for IGDs. Attempt %i", igdSearchCounter_); + // JAMI_DBG("NAT-PMP: Start search for IGDs. Attempt %i", igdSearchCounter_); // Cancel the current timer (if any) and re-schedule. if (searchForIgdTimer_) @@ -239,8 +239,8 @@ NatPmp::searchForIgd() NATPMP_SEARCH_RETRY_UNIT * igdSearchCounter_); } else { - JAMI_WARN("NAT-PMP: Setup failed after %u trials. NAT-PMP will be disabled!", - MAX_RESTART_SEARCH_RETRIES); + // JAMI_WARN("NAT-PMP: Setup failed after %u trials. NAT-PMP will be disabled!", + // MAX_RESTART_SEARCH_RETRIES); } } } @@ -259,7 +259,7 @@ bool NatPmp::isReady() const { if (observer_ == nullptr) { - JAMI_ERR("NAT-PMP: the observer is not set!"); + // JAMI_ERR("NAT-PMP: the observer is not set!"); return false; } @@ -286,7 +286,7 @@ NatPmp::incrementErrorsCounter(const std::shared_ptr<IGD>& igdIn) // Disable this IGD. igd_->setValid(false); // Notify the listener. - JAMI_WARN("NAT-PMP: No more valid IGD!"); + // JAMI_WARN("NAT-PMP: No more valid IGD!"); processIgdUpdate(UpnpIgdEvent::INVALID_STATE); } @@ -309,11 +309,11 @@ NatPmp::requestMappingAdd(const Mapping& mapping) assert(map.getIgd()); auto err = addPortMapping(map); if (err < 0) { - JAMI_WARN("NAT-PMP: Request for mapping %s on %s failed with error %i: %s", - map.toString().c_str(), - igd_->toString().c_str(), - err, - getNatPmpErrorStr(err)); + // JAMI_WARN("NAT-PMP: Request for mapping %s on %s failed with error %i: %s", + // map.toString().c_str(), + // igd_->toString().c_str(), + // err, + // getNatPmpErrorStr(err)); if (isErrorFatal(err)) { // Fatal error, increment the counter. @@ -322,9 +322,9 @@ NatPmp::requestMappingAdd(const Mapping& mapping) // Notify the listener. processMappingRequestFailed(std::move(map)); } else { - JAMI_DBG("NAT-PMP: Request for mapping %s on %s succeeded", - map.toString().c_str(), - igd_->toString().c_str()); + // JAMI_DBG("NAT-PMP: Request for mapping %s on %s succeeded", + // map.toString().c_str(), + // igd_->toString().c_str()); // Notify the listener. processMappingAdded(std::move(map)); } @@ -346,11 +346,11 @@ NatPmp::requestMappingRenew(const Mapping& mapping) Mapping map(mapping); auto err = addPortMapping(map); if (err < 0) { - JAMI_WARN("NAT-PMP: Renewal request for mapping %s on %s failed with error %i: %s", - map.toString().c_str(), - igd_->toString().c_str(), - err, - getNatPmpErrorStr(err)); + // JAMI_WARN("NAT-PMP: Renewal request for mapping %s on %s failed with error %i: %s", + // map.toString().c_str(), + // igd_->toString().c_str(), + // err, + // getNatPmpErrorStr(err)); // Notify the listener. processMappingRequestFailed(std::move(map)); @@ -359,9 +359,9 @@ NatPmp::requestMappingRenew(const Mapping& mapping) incrementErrorsCounter(igd_); } } else { - JAMI_DBG("NAT-PMP: Renewal request for mapping %s on %s succeeded", - map.toString().c_str(), - igd_->toString().c_str()); + // JAMI_DBG("NAT-PMP: Renewal request for mapping %s on %s succeeded", + // map.toString().c_str(), + // igd_->toString().c_str()); // Notify the listener. processMappingRenewed(map); } @@ -416,9 +416,9 @@ NatPmp::sendMappingRequest(const Mapping& mapping, uint32_t& lifetime) lifetime); if (err < 0) { - JAMI_ERR("NAT-PMP: Send mapping request failed with error %s %i", - getNatPmpErrorStr(err), - errno); + // JAMI_ERR("NAT-PMP: Send mapping request failed with error %s %i", + // getNatPmpErrorStr(err), + // errno); return err; } @@ -430,15 +430,15 @@ NatPmp::sendMappingRequest(const Mapping& mapping, uint32_t& lifetime) err = readResponse(natpmpHdl_, response); if (err < 0) { - JAMI_WARN("NAT-PMP: Read response on IGD %s failed with error %s", - igd_->toString().c_str(), - getNatPmpErrorStr(err)); + // JAMI_WARN("NAT-PMP: Read response on IGD %s failed with error %s", + // igd_->toString().c_str(), + // getNatPmpErrorStr(err)); } else if (response.type != NATPMP_RESPTYPE_TCPPORTMAPPING and response.type != NATPMP_RESPTYPE_UDPPORTMAPPING) { - JAMI_ERR("NAT-PMP: Unexpected response type (%i) for mapping %s from IGD %s.", - response.type, - mapping.toString().c_str(), - igd_->toString().c_str()); + // JAMI_ERR("NAT-PMP: Unexpected response type (%i) for mapping %s from IGD %s.", + // response.type, + // mapping.toString().c_str(), + // igd_->toString().c_str()); // Try to read again. continue; } @@ -515,8 +515,8 @@ NatPmp::removePortMapping(Mapping& mapping) if (err < 0) { // Nothing to do if the request fails, just log the error. - JAMI_WARN("NAT-PMP: Send remove request failed with error %s. Ignoring", - getNatPmpErrorStr(err)); + // JAMI_WARN("NAT-PMP: Send remove request failed with error %s. Ignoring", + // getNatPmpErrorStr(err)); } // Update and notify the listener. @@ -532,9 +532,9 @@ NatPmp::getIgdPublicAddress() // Set the public address for this IGD if it does not // have one already. if (igd_->getPublicIp()) { - JAMI_WARN("NAT-PMP: IGD %s already have a public address (%s)", - igd_->toString().c_str(), - igd_->getPublicIp().toString().c_str()); + // JAMI_WARN("NAT-PMP: IGD %s already have a public address (%s)", + // igd_->toString().c_str(), + // igd_->getPublicIp().toString().c_str()); return; } assert(igd_->getProtocol() == NatProtocolType::NAT_PMP); @@ -542,9 +542,9 @@ NatPmp::getIgdPublicAddress() int err = sendpublicaddressrequest(&natpmpHdl_); if (err < 0) { - JAMI_ERR("NAT-PMP: send public address request on IGD %s failed with error: %s", - igd_->toString().c_str(), - getNatPmpErrorStr(err)); + // JAMI_ERR("NAT-PMP: send public address request on IGD %s failed with error: %s", + // igd_->toString().c_str(), + // getNatPmpErrorStr(err)); if (isErrorFatal(err)) { // Fatal error, increment the counter. @@ -557,34 +557,34 @@ NatPmp::getIgdPublicAddress() err = readResponse(natpmpHdl_, response); if (err < 0) { - JAMI_WARN("NAT-PMP: Read response on IGD %s failed - %s", - igd_->toString().c_str(), - getNatPmpErrorStr(err)); + // JAMI_WARN("NAT-PMP: Read response on IGD %s failed - %s", + // igd_->toString().c_str(), + // getNatPmpErrorStr(err)); return; } if (response.type != NATPMP_RESPTYPE_PUBLICADDRESS) { - JAMI_ERR("NAT-PMP: Unexpected response type (%i) for public address request from IGD %s.", - response.type, - igd_->toString().c_str()); + // JAMI_ERR("NAT-PMP: Unexpected response type (%i) for public address request from IGD %s.", + // response.type, + // igd_->toString().c_str()); return; } IpAddr publicAddr(response.pnu.publicaddress.addr); if (not publicAddr) { - JAMI_ERR("NAT-PMP: IGD %s returned an invalid public address %s", - igd_->toString().c_str(), - publicAddr.toString().c_str()); + // JAMI_ERR("NAT-PMP: IGD %s returned an invalid public address %s", + // igd_->toString().c_str(), + // publicAddr.toString().c_str()); } // Update. igd_->setPublicIp(publicAddr); igd_->setValid(true); - JAMI_DBG("NAT-PMP: Setting IGD %s public address to %s", - igd_->toString().c_str(), - igd_->getPublicIp().toString().c_str()); + // JAMI_DBG("NAT-PMP: Setting IGD %s public address to %s", + // igd_->toString().c_str(), + // igd_->getPublicIp().toString().c_str()); } void @@ -592,18 +592,18 @@ NatPmp::removeAllMappings() { CHECK_VALID_THREAD(); - JAMI_WARN("NAT-PMP: Send request to close all existing mappings to IGD %s", - igd_->toString().c_str()); + // JAMI_WARN("NAT-PMP: Send request to close all existing mappings to IGD %s", + // igd_->toString().c_str()); int err = sendnewportmappingrequest(&natpmpHdl_, NATPMP_PROTOCOL_TCP, 0, 0, 0); if (err < 0) { - JAMI_WARN("NAT-PMP: Send close all TCP mappings request failed with error %s", - getNatPmpErrorStr(err)); + // JAMI_WARN("NAT-PMP: Send close all TCP mappings request failed with error %s", + // getNatPmpErrorStr(err)); } err = sendnewportmappingrequest(&natpmpHdl_, NATPMP_PROTOCOL_UDP, 0, 0, 0); if (err < 0) { - JAMI_WARN("NAT-PMP: Send close all UDP mappings request failed with error %s", - getNatPmpErrorStr(err)); + // JAMI_WARN("NAT-PMP: Send close all UDP mappings request failed with error %s", + // getNatPmpErrorStr(err)); } } @@ -706,9 +706,9 @@ bool NatPmp::validIgdInstance(const std::shared_ptr<IGD>& igdIn) { if (igd_.get() != igdIn.get()) { - JAMI_ERR("NAT-PMP: IGD (%s) does not match local instance (%s)", - igdIn->toString().c_str(), - igd_->toString().c_str()); + // JAMI_ERR("NAT-PMP: IGD (%s) does not match local instance (%s)", + // igdIn->toString().c_str(), + // igd_->toString().c_str()); return false; } diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp index cc63347..6ea5563 100644 --- a/src/upnp/protocol/pupnp/pupnp.cpp +++ b/src/upnp/protocol/pupnp/pupnp.cpp @@ -92,9 +92,9 @@ errorOnResponse(IXML_Document* doc) auto errorCode = getFirstDocItem(doc, "errorCode"); if (not errorCode.empty()) { auto errorDescription = getFirstDocItem(doc, "errorDescription"); - JAMI_WARNING("PUPnP: Response contains error: {:s}: {:s}", - errorCode, - errorDescription); + // JAMI_WARNING("PUPnP: Response contains error: {:s}: {:s}", + // errorCode, + // errorDescription); return true; } return false; @@ -104,16 +104,16 @@ errorOnResponse(IXML_Document* doc) PUPnP::PUPnP() { - JAMI_DBG("PUPnP: Creating instance [%p] ...", this); + // JAMI_DBG("PUPnP: Creating instance [%p] ...", this); runOnPUPnPQueue([this] { threadId_ = getCurrentThread(); - JAMI_DBG("PUPnP: Instance [%p] created", this); + // JAMI_DBG("PUPnP: Instance [%p] created", this); }); } PUPnP::~PUPnP() { - JAMI_DBG("PUPnP: Instance [%p] destroyed", this); + // JAMI_DBG("PUPnP: Instance [%p] destroyed", this); } void @@ -124,7 +124,7 @@ PUPnP::initUpnpLib() int upnp_err = UpnpInit2(nullptr, 0); if (upnp_err != UPNP_E_SUCCESS) { - JAMI_ERR("PUPnP: Can't initialize libupnp: %s", UpnpGetErrorMessage(upnp_err)); + // JAMI_ERR("PUPnP: Can't initialize libupnp: %s", UpnpGetErrorMessage(upnp_err)); UpnpFinish(); initialized_ = false; return; @@ -132,12 +132,12 @@ PUPnP::initUpnpLib() // Disable embedded WebServer if any. if (UpnpIsWebserverEnabled() == 1) { - JAMI_WARN("PUPnP: Web-server is enabled. Disabling"); + // JAMI_WARN("PUPnP: Web-server is enabled. Disabling"); UpnpEnableWebserver(0); if (UpnpIsWebserverEnabled() == 1) { - JAMI_ERR("PUPnP: Could not disable Web-server!"); + // JAMI_ERR("PUPnP: Could not disable Web-server!"); } else { - JAMI_DBG("PUPnP: Web-server successfully disabled"); + // JAMI_DBG("PUPnP: Web-server successfully disabled"); } } @@ -150,9 +150,9 @@ PUPnP::initUpnpLib() port6 = UpnpGetServerPort6(); #endif if (ip_address6 and port6) - JAMI_DBG("PUPnP: Initialized on %s:%u | %s:%u", ip_address, port, ip_address6, port6); + // JAMI_DBG("PUPnP: Initialized on %s:%u | %s:%u", ip_address, port, ip_address6, port6); else - JAMI_DBG("PUPnP: Initialized on %s:%u", ip_address, port); + // JAMI_DBG("PUPnP: Initialized on %s:%u", ip_address, port); // Relax the parser to allow malformed XML text. ixmlRelaxParser(1); @@ -177,9 +177,9 @@ PUPnP::registerClient() // Register Upnp control point. int upnp_err = UpnpRegisterClient(ctrlPtCallback, this, &ctrlptHandle_); if (upnp_err != UPNP_E_SUCCESS) { - JAMI_ERR("PUPnP: Can't register client: %s", UpnpGetErrorMessage(upnp_err)); + // JAMI_ERR("PUPnP: Can't register client: %s", UpnpGetErrorMessage(upnp_err)); } else { - JAMI_DBG("PUPnP: Successfully registered client"); + // JAMI_DBG("PUPnP: Successfully registered client"); clientRegistered_ = true; } } @@ -196,7 +196,7 @@ PUPnP::setObserver(UpnpMappingObserver* obs) return; } - JAMI_DBG("PUPnP: Setting observer to %p", obs); + // JAMI_DBG("PUPnP: Setting observer to %p", obs); observer_ = obs; } @@ -211,7 +211,7 @@ PUPnP::getHostAddress() const void PUPnP::terminate(std::condition_variable& cv) { - JAMI_DBG("PUPnP: Terminate instance %p", this); + // JAMI_DBG("PUPnP: Terminate instance %p", this); clientRegistered_ = false; observer_ = nullptr; @@ -220,7 +220,7 @@ PUPnP::terminate(std::condition_variable& cv) if (initialized_) { if (UpnpFinish() != UPNP_E_SUCCESS) { - JAMI_ERR("PUPnP: Failed to properly close lib-upnp"); + // JAMI_ERR("PUPnP: Failed to properly close lib-upnp"); } initialized_ = false; @@ -250,9 +250,9 @@ PUPnP::terminate() }); if (cv.wait_for(lk, std::chrono::seconds(10), [this] { return shutdownComplete_; })) { - JAMI_DBG("PUPnP: Shutdown completed"); + // JAMI_DBG("PUPnP: Shutdown completed"); } else { - JAMI_ERR("PUPnP: Shutdown timed-out"); + // JAMI_ERR("PUPnP: Shutdown timed-out"); // Force stop if the shutdown take too much time. shutdownComplete_ = true; } @@ -263,37 +263,37 @@ PUPnP::searchForDevices() { CHECK_VALID_THREAD(); - JAMI_DBG("PUPnP: Send IGD search request"); + // JAMI_DBG("PUPnP: Send IGD search request"); // Send out search for multiple types of devices, as some routers may possibly // only reply to one. auto err = UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_ROOT_DEVICE, this); if (err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Send search for UPNP_ROOT_DEVICE failed. Error %d: %s", - err, - UpnpGetErrorMessage(err)); + // JAMI_WARN("PUPnP: Send search for UPNP_ROOT_DEVICE failed. Error %d: %s", + // err, + // UpnpGetErrorMessage(err)); } err = UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_IGD_DEVICE, this); if (err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Send search for UPNP_IGD_DEVICE failed. Error %d: %s", - err, - UpnpGetErrorMessage(err)); + // JAMI_WARN("PUPnP: Send search for UPNP_IGD_DEVICE failed. Error %d: %s", + // err, + // UpnpGetErrorMessage(err)); } err = UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_WANIP_SERVICE, this); if (err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Send search for UPNP_WANIP_SERVICE failed. Error %d: %s", - err, - UpnpGetErrorMessage(err)); + // JAMI_WARN("PUPnP: Send search for UPNP_WANIP_SERVICE failed. Error %d: %s", + // err, + // UpnpGetErrorMessage(err)); } err = UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_WANPPP_SERVICE, this); if (err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Send search for UPNP_WANPPP_SERVICE failed. Error %d: %s", - err, - UpnpGetErrorMessage(err)); + // JAMI_WARN("PUPnP: Send search for UPNP_WANPPP_SERVICE failed. Error %d: %s", + // err, + // UpnpGetErrorMessage(err)); } } @@ -309,7 +309,7 @@ PUPnP::clearIgds() return; } - JAMI_DBG("PUPnP: clearing IGDs and devices lists"); + // JAMI_DBG("PUPnP: clearing IGDs and devices lists"); if (searchForIgdTimer_) searchForIgdTimer_->cancel(); @@ -344,23 +344,23 @@ PUPnP::searchForIgd() updateHostAddress(); if (isReady()) { - JAMI_DBG("PUPnP: Already have a valid IGD. Skip the search request"); + // JAMI_DBG("PUPnP: Already have a valid IGD. Skip the search request"); return; } if (igdSearchCounter_++ >= PUPNP_MAX_RESTART_SEARCH_RETRIES) { - JAMI_WARN("PUPnP: Setup failed after %u trials. PUPnP will be disabled!", - PUPNP_MAX_RESTART_SEARCH_RETRIES); + // JAMI_WARN("PUPnP: Setup failed after %u trials. PUPnP will be disabled!", + // PUPNP_MAX_RESTART_SEARCH_RETRIES); return; } - JAMI_DBG("PUPnP: Start search for IGD: attempt %u", igdSearchCounter_); + // JAMI_DBG("PUPnP: Start search for IGD: attempt %u", igdSearchCounter_); // Do not init if the host is not valid. Otherwise, the init will fail // anyway and may put libupnp in an unstable state (mainly deadlocks) // even if the UpnpFinish() method is called. if (not hasValidHostAddress()) { - JAMI_WARN("PUPnP: Host address is invalid. Skipping the IGD search"); + // JAMI_WARN("PUPnP: Host address is invalid. Skipping the IGD search"); } else { // Init and register if needed if (not initialized_) { @@ -374,7 +374,7 @@ PUPnP::searchForIgd() assert(initialized_); searchForDevices(); } else { - JAMI_WARN("PUPnP: PUPNP not fully setup. Skipping the IGD search"); + // JAMI_WARN("PUPnP: PUPNP not fully setup. Skipping the IGD search"); } } @@ -479,41 +479,41 @@ PUPnP::validateIgd(const std::string& location, IXML_Document* doc_container_ptr return false; } - JAMI_DBG("PUPnP: Validating the IGD candidate [UDN: %s]\n" - " Name : %s\n" - " Service Type : %s\n" - " Service ID : %s\n" - " Base URL : %s\n" - " Location URL : %s\n" - " control URL : %s\n" - " Event URL : %s", - igd_candidate->getUID().c_str(), - igd_candidate->getFriendlyName().c_str(), - igd_candidate->getServiceType().c_str(), - igd_candidate->getServiceId().c_str(), - igd_candidate->getBaseURL().c_str(), - igd_candidate->getLocationURL().c_str(), - igd_candidate->getControlURL().c_str(), - igd_candidate->getEventSubURL().c_str()); + // JAMI_DBG("PUPnP: Validating the IGD candidate [UDN: %s]\n" + // " Name : %s\n" + // " Service Type : %s\n" + // " Service ID : %s\n" + // " Base URL : %s\n" + // " Location URL : %s\n" + // " control URL : %s\n" + // " Event URL : %s", + // igd_candidate->getUID().c_str(), + // igd_candidate->getFriendlyName().c_str(), + // igd_candidate->getServiceType().c_str(), + // igd_candidate->getServiceId().c_str(), + // igd_candidate->getBaseURL().c_str(), + // igd_candidate->getLocationURL().c_str(), + // igd_candidate->getControlURL().c_str(), + // igd_candidate->getEventSubURL().c_str()); // Check if IGD is connected. if (not actionIsIgdConnected(*igd_candidate)) { - JAMI_WARN("PUPnP: IGD candidate %s is not connected", igd_candidate->getUID().c_str()); + // JAMI_WARN("PUPnP: IGD candidate %s is not connected", igd_candidate->getUID().c_str()); return false; } // Validate external Ip. igd_candidate->setPublicIp(actionGetExternalIP(*igd_candidate)); if (igd_candidate->getPublicIp().toString().empty()) { - JAMI_WARN("PUPnP: IGD candidate %s has no valid external Ip", - igd_candidate->getUID().c_str()); + // JAMI_WARN("PUPnP: IGD candidate %s has no valid external Ip", + // igd_candidate->getUID().c_str()); return false; } // Validate internal Ip. if (igd_candidate->getBaseURL().empty()) { - JAMI_WARN("PUPnP: IGD candidate %s has no valid internal Ip", - igd_candidate->getUID().c_str()); + // JAMI_WARN("PUPnP: IGD candidate %s has no valid internal Ip", + // igd_candidate->getUID().c_str()); return false; } @@ -523,8 +523,8 @@ PUPnP::validateIgd(const std::string& location, IXML_Document* doc_container_ptr if (const auto& localGw = ip_utils::getLocalGateway()) { igd_candidate->setLocalIp(localGw); } else { - JAMI_WARN("PUPnP: Could not set internal address for IGD candidate %s", - igd_candidate->getUID().c_str()); + // JAMI_WARN("PUPnP: Could not set internal address for IGD candidate %s", + // igd_candidate->getUID().c_str()); return false; } @@ -538,11 +538,11 @@ PUPnP::validateIgd(const std::string& location, IXML_Document* doc_container_ptr // Must not be a null pointer assert(igd.get() != nullptr); if (*igd == *igd_candidate) { - JAMI_DBG("PUPnP: Device [%s] with int/ext addresses [%s:%s] is already in the list " - "of valid IGDs", - igd_candidate->getUID().c_str(), - igd_candidate->toString().c_str(), - igd_candidate->getPublicIp().toString().c_str()); + // JAMI_DBG("PUPnP: Device [%s] with int/ext addresses [%s:%s] is already in the list " + // "of valid IGDs", + // igd_candidate->getUID().c_str(), + // igd_candidate->toString().c_str(), + // igd_candidate->getPublicIp().toString().c_str()); return true; } } @@ -551,12 +551,12 @@ PUPnP::validateIgd(const std::string& location, IXML_Document* doc_container_ptr // We have a valid IGD igd_candidate->setValid(true); - JAMI_DBG("PUPnP: Added a new IGD [%s] to the list of valid IGDs", - igd_candidate->getUID().c_str()); + // JAMI_DBG("PUPnP: Added a new IGD [%s] to the list of valid IGDs", + // igd_candidate->getUID().c_str()); - JAMI_DBG("PUPnP: New IGD addresses [int: %s - ext: %s]", - igd_candidate->toString().c_str(), - igd_candidate->getPublicIp().toString().c_str()); + // JAMI_DBG("PUPnP: New IGD addresses [int: %s - ext: %s]", + // igd_candidate->toString().c_str(), + // igd_candidate->getPublicIp().toString().c_str()); // Subscribe to IGD events. int upnp_err = UpnpSubscribeAsync(ctrlptHandle_, @@ -565,13 +565,13 @@ PUPnP::validateIgd(const std::string& location, IXML_Document* doc_container_ptr subEventCallback, this); if (upnp_err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Failed to send subscribe request to %s: error %i - %s", - igd_candidate->getUID().c_str(), - upnp_err, - UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Failed to send subscribe request to %s: error %i - %s", + // igd_candidate->getUID().c_str(), + // upnp_err, + // UpnpGetErrorMessage(upnp_err)); // return false; } else { - JAMI_DBG("PUPnP: Successfully subscribed to IGD %s", igd_candidate->getUID().c_str()); + // JAMI_DBG("PUPnP: Successfully subscribed to IGD %s", igd_candidate->getUID().c_str()); } { @@ -647,7 +647,7 @@ PUPnP::findMatchingIgd(const std::string& ctrlURL) const }); if (iter == validIgdList_.end()) { - JAMI_WARN("PUPnP: Did not find the IGD matching ctrl URL [%s]", ctrlURL.c_str()); + // JAMI_WARN("PUPnP: Did not find the IGD matching ctrl URL [%s]", ctrlURL.c_str()); return {}; } @@ -680,7 +680,7 @@ PUPnP::processRequestMappingFailure(const Mapping& map) runOnUpnpContextQueue([w = weak(), map] { if (auto upnpThis = w.lock()) { - JAMI_DBG("PUPnP: Failed to request mapping %s", map.toString().c_str()); + // JAMI_DBG("PUPnP: Failed to request mapping %s", map.toString().c_str()); if (upnpThis->observer_) upnpThis->observer_->onMappingRequestFailed(map); } @@ -696,7 +696,7 @@ PUPnP::processRemoveMapAction(const Mapping& map) return; runOnUpnpContextQueue([map, obs = observer_] { - JAMI_DBG("PUPnP: Closed mapping %s", map.toString().c_str()); + // JAMI_DBG("PUPnP: Closed mapping %s", map.toString().c_str()); obs->onMappingRemoved(map.getIgd(), std::move(map)); }); } @@ -746,7 +746,7 @@ PUPnP::ctrlPtCallback(Upnp_EventType event_type, const void* event, void* user_d auto pupnp = static_cast<PUPnP*>(user_data); if (pupnp == nullptr) { - JAMI_WARN("PUPnP: Control point callback without PUPnP"); + // JAMI_WARN("PUPnP: Control point callback without PUPnP"); return UPNP_E_SUCCESS; } @@ -794,7 +794,7 @@ PUPnP::processDiscoverySearchResult(const std::string& cpDeviceId, // The host address must be valid to proceed. if (not hasValidHostAddress()) { - JAMI_WARN("PUPnP: Local address is invalid. Ignore search result for now!"); + // JAMI_WARN("PUPnP: Local address is invalid. Ignore search result for now!"); return; } @@ -808,7 +808,7 @@ PUPnP::processDiscoverySearchResult(const std::string& cpDeviceId, return; } - JAMI_DBG("PUPnP: Discovered a new IGD [%s]", igdId.c_str()); + // JAMI_DBG("PUPnP: Discovered a new IGD [%s]", igdId.c_str()); // NOTE: here, we check if the location given is related to the source address. // If it's not the case, it's certainly a router plugged in the network, but not @@ -818,9 +818,9 @@ PUPnP::processDiscoverySearchResult(const std::string& cpDeviceId, // Only check the IP address (ignore the port number). dht::http::Url url(igdLocationUrl); if (IpAddr(url.host).toString(false) != dstAddr.toString(false)) { - JAMI_DBG("PUPnP: Returned location %s does not match the source address %s", - IpAddr(url.host).toString(true, true).c_str(), - dstAddr.toString(true, true).c_str()); + // JAMI_DBG("PUPnP: Returned location %s does not match the source address %s", + // IpAddr(url.host).toString(true, true).c_str(), + // dstAddr.toString(true, true).c_str()); return; } @@ -840,11 +840,11 @@ PUPnP::downLoadIgdDescription(const std::string& locationUrl) int upnp_err = UpnpDownloadXmlDoc(locationUrl.c_str(), &doc_container_ptr); if (upnp_err != UPNP_E_SUCCESS or not doc_container_ptr) { - JAMI_WARN("PUPnP: Error downloading device XML document from %s -> %s", - locationUrl.c_str(), - UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Error downloading device XML document from %s -> %s", + // locationUrl.c_str(), + // UpnpGetErrorMessage(upnp_err)); } else { - JAMI_DBG("PUPnP: Succeeded to download device XML document from %s", locationUrl.c_str()); + // JAMI_DBG("PUPnP: Succeeded to download device XML document from %s", locationUrl.c_str()); runOnPUPnPQueue([w = weak(), url = locationUrl, doc_container_ptr] { if (auto upnpThis = w.lock()) { upnpThis->validateIgd(url, doc_container_ptr); @@ -866,10 +866,10 @@ PUPnP::processDiscoveryAdvertisementByebye(const std::string& cpDeviceId) for (auto it = validIgdList_.begin(); it != validIgdList_.end();) { if ((*it)->getUID() == cpDeviceId) { igd = *it; - JAMI_DBG("PUPnP: Received [%s] for IGD [%s] %s. Will be removed.", - PUPnP::eventTypeToString(UPNP_DISCOVERY_ADVERTISEMENT_BYEBYE), - igd->getUID().c_str(), - igd->toString().c_str()); + // JAMI_DBG("PUPnP: Received [%s] for IGD [%s] %s. Will be removed.", + // PUPnP::eventTypeToString(UPNP_DISCOVERY_ADVERTISEMENT_BYEBYE), + // igd->getUID().c_str(), + // igd->toString().c_str()); igd->setValid(false); // Remove the IGD. it = validIgdList_.erase(it); @@ -895,10 +895,10 @@ PUPnP::processDiscoverySubscriptionExpired(Upnp_EventType event_type, const std: for (auto& it : validIgdList_) { if (auto igd = std::dynamic_pointer_cast<UPnPIGD>(it)) { if (igd->getEventSubURL() == eventSubUrl) { - JAMI_DBG("PUPnP: Received [%s] event for IGD [%s] %s. Request a new subscribe.", - PUPnP::eventTypeToString(event_type), - igd->getUID().c_str(), - igd->toString().c_str()); + // JAMI_DBG("PUPnP: Received [%s] event for IGD [%s] %s. Request a new subscribe.", + // PUPnP::eventTypeToString(event_type), + // igd->getUID().c_str(), + // igd->toString().c_str()); UpnpSubscribeAsync(ctrlptHandle_, eventSubUrl.c_str(), UPNP_INFINITE, @@ -923,8 +923,8 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) // First check the error code. auto upnp_status = UpnpDiscovery_get_ErrCode(d_event); if (upnp_status != UPNP_E_SUCCESS) { - JAMI_ERR("PUPnP: UPNP discovery is in erroneous state: %s", - UpnpGetErrorMessage(upnp_status)); + // JAMI_ERR("PUPnP: UPNP discovery is in erroneous state: %s", + // UpnpGetErrorMessage(upnp_status)); break; } @@ -969,10 +969,10 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) case UPNP_EVENT_AUTORENEWAL_FAILED: case UPNP_EVENT_SUBSCRIPTION_EXPIRED: // This event will occur only if autorenewal is disabled. { - JAMI_WARN("PUPnP: Received Subscription Event %s", eventTypeToString(event_type)); + // JAMI_WARN("PUPnP: Received Subscription Event %s", eventTypeToString(event_type)); const UpnpEventSubscribe* es_event = (const UpnpEventSubscribe*) event; if (es_event == nullptr) { - JAMI_WARN("PUPnP: Received Subscription Event with null pointer"); + // JAMI_WARN("PUPnP: Received Subscription Event with null pointer"); break; } std::string publisherUrl(UpnpEventSubscribe_get_PublisherUrl_cstr(es_event)); @@ -989,7 +989,7 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) case UPNP_EVENT_UNSUBSCRIBE_COMPLETE: { UpnpEventSubscribe* es_event = (UpnpEventSubscribe*) event; if (es_event == nullptr) { - JAMI_WARN("PUPnP: Received Subscription Event with null pointer"); + // JAMI_WARN("PUPnP: Received Subscription Event with null pointer"); } else { UpnpEventSubscribe_delete(es_event); } @@ -998,18 +998,18 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) case UPNP_CONTROL_ACTION_COMPLETE: { const UpnpActionComplete* a_event = (const UpnpActionComplete*) event; if (a_event == nullptr) { - JAMI_WARN("PUPnP: Received Action Complete Event with null pointer"); + // JAMI_WARN("PUPnP: Received Action Complete Event with null pointer"); break; } auto res = UpnpActionComplete_get_ErrCode(a_event); if (res != UPNP_E_SUCCESS and res != UPNP_E_TIMEDOUT) { auto err = UpnpActionComplete_get_ErrCode(a_event); - JAMI_WARN("PUPnP: Received Action Complete error %i %s", err, UpnpGetErrorMessage(err)); + // JAMI_WARN("PUPnP: Received Action Complete error %i %s", err, UpnpGetErrorMessage(err)); } else { auto actionRequest = UpnpActionComplete_get_ActionRequest(a_event); // Abort if there is no action to process. if (actionRequest == nullptr) { - JAMI_WARN("PUPnP: Can't get the Action Request data from the event"); + // JAMI_WARN("PUPnP: Can't get the Action Request data from the event"); break; } @@ -1017,13 +1017,13 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) if (actionResult != nullptr) { ixmlDocument_free(actionResult); } else { - JAMI_WARN("PUPnP: Action Result document not found"); + // JAMI_WARN("PUPnP: Action Result document not found"); } } break; } default: { - JAMI_WARN("PUPnP: Unhandled Control Point event"); + // JAMI_WARN("PUPnP: Unhandled Control Point event"); break; } } @@ -1036,7 +1036,7 @@ PUPnP::subEventCallback(Upnp_EventType event_type, const void* event, void* user { if (auto pupnp = static_cast<PUPnP*>(user_data)) return pupnp->handleSubscriptionUPnPEvent(event_type, event); - JAMI_WARN("PUPnP: Subscription callback without service Id string"); + // JAMI_WARN("PUPnP: Subscription callback without service Id string"); return 0; } @@ -1046,15 +1046,15 @@ PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType, const void* event) UpnpEventSubscribe* es_event = static_cast<UpnpEventSubscribe*>(const_cast<void*>(event)); if (es_event == nullptr) { - JAMI_ERR("PUPnP: Unexpected null pointer!"); + // JAMI_ERR("PUPnP: Unexpected null pointer!"); return UPNP_E_INVALID_ARGUMENT; } std::string publisherUrl(UpnpEventSubscribe_get_PublisherUrl_cstr(es_event)); int upnp_err = UpnpEventSubscribe_get_ErrCode(es_event); if (upnp_err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Subscription error %s from %s", - UpnpGetErrorMessage(upnp_err), - publisherUrl.c_str()); + // JAMI_WARN("PUPnP: Subscription error %s from %s", + // UpnpGetErrorMessage(upnp_err), + // publisherUrl.c_str()); return upnp_err; } @@ -1070,7 +1070,7 @@ PUPnP::parseIgd(IXML_Document* doc, std::string locationUrl) // Check the UDN to see if its already in our device list. std::string UDN(getFirstDocItem(doc, "UDN")); if (UDN.empty()) { - JAMI_WARN("PUPnP: could not find UDN in description document of device"); + // JAMI_WARN("PUPnP: could not find UDN in description document of device"); return nullptr; } else { std::lock_guard<std::mutex> lk(pupnpMutex_); @@ -1082,7 +1082,7 @@ PUPnP::parseIgd(IXML_Document* doc, std::string locationUrl) } } - JAMI_DBG("PUPnP: Found new device [%s]", UDN.c_str()); + // JAMI_DBG("PUPnP: Found new device [%s]", UDN.c_str()); std::unique_ptr<UPnPIGD> new_igd; int upnp_err; @@ -1146,15 +1146,15 @@ PUPnP::parseIgd(IXML_Document* doc, std::string locationUrl) if (upnp_err == UPNP_E_SUCCESS) controlURL = absolute_control_url; else - JAMI_WARN("PUPnP: Error resolving absolute controlURL -> %s", - UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Error resolving absolute controlURL -> %s", + // UpnpGetErrorMessage(upnp_err)); std::free(absolute_control_url); // Get the relative eventSubURL and turn it into absolute address using the URLBase. std::string eventSubURL(getFirstElementItem(service_element, "eventSubURL")); if (eventSubURL.empty()) { - JAMI_WARN("PUPnP: IGD event sub URL is empty. Going to next node"); + // JAMI_WARN("PUPnP: IGD event sub URL is empty. Going to next node"); continue; } @@ -1163,8 +1163,8 @@ PUPnP::parseIgd(IXML_Document* doc, std::string locationUrl) if (upnp_err == UPNP_E_SUCCESS) eventSubURL = absolute_event_sub_url; else - JAMI_WARN("PUPnP: Error resolving absolute eventSubURL -> %s", - UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Error resolving absolute eventSubURL -> %s", + // UpnpGetErrorMessage(upnp_err)); std::free(absolute_event_sub_url); @@ -1195,7 +1195,7 @@ PUPnP::actionIsIgdConnected(const UPnPIGD& igd) 0, nullptr); if (not action_container_ptr) { - JAMI_WARN("PUPnP: Failed to make GetStatusInfo action"); + // JAMI_WARN("PUPnP: Failed to make GetStatusInfo action"); return false; } XMLDocument action(action_container_ptr, ixmlDocument_free); // Action pointer. @@ -1208,16 +1208,16 @@ PUPnP::actionIsIgdConnected(const UPnPIGD& igd) action.get(), &response_container_ptr); if (not response_container_ptr or upnp_err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Failed to send GetStatusInfo action -> %s", UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Failed to send GetStatusInfo action -> %s", UpnpGetErrorMessage(upnp_err)); return false; } XMLDocument response(response_container_ptr, ixmlDocument_free); if (errorOnResponse(response.get())) { - JAMI_WARN("PUPnP: Failed to get GetStatusInfo from %s -> %d: %s", - igd.getServiceType().c_str(), - upnp_err, - UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Failed to get GetStatusInfo from %s -> %d: %s", + // igd.getServiceType().c_str(), + // upnp_err, + // UpnpGetErrorMessage(upnp_err)); return false; } @@ -1246,7 +1246,7 @@ PUPnP::actionGetExternalIP(const UPnPIGD& igd) action.reset(action_container_ptr); if (not action) { - JAMI_WARN("PUPnP: Failed to make GetExternalIPAddress action"); + // JAMI_WARN("PUPnP: Failed to make GetExternalIPAddress action"); return {}; } @@ -1260,16 +1260,16 @@ PUPnP::actionGetExternalIP(const UPnPIGD& igd) response.reset(response_container_ptr); if (not response or upnp_err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Failed to send GetExternalIPAddress action -> %s", - UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Failed to send GetExternalIPAddress action -> %s", + // UpnpGetErrorMessage(upnp_err)); return {}; } if (errorOnResponse(response.get())) { - JAMI_WARN("PUPnP: Failed to get GetExternalIPAddress from %s -> %d: %s", - igd.getServiceType().c_str(), - upnp_err, - UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: Failed to get GetExternalIPAddress from %s -> %d: %s", + // igd.getServiceType().c_str(), + // upnp_err, + // UpnpGetErrorMessage(upnp_err)); return {}; } @@ -1307,7 +1307,7 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string action.reset(action_container_ptr); if (not action) { - JAMI_WARN("PUPnP: Failed to add NewPortMappingIndex action"); + // JAMI_WARN("PUPnP: Failed to add NewPortMappingIndex action"); break; } @@ -1325,7 +1325,7 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string } if (upnp_err != UPNP_E_SUCCESS) { - JAMI_ERR("PUPnP: GetGenericPortMappingEntry returned with error: %i", upnp_err); + // JAMI_ERR("PUPnP: GetGenericPortMappingEntry returned with error: %i", upnp_err); break; } @@ -1335,7 +1335,7 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string auto error = to_int<int>(errorCode); if (error == ARRAY_IDX_INVALID or error == CONFLICT_IN_MAPPING) { // No more port mapping entries in the response. - JAMI_DBG("PUPnP: No more mappings (found a total of %i mappings", entry_idx); + // JAMI_DBG("PUPnP: No more mappings (found a total of %i mappings", entry_idx); break; } else { auto errorDescription = getFirstDocItem(response.get(), "errorDescription"); @@ -1363,8 +1363,8 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string std::string transport(getFirstDocItem(response.get(), "NewProtocol")); if (port_internal.empty() || port_external.empty() || transport.empty()) { - JAMI_ERR("PUPnP: GetGenericPortMappingEntry returned an invalid entry at index %i", - entry_idx); + // JAMI_ERR("PUPnP: GetGenericPortMappingEntry returned an invalid entry at index %i", + // entry_idx); continue; } @@ -1392,9 +1392,9 @@ PUPnP::deleteMappingsByDescription(const std::shared_ptr<IGD>& igd, const std::s if (not(clientRegistered_ and igd->getLocalIp())) return; - JAMI_DBG("PUPnP: Remove all mappings (if any) on IGD %s matching descr prefix %s", - igd->toString().c_str(), - Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX); + // JAMI_DBG("PUPnP: Remove all mappings (if any) on IGD %s matching descr prefix %s", + // igd->toString().c_str(), + // Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX); auto mapList = getMappingsListByDescr(igd, description); @@ -1482,13 +1482,13 @@ PUPnP::actionAddPortMapping(const Mapping& mapping) bool success = true; if (upnp_err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Failed to send action %s for mapping %s. %d: %s", - ACTION_ADD_PORT_MAPPING, - mapping.toString().c_str(), - upnp_err, - UpnpGetErrorMessage(upnp_err)); - JAMI_WARN("PUPnP: IGD ctrlUrl %s", igd->getControlURL().c_str()); - JAMI_WARN("PUPnP: IGD service type %s", igd->getServiceType().c_str()); + // JAMI_WARN("PUPnP: Failed to send action %s for mapping %s. %d: %s", + // ACTION_ADD_PORT_MAPPING, + // mapping.toString().c_str(), + // upnp_err, + // UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: IGD ctrlUrl %s", igd->getControlURL().c_str()); + // JAMI_WARN("PUPnP: IGD service type %s", igd->getServiceType().c_str()); success = false; } @@ -1503,10 +1503,10 @@ PUPnP::actionAddPortMapping(const Mapping& mapping) errorDescription = getFirstDocItem(response.get(), "errorDescription"); } - JAMI_WARNING("PUPnP: {:s} returned with error: {:s} {:s}", - ACTION_ADD_PORT_MAPPING, - errorCode, - errorDescription); + // JAMI_WARNING("PUPnP: {:s} returned with error: {:s} {:s}", + // ACTION_ADD_PORT_MAPPING, + // errorCode, + // errorDescription); } return success; } @@ -1565,19 +1565,19 @@ PUPnP::actionDeletePortMapping(const Mapping& mapping) bool success = true; if (upnp_err != UPNP_E_SUCCESS) { - JAMI_WARN("PUPnP: Failed to send action %s for mapping from %s. %d: %s", - ACTION_DELETE_PORT_MAPPING, - mapping.toString().c_str(), - upnp_err, - UpnpGetErrorMessage(upnp_err)); - JAMI_WARN("PUPnP: IGD ctrlUrl %s", igd->getControlURL().c_str()); - JAMI_WARN("PUPnP: IGD service type %s", igd->getServiceType().c_str()); + // JAMI_WARN("PUPnP: Failed to send action %s for mapping from %s. %d: %s", + // ACTION_DELETE_PORT_MAPPING, + // mapping.toString().c_str(), + // upnp_err, + // UpnpGetErrorMessage(upnp_err)); + // JAMI_WARN("PUPnP: IGD ctrlUrl %s", igd->getControlURL().c_str()); + // JAMI_WARN("PUPnP: IGD service type %s", igd->getServiceType().c_str()); success = false; } if (not response) { - JAMI_WARN("PUPnP: Failed to get response for %s", ACTION_DELETE_PORT_MAPPING); + // JAMI_WARN("PUPnP: Failed to get response for %s", ACTION_DELETE_PORT_MAPPING); success = false; } @@ -1585,10 +1585,10 @@ PUPnP::actionDeletePortMapping(const Mapping& mapping) auto errorCode = getFirstDocItem(response.get(), "errorCode"); if (not errorCode.empty()) { auto errorDescription = getFirstDocItem(response.get(), "errorDescription"); - JAMI_WARNING("PUPnP: {:s} returned with error: {:s}: {:s}", - ACTION_DELETE_PORT_MAPPING, - errorCode, - errorDescription); + // JAMI_WARNING("PUPnP: {:s} returned with error: {:s}: {:s}", + // ACTION_DELETE_PORT_MAPPING, + // errorCode, + // errorDescription); success = false; } diff --git a/src/upnp/protocol/upnp_protocol.h b/src/upnp/protocol/upnp_protocol.h index b38a4dd..8ce0820 100644 --- a/src/upnp/protocol/upnp_protocol.h +++ b/src/upnp/protocol/upnp_protocol.h @@ -24,7 +24,6 @@ #include "igd.h" #include "mapping.h" #include "ip_utils.h" -//#include "upnp/upnp_thread_util.h" #include <map> #include <string> @@ -47,27 +46,6 @@ constexpr static const char* UPNP_WANIP_SERVICE = "urn:schemas-upnp-org:service: constexpr static const char* UPNP_WANPPP_SERVICE = "urn:schemas-upnp-org:service:WANPPPConnection:1"; -enum class UpnpIgdEvent { ADDED, REMOVED, INVALID_STATE }; - -// Interface used to report mapping event from the protocol implementations. -// This interface is meant to be implemented only by UPnPConext class. Sincce -// this class is a singleton, it's assumed that it out-lives the protocol -// implementations. In other words, the observer is always assumed to point to a -// valid instance. -class UpnpMappingObserver -{ -public: - UpnpMappingObserver() {}; - virtual ~UpnpMappingObserver() {}; - - virtual void onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) = 0; - virtual void onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; - virtual void onMappingRequestFailed(const Mapping& map) = 0; -#if HAVE_LIBNATPMP - virtual void onMappingRenewed(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; -#endif - virtual void onMappingRemoved(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; -}; // Pure virtual interface class that UPnPContext uses to call protocol functions. class UPnPProtocol : public std::enable_shared_from_this<UPnPProtocol>//, protected UpnpThreadUtil diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp index ef556f1..11112b7 100644 --- a/src/upnp/upnp_context.cpp +++ b/src/upnp/upnp_context.cpp @@ -20,7 +20,8 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "upnp_context.h" +#include "upnp/upnp_context.h" +#include <asio/steady_timer.hpp> namespace jami { namespace upnp { @@ -36,7 +37,7 @@ constexpr static uint16_t UPNP_UDP_PORT_MAX {UPNP_UDP_PORT_MIN + 5000}; UPnPContext::UPnPContext() { - JAMI_DBG("Creating UPnPContext instance [%p]", this); + // JAMI_DBG("Creating UPnPContext instance [%p]", this); // Set port ranges portRange_.emplace(PortType::TCP, std::make_pair(UPNP_TCP_PORT_MIN, UPNP_TCP_PORT_MAX)); @@ -59,7 +60,7 @@ UPnPContext::getUPnPContext() void UPnPContext::shutdown(std::condition_variable& cv) { - JAMI_DBG("Shutdown UPnPContext instance [%p]", this); + // JAMI_DBG("Shutdown UPnPContext instance [%p]", this); stopUpnp(true); @@ -87,18 +88,18 @@ UPnPContext::shutdown() runOnUpnpContextQueue([&, this] { shutdown(cv); }); - JAMI_DBG("Waiting for shutdown ..."); + // JAMI_DBG("Waiting for shutdown ..."); if (cv.wait_for(lk, std::chrono::seconds(30), [this] { return shutdownComplete_; })) { - JAMI_DBG("Shutdown completed"); + // JAMI_DBG("Shutdown completed"); } else { - JAMI_ERR("Shutdown timed-out"); + // JAMI_ERR("Shutdown timed-out"); } } UPnPContext::~UPnPContext() { - JAMI_DBG("UPnPContext instance [%p] destroyed", this); + // JAMI_DBG("UPnPContext instance [%p] destroyed", this); } void @@ -127,7 +128,7 @@ UPnPContext::startUpnp() CHECK_VALID_THREAD(); - JAMI_DBG("Starting UPNP context"); + // JAMI_DBG("Starting UPNP context"); // Request a new IGD search. for (auto const& [_, protocol] : protocolList_) { @@ -145,7 +146,7 @@ UPnPContext::stopUpnp(bool forceRelease) return; } - JAMI_DBG("Stopping UPNP context"); + // JAMI_DBG("Stopping UPNP context"); // Clear all current mappings if any. @@ -199,7 +200,7 @@ UPnPContext::generateRandomPort(PortType type, bool mustBeEven) auto maxPort = type == PortType::TCP ? UPNP_TCP_PORT_MAX : UPNP_UDP_PORT_MAX; if (minPort >= maxPort) { - JAMI_ERR("Max port number (%i) must be greater than min port number (%i)", maxPort, minPort); + // JAMI_ERR("Max port number (%i) must be greater than min port number (%i)", maxPort, minPort); // Must be called with valid range. assert(false); } @@ -227,7 +228,7 @@ UPnPContext::connectivityChanged() auto hostAddr = ip_utils::getLocalAddr(AF_INET); - JAMI_DBG("Connectivity change check: host address %s", hostAddr.toString().c_str()); + // JAMI_DBG("Connectivity change check: host address %s", hostAddr.toString().c_str()); auto restartUpnp = false; @@ -241,9 +242,9 @@ UPnPContext::connectivityChanged() // Check if the host address changed. for (auto const& [_, protocol] : protocolList_) { if (protocol->isReady() and hostAddr != protocol->getHostAddress()) { - JAMI_WARN("Host address changed from %s to %s", - protocol->getHostAddress().toString().c_str(), - hostAddr.toString().c_str()); + // JAMI_WARN("Host address changed from %s to %s", + // protocol->getHostAddress().toString().c_str(), + // hostAddr.toString().c_str()); protocol->clearIgds(); restartUpnp = true; break; @@ -262,7 +263,7 @@ UPnPContext::connectivityChanged() if (controllerList_.empty()) return; - JAMI_DBG("Connectivity changed. Clear the IGDs and restart"); + // JAMI_DBG("Connectivity changed. Clear the IGDs and restart"); stopUpnp(); startUpnp(); @@ -280,7 +281,7 @@ UPnPContext::setPublicAddress(const IpAddr& addr) std::lock_guard<std::mutex> lock(mappingMutex_); if (knownPublicAddress_ != addr) { knownPublicAddress_ = std::move(addr); - JAMI_DBG("Setting the known public address to %s", addr.toString().c_str()); + // JAMI_DBG("Setting the known public address to %s", addr.toString().c_str()); } } @@ -308,10 +309,10 @@ UPnPContext::reserveMapping(Mapping& requestedMap) auto desiredPort = requestedMap.getExternalPort(); if (desiredPort == 0) { - JAMI_DBG("Desired port is not set, will provide the first available port for [%s]", + // JAMI_DBG("Desired port is not set, will provide the first available port for [%s]", requestedMap.getTypeStr()); } else { - JAMI_DBG("Try to find mapping for port %i [%s]", desiredPort, requestedMap.getTypeStr()); + // JAMI_DBG("Try to find mapping for port %i [%s]", desiredPort, requestedMap.getTypeStr()); } Mapping::sharedPtr_t mapRes; @@ -343,7 +344,7 @@ UPnPContext::reserveMapping(Mapping& requestedMap) // Create a mapping if none was available. if (not mapRes) { - JAMI_WARN("Did not find any available mapping. Will request one now"); + // JAMI_WARN("Did not find any available mapping. Will request one now"); mapRes = registerMapping(requestedMap); } @@ -375,12 +376,12 @@ UPnPContext::releaseMapping(const Mapping& map) if (not mapPtr) { // Might happen if the mapping failed or was never granted. - JAMI_DBG("Mapping %s does not exist or was already removed", map.toString().c_str()); + // JAMI_DBG("Mapping %s does not exist or was already removed", map.toString().c_str()); return; } if (mapPtr->isAvailable()) { - JAMI_WARN("Trying to release an unused mapping %s", mapPtr->toString().c_str()); + // JAMI_WARN("Trying to release an unused mapping %s", mapPtr->toString().c_str()); return; } @@ -395,7 +396,7 @@ UPnPContext::registerController(void* controller) { std::lock_guard<std::mutex> lock(mappingMutex_); if (shutdownComplete_) { - JAMI_WARN("UPnPContext already shut down"); + // JAMI_WARN("UPnPContext already shut down"); return; } } @@ -407,11 +408,11 @@ UPnPContext::registerController(void* controller) auto ret = controllerList_.emplace(controller); if (not ret.second) { - JAMI_WARN("Controller %p is already registered", controller); + // JAMI_WARN("Controller %p is already registered", controller); return; } - JAMI_DBG("Successfully registered controller %p", controller); + // JAMI_DBG("Successfully registered controller %p", controller); if (not started_) startUpnp(); } @@ -425,9 +426,9 @@ UPnPContext::unregisterController(void* controller) } if (controllerList_.erase(controller) == 1) { - JAMI_DBG("Successfully unregistered controller %p", controller); + // JAMI_DBG("Successfully unregistered controller %p", controller); } else { - JAMI_DBG("Controller %p was already removed", controller); + // JAMI_DBG("Controller %p was already removed", controller); } if (controllerList_.empty()) { @@ -452,7 +453,7 @@ UPnPContext::getAvailablePortNumber(PortType type) } // Very unlikely to get here. - JAMI_ERR("Could not find an available port after %i trials", MAX_REQUEST_RETRIES); + // JAMI_ERR("Could not find an available port after %i trials", MAX_REQUEST_RETRIES); return 0; } @@ -472,16 +473,16 @@ UPnPContext::requestMapping(const Mapping::sharedPtr_t& map) // because the processing is asynchronous, it's possible that the IGD // was invalidated when the this code executed. if (not igd) { - JAMI_DBG("No valid IGDs available"); + // JAMI_DBG("No valid IGDs available"); return; } map->setIgd(igd); - JAMI_DBG("Request mapping %s using protocol [%s] IGD [%s]", - map->toString().c_str(), - igd->getProtocolName(), - igd->toString().c_str()); + // JAMI_DBG("Request mapping %s using protocol [%s] IGD [%s]", + // map->toString().c_str(), + // igd->getProtocolName(), + // igd->toString().c_str()); if (map->getState() != MappingState::IN_PROGRESS) updateMappingState(map, MappingState::IN_PROGRESS); @@ -493,7 +494,7 @@ UPnPContext::requestMapping(const Mapping::sharedPtr_t& map) bool UPnPContext::provisionNewMappings(PortType type, int portCount) { - JAMI_DBG("Provision %i new mappings of type [%s]", portCount, Mapping::getTypeStr(type)); + // JAMI_DBG("Provision %i new mappings of type [%s]", portCount, Mapping::getTypeStr(type)); assert(portCount > 0); @@ -506,7 +507,7 @@ UPnPContext::provisionNewMappings(PortType type, int portCount) registerMapping(map); } else { // Very unlikely to get here! - JAMI_ERR("Can not find any available port to provision!"); + // JAMI_ERR("Can not find any available port to provision!"); return false; } } @@ -517,7 +518,7 @@ UPnPContext::provisionNewMappings(PortType type, int portCount) bool UPnPContext::deleteUnneededMappings(PortType type, int portCount) { - JAMI_DBG("Remove %i unneeded mapping of type [%s]", portCount, Mapping::getTypeStr(type)); + // JAMI_DBG("Remove %i unneeded mapping of type [%s]", portCount, Mapping::getTypeStr(type)); assert(portCount > 0); @@ -583,10 +584,10 @@ UPnPContext::updatePreferredIgd() } if (preferredIgd_ and preferredIgd_->isValid()) { - JAMI_DBG("Preferred IGD updated to [%s] IGD [%s %s] ", - preferredIgd_->getProtocolName(), - preferredIgd_->getUID().c_str(), - preferredIgd_->toString().c_str()); + // JAMI_DBG("Preferred IGD updated to [%s] IGD [%s %s] ", + // preferredIgd_->getProtocolName(), + // preferredIgd_->getUID().c_str(), + // preferredIgd_->toString().c_str()); } } @@ -624,7 +625,7 @@ UPnPContext::updateMappingList(bool async) // Cancel the current timer (if any) and re-schedule. std::shared_ptr<IGD> prefIgd = getPreferredIgd(); if (not prefIgd) { - JAMI_DBG("UPNP/NAT-PMP enabled, but no valid IGDs available"); + // JAMI_DBG("UPNP/NAT-PMP enabled, but no valid IGDs available"); // No valid IGD. Nothing to do. return; } @@ -647,25 +648,25 @@ UPnPContext::updateMappingList(bool async) MappingStatus status; getMappingStatus(type, status); - JAMI_DBG("Mapping status [%s] - overall %i: %i open (%i ready + %i in use), %i pending, %i " - "in-progress, %i failed", - Mapping::getTypeStr(type), - status.sum(), - status.openCount_, - status.readyCount_, - status.openCount_ - status.readyCount_, - status.pendingCount_, - status.inProgressCount_, - status.failedCount_); + // JAMI_DBG("Mapping status [%s] - overall %i: %i open (%i ready + %i in use), %i pending, %i " + // "in-progress, %i failed", + // Mapping::getTypeStr(type), + // status.sum(), + // status.openCount_, + // status.readyCount_, + // status.openCount_ - status.readyCount_, + // status.pendingCount_, + // status.inProgressCount_, + // status.failedCount_); if (status.failedCount_ > 0) { std::lock_guard<std::mutex> lock(mappingMutex_); auto const& mappingList = getMappingList(type); for (auto const& [_, map] : mappingList) { if (map->getState() == MappingState::FAILED) { - JAMI_DBG("Mapping status [%s] - Available [%s]", - map->toString(true).c_str(), - map->isAvailable() ? "YES" : "NO"); + // JAMI_DBG("Mapping status [%s] - Available [%s]", + // map->toString(true).c_str(), + // map->isAvailable() ? "YES" : "NO"); } } } @@ -726,7 +727,7 @@ UPnPContext::pruneMappingList() if (remoteMapList.empty()) { std::lock_guard<std::mutex> lock(mappingMutex_); if (not getMappingList(PortType::TCP).empty() or getMappingList(PortType::TCP).empty()) { - JAMI_WARN("We have provisionned mappings but the PUPNP IGD returned an empty list!"); + // JAMI_WARN("We have provisionned mappings but the PUPNP IGD returned an empty list!"); } } @@ -760,10 +761,10 @@ UPnPContext::pruneUnMatchedMappings(const std::shared_ptr<IGD>& igd, and remoteMapList.find(map->getMapKey()) == remoteMapList.end()) { toRemoveList.emplace_back(map); - JAMI_WARN("Mapping %s (IGD %s) marked as \"OPEN\" but not found in the " - "remote list. Mark as failed!", - map->toString().c_str(), - igd->toString().c_str()); + // JAMI_WARN("Mapping %s (IGD %s) marked as \"OPEN\" but not found in the " + // "remote list. Mark as failed!", + // map->toString().c_str(), + // igd->toString().c_str()); } } } @@ -829,10 +830,10 @@ UPnPContext::pruneMappingsWithInvalidIgds(const std::shared_ptr<IGD>& igd) } for (auto const& map : toRemoveList) { - JAMI_DBG("Remove mapping %s (has an invalid IGD %s [%s])", - map->toString().c_str(), - igd->toString().c_str(), - igd->getProtocolName()); + // JAMI_DBG("Remove mapping %s (has an invalid IGD %s [%s])", + // map->toString().c_str(), + // igd->toString().c_str(), + // igd->getProtocolName()); updateMappingState(map, MappingState::FAILED); unregisterMapping(map); } @@ -855,9 +856,9 @@ UPnPContext::processPendingRequests(const std::shared_ptr<IGD>& igd) auto& mappingList = getMappingList(type); for (auto& [_, map] : mappingList) { if (map->getState() == MappingState::PENDING) { - JAMI_DBG("Send pending request for mapping %s to IGD %s", - map->toString().c_str(), - igd->toString().c_str()); + // JAMI_DBG("Send pending request for mapping %s to IGD %s", + // map->toString().c_str(), + // igd->toString().c_str()); requestsList.emplace_back(map); } } @@ -895,8 +896,8 @@ UPnPContext::processMappingWithAutoUpdate() for (auto const& oldMap : requestsList) { // Request a new mapping if auto-update is enabled. - JAMI_DBG("Mapping %s has auto-update enabled, a new mapping will be requested", - oldMap->toString().c_str()); + // JAMI_DBG("Mapping %s has auto-update enabled, a new mapping will be requested", + // oldMap->toString().c_str()); // Reserve a new mapping. Mapping newMapping(oldMap->getType()); @@ -934,38 +935,38 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) auto const& igdLocalAddr = igd->getLocalIp(); auto protocolName = igd->getProtocolName(); - JAMI_DBG("New event for IGD [%s %s] [%s]: [%s]", - igd->getUID().c_str(), - igd->toString().c_str(), - protocolName, - IgdState); + // JAMI_DBG("New event for IGD [%s %s] [%s]: [%s]", + // igd->getUID().c_str(), + // igd->toString().c_str(), + // protocolName, + // IgdState); // Check if the IGD has valid addresses. if (not igdLocalAddr) { - JAMI_WARN("[%s] IGD has an invalid local address", protocolName); + // JAMI_WARN("[%s] IGD has an invalid local address", protocolName); return; } if (not igd->getPublicIp()) { - JAMI_WARN("[%s] IGD has an invalid public address", protocolName); + // JAMI_WARN("[%s] IGD has an invalid public address", protocolName); return; } if (knownPublicAddress_ and igd->getPublicIp() != knownPublicAddress_) { - JAMI_WARN("[%s] IGD external address [%s] does not match known public address [%s]." - " The mapped addresses might not be reachable", - protocolName, - igd->getPublicIp().toString().c_str(), - knownPublicAddress_.toString().c_str()); + // JAMI_WARN("[%s] IGD external address [%s] does not match known public address [%s]." + // " The mapped addresses might not be reachable", + // protocolName, + // igd->getPublicIp().toString().c_str(), + // knownPublicAddress_.toString().c_str()); } // The IGD was removed or is invalid. if (event == UpnpIgdEvent::REMOVED or event == UpnpIgdEvent::INVALID_STATE) { - JAMI_WARN("State of IGD [%s %s] [%s] changed to [%s]. Pruning the mapping list", - igd->getUID().c_str(), - igd->toString().c_str(), - protocolName, - IgdState); + // JAMI_WARN("State of IGD [%s %s] [%s] changed to [%s]. Pruning the mapping list", + // igd->getUID().c_str(), + // igd->toString().c_str(), + // protocolName, + // IgdState); pruneMappingsWithInvalidIgds(igd); @@ -979,14 +980,14 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) std::lock_guard<std::mutex> lock(mappingMutex_); auto ret = validIgdList_.emplace(igd); if (ret.second) { - JAMI_DBG("IGD [%s] on address %s was added. Will process any pending requests", - protocolName, - igdLocalAddr.toString(true, true).c_str()); + // JAMI_DBG("IGD [%s] on address %s was added. Will process any pending requests", + // protocolName, + // igdLocalAddr.toString(true, true).c_str()); } else { // Already in the list. - JAMI_ERR("IGD [%s] on address %s already in the list", - protocolName, - igdLocalAddr.toString(true, true).c_str()); + // JAMI_ERR("IGD [%s] on address %s already in the list", + // protocolName, + // igdLocalAddr.toString(true, true).c_str()); return; } } @@ -1004,10 +1005,10 @@ UPnPContext::onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& mapR auto map = getMappingWithKey(mapRes.getMapKey()); if (not map) { // We may receive a response for a canceled request. Just ignore it. - JAMI_DBG("Response for mapping %s [IGD %s] [%s] does not have a local match", - mapRes.toString().c_str(), - igd->toString().c_str(), - mapRes.getProtocolName()); + // JAMI_DBG("Response for mapping %s [IGD %s] [%s] does not have a local match", + // mapRes.toString().c_str(), + // igd->toString().c_str(), + // mapRes.getProtocolName()); return; } @@ -1019,10 +1020,10 @@ UPnPContext::onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& mapR // Update the state and report to the owner. updateMappingState(map, MappingState::OPEN); - JAMI_DBG("Mapping %s (on IGD %s [%s]) successfully performed", - map->toString().c_str(), - igd->toString().c_str(), - map->getProtocolName()); + // JAMI_DBG("Mapping %s (on IGD %s [%s]) successfully performed", + // map->toString().c_str(), + // igd->toString().c_str(), + // map->getProtocolName()); // Call setValid() to reset the errors counter. We need // to reset the counter on each successful response. @@ -1037,18 +1038,18 @@ UPnPContext::onMappingRenewed(const std::shared_ptr<IGD>& igd, const Mapping& ma if (not mapPtr) { // We may receive a notification for a canceled request. Ignore it. - JAMI_WARN("Renewed mapping %s from IGD %s [%s] does not have a match in local list", - map.toString().c_str(), - igd->toString().c_str(), - map.getProtocolName()); + // JAMI_WARN("Renewed mapping %s from IGD %s [%s] does not have a match in local list", + // map.toString().c_str(), + // igd->toString().c_str(), + // map.getProtocolName()); return; } if (mapPtr->getProtocol() != NatProtocolType::NAT_PMP or not mapPtr->isValid() or mapPtr->getState() != MappingState::OPEN) { - JAMI_WARN("Renewed mapping %s from IGD %s [%s] is in unexpected state", - mapPtr->toString().c_str(), - igd->toString().c_str(), - mapPtr->getProtocolName()); + // JAMI_WARN("Renewed mapping %s from IGD %s [%s] is in unexpected state", + // mapPtr->toString().c_str(), + // igd->toString().c_str(), + // mapPtr->getProtocolName()); return; } @@ -1062,7 +1063,7 @@ UPnPContext::requestRemoveMapping(const Mapping::sharedPtr_t& map) CHECK_VALID_THREAD(); if (not map) { - JAMI_ERR("Mapping shared pointer is null!"); + // JAMI_ERR("Mapping shared pointer is null!"); return; } @@ -1112,7 +1113,7 @@ Mapping::sharedPtr_t UPnPContext::registerMapping(Mapping& map) { if (map.getExternalPort() == 0) { - JAMI_DBG("Port number not set. Will set a random port number"); + // JAMI_DBG("Port number not set. Will set a random port number"); auto port = getAvailablePortNumber(map.getType()); map.setExternalPort(port); map.setInternalPort(port); @@ -1129,7 +1130,7 @@ UPnPContext::registerMapping(Mapping& map) auto ret = mappingList.emplace(map.getMapKey(), std::make_shared<Mapping>(map)); if (not ret.second) { - JAMI_WARN("Mapping request for %s already added!", map.toString().c_str()); + // JAMI_WARN("Mapping request for %s already added!", map.toString().c_str()); return {}; } mapPtr = ret.first->second; @@ -1139,7 +1140,7 @@ UPnPContext::registerMapping(Mapping& map) // No available IGD. The pending mapping requests will be processed // when a IGD becomes available (in onIgdAdded() method). if (not isReady()) { - JAMI_WARN("No IGD available. Mapping will be requested when an IGD becomes available"); + // JAMI_WARN("No IGD available. Mapping will be requested when an IGD becomes available"); } else { requestMapping(mapPtr); } @@ -1166,7 +1167,7 @@ UPnPContext::unregisterMapping(const Mapping::sharedPtr_t& map) CHECK_VALID_THREAD(); if (not map) { - JAMI_ERR("Mapping pointer is null"); + // JAMI_ERR("Mapping pointer is null"); return; } @@ -1177,12 +1178,12 @@ UPnPContext::unregisterMapping(const Mapping::sharedPtr_t& map) auto& mappingList = getMappingList(map->getType()); if (mappingList.erase(map->getMapKey()) == 1) { - JAMI_DBG("Unregistered mapping %s", map->toString().c_str()); + // JAMI_DBG("Unregistered mapping %s", map->toString().c_str()); } else { // The mapping may already be un-registered. Just ignore it. - JAMI_DBG("Mapping %s [%s] does not have a local match", - map->toString().c_str(), - map->getProtocolName()); + // JAMI_DBG("Mapping %s [%s] does not have a local match", + // map->toString().c_str(), + // map->getProtocolName()); } } @@ -1254,25 +1255,25 @@ UPnPContext::onMappingRequestFailed(const Mapping& mapRes) auto const& map = getMappingWithKey(mapRes.getMapKey()); if (not map) { // We may receive a response for a removed request. Just ignore it. - JAMI_DBG("Mapping %s [IGD %s] does not have a local match", - mapRes.toString().c_str(), - mapRes.getProtocolName()); + // JAMI_DBG("Mapping %s [IGD %s] does not have a local match", + // mapRes.toString().c_str(), + // mapRes.getProtocolName()); return; } auto igd = map->getIgd(); if (not igd) { - JAMI_ERR("IGD pointer is null"); + // JAMI_ERR("IGD pointer is null"); return; } updateMappingState(map, MappingState::FAILED); unregisterMapping(map); - JAMI_WARN("Mapping request for %s failed on IGD %s [%s]", - map->toString().c_str(), - igd->toString().c_str(), - igd->getProtocolName()); + // JAMI_WARN("Mapping request for %s failed on IGD %s [%s]", + // map->toString().c_str(), + // igd->toString().c_str(), + // igd->getProtocolName()); } void @@ -1284,7 +1285,7 @@ UPnPContext::updateMappingState(const Mapping::sharedPtr_t& map, MappingState ne // Ignore if the state did not change. if (newState == map->getState()) { - JAMI_DBG("Mapping %s already in state %s", map->toString().c_str(), map->getStateStr()); + // JAMI_DBG("Mapping %s already in state %s", map->toString().c_str(), map->getStateStr()); return; } diff --git a/src/upnp/upnp_control.cpp b/src/upnp/upnp_control.cpp index b255617..30de6da 100644 --- a/src/upnp/upnp_control.cpp +++ b/src/upnp/upnp_control.cpp @@ -20,7 +20,8 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "upnp_control.h" +#include "upnp/upnp_control.h" +#include "upnp/upnp_context.h" namespace jami { namespace upnp { @@ -30,18 +31,18 @@ Controller::Controller() try { upnpContext_ = UPnPContext::getUPnPContext(); } catch (std::runtime_error& e) { - JAMI_ERR("UPnP context error: %s", e.what()); + // JAMI_ERR("UPnP context error: %s", e.what()); } assert(upnpContext_); upnpContext_->registerController(this); - JAMI_DBG("Controller@%p: Created UPnP Controller session", this); + // JAMI_DBG("Controller@%p: Created UPnP Controller session", this); } Controller::~Controller() { - JAMI_DBG("Controller@%p: Destroying UPnP Controller session", this); + // JAMI_DBG("Controller@%p: Destroying UPnP Controller session", this); releaseAllMappings(); upnpContext_->unregisterController(this); @@ -121,7 +122,7 @@ Controller::addLocalMap(const Mapping& map) std::lock_guard<std::mutex> lock(mapListMutex_); auto ret = mappingList_.emplace(map.getMapKey(), map); if (not ret.second) { - JAMI_WARN("Mapping request for %s already in the list!", map.toString().c_str()); + // JAMI_WARN("Mapping request for %s already in the list!", map.toString().c_str()); } } } @@ -133,7 +134,7 @@ Controller::removeLocalMap(const Mapping& map) std::lock_guard<std::mutex> lk(mapListMutex_); if (mappingList_.erase(map.getMapKey()) != 1) { - JAMI_ERR("Failed to remove mapping %s from local list", map.getTypeStr()); + // JAMI_ERR("Failed to remove mapping %s from local list", map.getTypeStr()); return false; } -- GitLab