diff --git a/src/account.cpp b/src/account.cpp
index b9a70ae8dd464bde961cbde725e636159b5766f7..a391b395f58f37a7f2ce21bb3982066bed7ccac3 100644
--- a/src/account.cpp
+++ b/src/account.cpp
@@ -543,7 +543,7 @@ Account::getUPnPActive() const
{
std::lock_guard<std::mutex> lk(upnp_mtx);
if (upnpCtrl_)
- return upnpCtrl_->hasValidIGD();
+ return upnpCtrl_->isReady();
return false;
}
diff --git a/src/ice_transport.cpp b/src/ice_transport.cpp
index 146eaceae8bc11f8cb001d973f9a98e65573bcc0..cabb535ab0ec6ca687868581079a5db5abb80e89 100644
--- a/src/ice_transport.cpp
+++ b/src/ice_transport.cpp
@@ -552,9 +552,9 @@ IceTransport::Impl::handleEvents(unsigned max_msec)
void
IceTransport::Impl::onComplete(pj_ice_strans* ice_st, pj_ice_strans_op op, pj_status_t status)
{
- const char* opname = op == PJ_ICE_STRANS_OP_INIT ? "initialization"
- : op == PJ_ICE_STRANS_OP_NEGOTIATION ? "negotiation"
- : "unknown_op";
+ const char* opname = op == PJ_ICE_STRANS_OP_INIT
+ ? "initialization"
+ : op == PJ_ICE_STRANS_OP_NEGOTIATION ? "negotiation" : "unknown_op";
const bool done = status == PJ_SUCCESS;
if (done) {
@@ -793,8 +793,9 @@ IceTransport::Impl::requestUpnpMappings()
// Request the mapping
Mapping::sharedPtr_t mapPtr = upnp_->reserveMapping(requestedMap);
- // To use a mapping, it must be valid and open.
- if (mapPtr and mapPtr->getMapKey() and (mapPtr->getState() == MappingState::OPEN)) {
+ // To use a mapping, it must be valid, open and has valid host address.
+ if (mapPtr and mapPtr->getMapKey() and (mapPtr->getState() == MappingState::OPEN)
+ and mapPtr->hasValidHostAddress()) {
std::lock_guard<std::mutex> lock(upnpMappingsMutex_);
auto ret = upnpMappings_.emplace(mapPtr->getMapKey(), *mapPtr);
if (ret.second) {
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index a111671c3f1ccc1a956ce59c065bd2c643f6e4d2..721838f0bc7a0b2c6cb6ee432fe54b0e742de105 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -3376,7 +3376,7 @@ JamiAccount::storeActiveIpAddress()
publicAddr.toString().c_str());
// Check if we need to create a new UPNP controller instance.
- if (upnpEnabled_ and (not upnpCtrl_ or not upnpCtrl_->hasValidIGD())) {
+ if (upnpEnabled_ and (not upnpCtrl_ or not upnpCtrl_->isReady())) {
enableUpnp(true);
}
}
diff --git a/src/upnp/protocol/mapping.cpp b/src/upnp/protocol/mapping.cpp
index 51fa3522a1a5b29475c0e64f38774e1154cadad9..0ede1470729c7df0354ff0e0d1a7307822fd51db 100644
--- a/src/upnp/protocol/mapping.cpp
+++ b/src/upnp/protocol/mapping.cpp
@@ -26,8 +26,9 @@ namespace jami {
namespace upnp {
Mapping::Mapping(uint16_t portExternal, uint16_t portInternal, PortType type, bool available)
- : externalAddr_()
- , internalAddr_()
+ : internalAddr_()
+ , internalPort_(portInternal)
+ , externalPort_(portExternal)
, type_(type)
, igd_()
, available_(available)
@@ -38,32 +39,15 @@ Mapping::Mapping(uint16_t portExternal, uint16_t portInternal, PortType type, bo
#if HAVE_LIBNATPMP
, renewalTime_(sys_clock::now())
#endif
-{
- externalAddr_.port_ = portExternal;
- internalAddr_.port_ = portInternal;
-}
-
-Mapping::Mapping(Mapping&& other) noexcept
- : externalAddr_(std::move(other.externalAddr_))
- , internalAddr_(std::move(other.internalAddr_))
- , type_(other.type_)
- , igd_(other.igd_)
- , available_(other.available_)
- , state_(other.state_)
- , notifyCb_(std::move(other.notifyCb_))
- , timeoutTimer_(std::move(other.timeoutTimer_))
- , autoUpdate_(other.autoUpdate_)
-#if HAVE_LIBNATPMP
- , renewalTime_(other.renewalTime_)
-#endif
{}
Mapping::Mapping(const Mapping& other)
{
std::lock_guard<std::mutex> lock(mutex_);
- externalAddr_ = other.externalAddr_;
internalAddr_ = other.internalAddr_;
+ internalPort_ = other.internalPort_;
+ externalPort_ = other.externalPort_;
type_ = other.type_;
igd_ = other.igd_;
available_ = other.available_;
@@ -82,8 +66,9 @@ Mapping::operator=(Mapping&& other) noexcept
std::lock_guard<std::mutex> lock(mutex_);
if (this != &other) {
- externalAddr_ = other.externalAddr_;
internalAddr_ = other.internalAddr_;
+ internalPort_ = other.internalPort_;
+ externalPort_ = other.externalPort_;
type_ = other.type_;
igd_ = other.igd_;
other.igd_.reset();
@@ -109,16 +94,16 @@ Mapping::operator==(const Mapping& other) const noexcept
{
std::lock_guard<std::mutex> lock(mutex_);
- if (externalAddr_.addr_ != other.externalAddr_.addr_)
- return false;
- if (externalAddr_.port_ != other.externalAddr_.port_)
+ if (internalAddr_ != other.internalAddr_)
return false;
- if (internalAddr_.addr_ != other.internalAddr_.addr_)
+ if (internalPort_ != other.internalPort_)
return false;
- if (internalAddr_.port_ != other.internalAddr_.port_)
+ if (externalPort_ != other.externalPort_)
return false;
if (type_ != other.type_)
return false;
+ if (igd_ != other.igd_)
+ return false;
return true;
}
@@ -128,6 +113,20 @@ Mapping::operator!=(const Mapping& other) const noexcept
return not(*this == other);
}
+void
+Mapping::updateFrom(const Mapping& other)
+{
+ if (internalPort_ != other.internalPort_ or type_ != other.type_) {
+ JAMI_ERR("The source and destination mappings must match (same type and port)");
+ assert(false);
+ }
+
+ internalAddr_ = std::move(other.internalAddr_);
+ externalPort_ = other.externalPort_;
+ igd_ = other.igd_;
+ state_ = other.state_;
+}
+
void
Mapping::setAvailable(bool val)
{
@@ -171,7 +170,7 @@ Mapping::toString(bool addState) const
std::lock_guard<std::mutex> lock(mutex_);
std::ostringstream descr;
descr << UPNP_MAPPING_DESCRIPTION_PREFIX << "-" << getTypeStr(type_);
- descr << ":" << std::to_string(externalAddr_.port_);
+ descr << ":" << std::to_string(internalPort_);
if (addState)
descr << " (state=" << getStateStr(state_) << ")";
@@ -182,16 +181,41 @@ Mapping::toString(bool addState) const
bool
Mapping::isValid() const
{
- std::lock_guard<std::mutex> lock(mutex_);
- if (externalAddr_.port_ == 0)
- return false;
- if (internalAddr_.port_ == 0)
- return false;
- if (state_ == MappingState::FAILED)
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (state_ == MappingState::FAILED)
+ return false;
+ if (internalPort_ == 0)
+ return false;
+ if (externalPort_ == 0)
+ return false;
+ if (not igd_ or not igd_->isValid())
+ return false;
+ }
+
+ if (not hasValidHostAddress())
return false;
+
return true;
}
+bool
+Mapping::hasValidHostAddress() const
+{
+ std::lock_guard<std::mutex> lock(mutex_);
+
+ IpAddr intAddr(internalAddr_);
+ return intAddr and not intAddr.isLoopback();
+}
+
+bool
+Mapping::hasPublicAddress() const
+{
+ std::lock_guard<std::mutex> lock(mutex_);
+
+ return igd_ and igd_->getPublicIp() and not igd_->getPublicIp().isPrivate();
+}
+
void
Mapping::setTimeoutTimer(std::shared_ptr<Task> timer)
{
@@ -218,7 +242,7 @@ Mapping::getMapKey() const
{
std::lock_guard<std::mutex> lock(mutex_);
- key_t mapKey = externalAddr_.port_;
+ key_t mapKey = internalPort_;
if (type_ == PortType::UDP)
mapKey |= 1 << (sizeof(uint16_t) * 8);
return mapKey;
@@ -230,74 +254,69 @@ Mapping::getTypeFromMapKey(key_t key)
return (key >> (sizeof(uint16_t) * 8)) ? PortType::UDP : PortType::TCP;
}
-void
-Mapping::setExternalAddress(const std::string& addr)
-{
- std::lock_guard<std::mutex> lock(mutex_);
- externalAddr_.addr_ = addr;
-}
-
std::string
Mapping::getExternalAddress() const
{
std::lock_guard<std::mutex> lock(mutex_);
- return externalAddr_.addr_;
+ if (igd_)
+ return igd_->getPublicIp().toString();
+ return {};
}
void
Mapping::setExternalPort(uint16_t port)
{
std::lock_guard<std::mutex> lock(mutex_);
- externalAddr_.port_ = port;
+ externalPort_ = port;
}
uint16_t
Mapping::getExternalPort() const
{
std::lock_guard<std::mutex> lock(mutex_);
- return externalAddr_.port_;
+ return externalPort_;
}
std::string
Mapping::getExternalPortStr() const
{
std::lock_guard<std::mutex> lock(mutex_);
- return std::to_string(externalAddr_.port_);
+ return std::to_string(externalPort_);
}
void
Mapping::setInternalAddress(const std::string& addr)
{
std::lock_guard<std::mutex> lock(mutex_);
- internalAddr_.addr_ = addr;
+ internalAddr_ = addr;
}
std::string
Mapping::getInternalAddress() const
{
std::lock_guard<std::mutex> lock(mutex_);
- return internalAddr_.addr_;
+ return internalAddr_;
}
void
Mapping::setInternalPort(uint16_t port)
{
std::lock_guard<std::mutex> lock(mutex_);
- internalAddr_.port_ = port;
+ internalPort_ = port;
}
uint16_t
Mapping::getInternalPort() const
{
std::lock_guard<std::mutex> lock(mutex_);
- return internalAddr_.port_;
+ return internalPort_;
}
std::string
Mapping::getInternalPortStr() const
{
std::lock_guard<std::mutex> lock(mutex_);
- return std::to_string(internalAddr_.port_);
+ return std::to_string(internalPort_);
}
PortType
diff --git a/src/upnp/protocol/mapping.h b/src/upnp/protocol/mapping.h
index 25bbfe3f49044409fe1f70bfa74e5cec53b8a22c..694784e5b2221c22a19192e0e66314c0c4861334 100644
--- a/src/upnp/protocol/mapping.h
+++ b/src/upnp/protocol/mapping.h
@@ -58,31 +58,6 @@ public:
using sharedPtr_t = std::shared_ptr<Mapping>;
using NotifyCallback = std::function<void(sharedPtr_t)>;
- struct MappingAddr
- {
- std::string addr_ {};
- uint16_t port_ {0};
- MappingAddr()
- : addr_()
- , port_(0)
- {}
-
- MappingAddr(const MappingAddr& other)
- {
- addr_ = other.addr_;
- port_ = other.port_;
- }
-
- MappingAddr& operator=(const MappingAddr& other)
- {
- if (this != &other) {
- addr_ = other.addr_;
- port_ = other.port_;
- }
- return *this;
- }
- };
-
static constexpr char const* MAPPING_STATE_STR[4] {"PENDING", "IN_PROGRESS", "FAILED", "OPEN"};
static constexpr char const* UPNP_MAPPING_DESCRIPTION_PREFIX {"JAMI"};
@@ -90,8 +65,8 @@ public:
uint16_t portInternal = 0,
PortType type = PortType::UDP,
bool available = true);
- Mapping(Mapping&& other) noexcept;
Mapping(const Mapping& other);
+ Mapping(Mapping&& other) = delete;
~Mapping() = default;
Mapping& operator=(Mapping&& other) noexcept;
@@ -106,6 +81,7 @@ public:
inline explicit operator bool() const { return isValid(); }
+ void updateFrom(const Mapping& other);
std::string getExternalAddress() const;
uint16_t getExternalPort() const;
std::string getExternalPortStr() const;
@@ -127,6 +103,8 @@ public:
}
std::string toString(bool addState = false) const;
bool isValid() const;
+ bool hasValidHostAddress() const;
+ bool hasPublicAddress() const;
void setNotifyCallback(NotifyCallback cb);
void enableAutoUpdate(bool enable);
bool getAutoUpdate() const;
@@ -138,10 +116,8 @@ public:
private:
NotifyCallback getNotifyCallback() const;
- // TODO_MC. Remove SetXXadress. already have IGD. Set ports instead.
- void setExternalAddress(const std::string& addr);
- void setExternalPort(uint16_t port);
void setInternalAddress(const std::string& addr);
+ void setExternalPort(uint16_t port);
void setInternalPort(uint16_t port);
void setIgd(const std::shared_ptr<IGD>& igd);
@@ -155,8 +131,9 @@ private:
#endif
mutable std::mutex mutex_;
- MappingAddr externalAddr_;
- MappingAddr internalAddr_;
+ std::string internalAddr_;
+ uint16_t internalPort_ {0};
+ uint16_t externalPort_ {0};
PortType type_;
// Protocol and
std::shared_ptr<IGD> igd_;
diff --git a/src/upnp/protocol/natpmp/nat_pmp.cpp b/src/upnp/protocol/natpmp/nat_pmp.cpp
index 37ea9fa4ab8f303089ea03a206c54bddd6285afa..6a58e7af12ff69ee6452875f0bdf4372dfac09a2 100644
--- a/src/upnp/protocol/natpmp/nat_pmp.cpp
+++ b/src/upnp/protocol/natpmp/nat_pmp.cpp
@@ -20,6 +20,8 @@
#include "nat_pmp.h"
+#if HAVE_LIBNATPMP
+
namespace jami {
namespace upnp {
@@ -46,10 +48,16 @@ NatPmp::initNatPmp()
CHECK_VALID_THREAD();
- userLocalIp_ = ip_utils::getLocalAddr(pj_AF_INET());
initialized_ = false;
- assert(igd_);
+ hostAddress_ = ip_utils::getLocalAddr(pj_AF_INET());
+
+ // Local address must be valid.
+ if (not getHostAddress() or getHostAddress().isLoopback()) {
+ JAMI_WARN("NAT-PMP: Does not have a valid local address!");
+ return;
+ }
+ assert(igd_);
if (igd_->isValid()) {
igd_->setValid(false);
processIgdUpdate(UpnpIgdEvent::REMOVED);
@@ -91,6 +99,9 @@ NatPmp::initNatPmp()
// Set the local (gateway) address.
igd_->setLocalIp(igdAddr);
+ // NAT-PMP protocol does not have UID, but we will set generic
+ // one debugging purposes.
+ igd_->setUID("NAT-PMP Gateway");
// Search and set the public address.
getIgdPublicAddress();
@@ -143,14 +154,6 @@ NatPmp::searchForIgd()
initNatPmp();
}
- if (initialized_ and not hasValidIgd()) {
- // Log current IGD.
- JAMI_DBG("NAT-PMP: Current top IGD [%s:%s] valid %s",
- igd_->getLocalIp().toString().c_str(),
- igd_->getLocalIp().toString().c_str(),
- igd_->isValid() ? "YES" : "NO");
- }
-
// Schedule a retry in case init failed.
if (not initialized_) {
if (igdSearchCounter_++ < MAX_RESTART_SEARCH_RETRIES) {
@@ -175,8 +178,11 @@ NatPmp::getIgdList(std::list<std::shared_ptr<IGD>>& igdList) const
}
bool
-NatPmp::hasValidIgd() const
+NatPmp::isReady() const
{
+ // Must at least have a valid local address.
+ if (not getHostAddress() or getHostAddress().isLoopback())
+ return false;
return igd_ and igd_->isValid();
}
@@ -332,8 +338,7 @@ NatPmp::addPortMapping(const std::shared_ptr<IGD>& igdIn, Mapping& mapping, bool
Mapping mapToAdd(mapping.getExternalPort(),
mapping.getInternalPort(),
mapping.getType() == PortType::UDP ? upnp::PortType::UDP : upnp::PortType::TCP);
- mapToAdd.setExternalAddress(igd_->getPublicIp().toString());
- mapToAdd.setInternalAddress(getUserLocalIp().toString());
+ mapToAdd.setInternalAddress(getHostAddress().toString());
mapToAdd.setIgd(igd_);
uint32_t lifetime = MAPPING_ALLOCATION_LIFETIME;
@@ -639,3 +644,5 @@ NatPmp::processMappingRemoved(const Mapping& map)
} // namespace upnp
} // namespace jami
+
+#endif //-- #if HAVE_LIBNATPMP
diff --git a/src/upnp/protocol/natpmp/nat_pmp.h b/src/upnp/protocol/natpmp/nat_pmp.h
index 2df50e4a6991dadf09fee7d74d9881a0373e4ec4..ddf0a7ea8bfa40916b321a826cc7ca565625d150 100644
--- a/src/upnp/protocol/natpmp/nat_pmp.h
+++ b/src/upnp/protocol/natpmp/nat_pmp.h
@@ -83,7 +83,7 @@ public:
void getIgdList(std::list<std::shared_ptr<IGD>>& igdList) const override;
// Return true if it has at least one valid IGD.
- bool hasValidIgd() const override;
+ bool isReady() const override;
// Increment errors counter.
void incrementErrorsCounter(const std::shared_ptr<IGD>& igd) override;
diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp
index 2bedf3ad26ead03b47c437cd5a3be372fc32045b..cf6334101cca85ec35c5515381d4c3d3e044fa0c 100644
--- a/src/upnp/protocol/pupnp/pupnp.cpp
+++ b/src/upnp/protocol/pupnp/pupnp.cpp
@@ -94,11 +94,12 @@ errorOnResponse(IXML_Document* doc)
return false;
}
-
// UPNP class implementation
PUPnP::PUPnP()
{
+ threadId_ = getCurrentThread();
+
if (not initUpnpLib())
return;
if (not registerClient())
@@ -249,10 +250,12 @@ PUPnP::setObserver(UpnpMappingObserver* obs)
void
PUPnP::searchForDevices()
{
- userLocalIp_ = ip_utils::getLocalAddr(pj_AF_INET());
+ std::unique_lock<std::mutex> lk(ctrlptMutex_);
+
+ JAMI_DBG("PUPnP: Send IGD search request");
+
// Send out search for multiple types of devices, as some routers may possibly
// only reply to one.
- std::unique_lock<std::mutex> lk(ctrlptMutex_);
UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_ROOT_DEVICE, this);
UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_IGD_DEVICE, this);
UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_WANIP_SERVICE, this);
@@ -264,6 +267,15 @@ PUPnP::clearIgds()
{
JAMI_DBG("PUPnP: clearing IGDs and devices lists");
+ CHECK_VALID_THREAD();
+
+ hostAddress_ = {};
+
+ if (searchForIgdTimer_)
+ searchForIgdTimer_->cancel();
+
+ igdSearchCounter_ = 0;
+
{
std::lock_guard<std::mutex> lock(validIgdListMutex_);
validIgdList_.clear();
@@ -281,9 +293,30 @@ PUPnP::clearIgds()
void
PUPnP::searchForIgd()
{
- // Notify thread to execute in non-blocking fashion.
- searchForIgd_ = true;
- pupnpCv_.notify_one();
+ CHECK_VALID_THREAD();
+
+ // Update local address before searching.
+ hostAddress_ = ip_utils::getLocalAddr(pj_AF_INET());
+
+ if (not isReady()) {
+ if (igdSearchCounter_++ < PUPNP_MAX_RESTART_SEARCH_RETRIES) {
+ JAMI_DBG("PUPnP: Start search for IGD: attempt %u", igdSearchCounter_);
+
+ searchForIgd_ = true;
+ pupnpCv_.notify_one();
+
+ // Cancel the current timer (if any) and re-schedule.
+ if (searchForIgdTimer_)
+ searchForIgdTimer_->cancel();
+
+ searchForIgdTimer_ = getUpnContextScheduler()
+ ->scheduleIn([this] { searchForIgd(); },
+ PUPNP_TIMEOUT_BEFORE_IGD_SEARCH_RETRY);
+ } else {
+ JAMI_WARN("PUPnP: Setup failed after %u trials. PUPnP will be disabled!",
+ PUPNP_MAX_RESTART_SEARCH_RETRIES);
+ }
+ }
}
void
@@ -298,6 +331,16 @@ PUPnP::getIgdList(std::list<std::shared_ptr<IGD>>& igdList) const
}
}
+bool
+PUPnP::isReady() const
+{
+ // Must at least have a valid local address.
+ if (not getHostAddress() or getHostAddress().isLoopback())
+ return false;
+
+ return hasValidIgd();
+}
+
bool
PUPnP::hasValidIgd() const
{
@@ -309,6 +352,17 @@ PUPnP::hasValidIgd() const
return false;
}
+bool
+PUPnP::updateAndCheckHostAddress()
+{
+ if (hostAddress_ and not hostAddress_.isLoopback())
+ return true;
+
+ hostAddress_ = ip_utils::getLocalAddr(pj_AF_INET());
+
+ return hostAddress_ and not hostAddress_.isLoopback();
+}
+
void
PUPnP::incrementErrorsCounter(const std::shared_ptr<IGD>& igd)
{
@@ -344,8 +398,7 @@ PUPnP::validateIgd(const IGDInfo& info)
return false;
}
- JAMI_DBG("PUPnP: Validating IGD candidate\n"
- " UDN : %s\n"
+ JAMI_DBG("PUPnP: Validating the IGD candidate [UDN: %s]\n"
" Name : %s\n"
" Service Type : %s\n"
" Service ID : %s\n"
@@ -404,7 +457,8 @@ PUPnP::validateIgd(const IGDInfo& info)
// 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",
+ 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->getLocalIp().toString().c_str(),
igd_candidate->getPublicIp().toString().c_str());
@@ -413,8 +467,10 @@ PUPnP::validateIgd(const IGDInfo& info)
}
}
- JAMI_DBG("PUPnP: Found new valid IGD [%s] with int/ext addresses [%s:%s]",
- 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->getLocalIp().toString().c_str(),
igd_candidate->getPublicIp().toString().c_str());
@@ -448,7 +504,6 @@ void
PUPnP::requestMappingAdd(const std::shared_ptr<IGD>& igd, const Mapping& mapping)
{
if (auto pupnp_igd = std::dynamic_pointer_cast<const UPnPIGD>(igd)) {
- JAMI_DBG("PUPnP: Attempting to open port %s", mapping.toString().c_str());
actionAddPortMappingAsync(*pupnp_igd, mapping);
}
}
@@ -486,7 +541,7 @@ PUPnP::actionAddPortMappingAsync(const UPnPIGD& igd, const Mapping& mapping)
ACTION_ADD_PORT_MAPPING,
igd.getServiceType().c_str(),
"NewInternalClient",
- getUserLocalIp().toString().c_str());
+ getHostAddress().toString().c_str());
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
igd.getServiceType().c_str(),
@@ -537,8 +592,7 @@ PUPnP::processAddMapAction(const std::string& ctrlURL,
for (auto const& it : validIgdList_) {
if (auto igd = std::dynamic_pointer_cast<UPnPIGD>(it)) {
if (igd->getControlURL() == ctrlURL) {
- mapToAdd.setExternalAddress(igd->getPublicIp().toString());
- mapToAdd.setInternalAddress(getUserLocalIp().toString());
+ mapToAdd.setInternalAddress(getHostAddress().toString());
mapToAdd.setIgd(igd);
break;
}
@@ -590,9 +644,6 @@ PUPnP::requestMappingRemove(const Mapping& igdMapping)
// Iterate over all IGDs in internal list and try to remove selected mapping.
for (auto const& it : validIgdList_) {
if (auto igd = std::dynamic_pointer_cast<UPnPIGD>(it)) {
- JAMI_DBG("PUPnP: Attempting to close port %s %s",
- igdMapping.getExternalPortStr().c_str(),
- igdMapping.getTypeStr());
std::lock_guard<std::mutex> lk2(ctrlptMutex_);
actionDeletePortMappingAsync(*igd,
igdMapping.getExternalPortStr(),
@@ -676,16 +727,25 @@ PUPnP::processDiscoverySearchResult(const std::string& cpDeviceId,
const std::string& igdLocationUrl,
const IpAddr& dstAddr)
{
+ // Must first have a valid local address.
+ if (not updateAndCheckHostAddress()) {
+ JAMI_WARN("PUPnP: Local address is invalid. Ignore search result for now!");
+ return;
+ }
+
dht::http::Url url(igdLocationUrl);
std::lock_guard<std::mutex> lk(igdListMutex_);
- if (not discoveredIgdList_.emplace(cpDeviceId).second)
+ // Use the device ID and the URL as ID. This is necessary as some
+ // IGDs may have the same device ID but different URLs.
+
+ auto igdId = cpDeviceId + " url: " + igdLocationUrl;
+
+ if (not discoveredIgdList_.emplace(igdId).second)
return;
- JAMI_DBG("PUPnP: Added IGD %s [%s] to known IGD list",
- cpDeviceId.c_str(),
- IpAddr(url.host).toString(true, true).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
@@ -698,8 +758,6 @@ PUPnP::processDiscoverySearchResult(const std::string& cpDeviceId,
IpAddr(url.host).toString(true, true).c_str(),
dstAddr.toString(true, true).c_str());
return;
- } else {
- JAMI_DBG("PUPnP: Returned IGD location %s", IpAddr(url.host).toString(true, true).c_str());
}
dwnldlXmlList_.emplace_back(
@@ -834,11 +892,14 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
break;
}
case UPNP_DISCOVERY_SEARCH_TIMEOUT: {
- // Nothing to do here.
+ // Even if the discovery search is successful, it's normal to receive
+ // time-out events. This because we send search requests using various
+ // device types, which some of them may not return a response.
+ JAMI_DBG("PUPnP: Received discovery search time-out");
break;
}
case UPNP_EVENT_RECEIVED: {
- // TODO: Handle event by updating any changed state variables */
+ // Nothing to do.
break;
}
case UPNP_EVENT_AUTORENEWAL_FAILED: // Fall through. Treat failed autorenewal like an expired
@@ -933,18 +994,24 @@ PUPnP::subEventCallback(Upnp_EventType event_type, const void* event, void* user
}
int
-PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType /*event_type */, const void* event)
+PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType /*unused*/, const void* event)
{
+ if (event == nullptr) {
+ JAMI_ERR("PUPnP: Unexpected null pointer!");
+ return UPNP_E_INVALID_ARGUMENT;
+ }
+
const UpnpEventSubscribe* es_event = (const UpnpEventSubscribe*) event;
+ 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: Error when trying to handle subscription callback -> %s",
- UpnpGetErrorMessage(upnp_err));
+ JAMI_WARN("PUPnP: Subscription error %s from %s",
+ UpnpGetErrorMessage(upnp_err),
+ publisherUrl.c_str());
return upnp_err;
}
- // TODO: Handle subscription event.
-
return UPNP_E_SUCCESS;
}
@@ -1238,7 +1305,7 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string
std::string desc_actual = getFirstDocItem(response.get(), "NewPortMappingDescription");
std::string client_ip = getFirstDocItem(response.get(), "NewInternalClient");
- if (client_ip != getUserLocalIp().toString()) {
+ if (client_ip != getHostAddress().toString()) {
// Silently ignore un-matching addresses.
continue;
}
@@ -1284,7 +1351,7 @@ PUPnP::deleteMappingsByDescription(const std::shared_ptr<IGD>& igd, const std::s
igd->getLocalIp().toString().c_str(),
Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX);
- auto mapList = getMappingsListByDescr(igd, Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX);
+ auto mapList = getMappingsListByDescr(igd, description);
for (auto const& [_, map] : mapList) {
requestMappingRemove(map);
@@ -1305,7 +1372,6 @@ PUPnP::actionDeletePortMapping(const UPnPIGD& igd,
std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&>
response(nullptr, ixmlDocument_free); // Response pointer.
IXML_Document* action_container_ptr = nullptr;
- IXML_Document* response_container_ptr = nullptr;
// Set action name.
std::string action_name {"DeletePortMapping"};
@@ -1345,7 +1411,7 @@ PUPnP::actionDeletePortMapping(const UPnPIGD& igd,
return false;
}
- JAMI_DBG("PUPnP: Successfully sent %s request from %s",
+ JAMI_DBG("PUPnP: Successfully sent %s request to %s",
action_name.c_str(),
igd.getServiceType().c_str());
@@ -1396,7 +1462,7 @@ PUPnP::actionAddPortMapping(const UPnPIGD& igd,
action_name.c_str(),
igd.getServiceType().c_str(),
"NewInternalClient",
- getUserLocalIp().toString().c_str());
+ getHostAddress().toString().c_str());
UpnpAddToAction(&action_container_ptr,
action_name.c_str(),
igd.getServiceType().c_str(),
diff --git a/src/upnp/protocol/pupnp/pupnp.h b/src/upnp/protocol/pupnp/pupnp.h
index b431b01c03a3c89468682ee81582d01673f736d1..3290bab6ae65e737c4fa1bdff2052bdce4a3d750 100644
--- a/src/upnp/protocol/pupnp/pupnp.h
+++ b/src/upnp/protocol/pupnp/pupnp.h
@@ -70,6 +70,10 @@ constexpr static int CONFLICT_IN_MAPPING = 718;
// Timeout values (in seconds).
constexpr static unsigned int SEARCH_TIMEOUT {60};
constexpr static unsigned int SUBSCRIBE_TIMEOUT {300};
+// Max number of IGD search attempts before failure.
+constexpr static unsigned int PUPNP_MAX_RESTART_SEARCH_RETRIES {5};
+// Time-out between two successive IGD search.
+constexpr static auto PUPNP_TIMEOUT_BEFORE_IGD_SEARCH_RETRY {std::chrono::seconds(60)};
class PUPnP : public UPnPProtocol
{
@@ -118,8 +122,8 @@ public:
// Get the IGD list.
void getIgdList(std::list<std::shared_ptr<IGD>>& igdList) const override;
- // Return true if it has at least one valid IGD.
- bool hasValidIgd() const override;
+ // Return true if the it's fully setup.
+ bool isReady() const override;
// Increment IGD errors counter.
void incrementErrorsCounter(const std::shared_ptr<IGD>& igd) override;
@@ -139,6 +143,14 @@ public:
void requestMappingRemove(const Mapping& igdMapping) override;
private:
+ ScheduledExecutor* getUpnContextScheduler() { return UpnpThreadUtil::getScheduler(); }
+
+ // Return true if it has at least one valid IGD.
+ bool hasValidIgd() const;
+
+ // Update and check the host (local) address. Returns true
+ // if the address is valid.
+ bool updateAndCheckHostAddress();
// Delete mappings matching the description
void deleteMappingsByDescription(const std::shared_ptr<IGD>& igd,
@@ -190,7 +202,7 @@ private:
#endif
// Callback subscription event function for handling subscription request.
- int handleSubscriptionUPnPEvent(Upnp_EventType event_type, const void* event);
+ int handleSubscriptionUPnPEvent(Upnp_EventType /*unused*/, const void* event);
// Parses the IGD candidate.
std::unique_ptr<UPnPIGD> parseIgd(IXML_Document* doc, std::string locationUrl);
@@ -225,6 +237,8 @@ private:
std::condition_variable pupnpCv_ {}; // Condition variable for thread-safe signaling.
std::atomic_bool pupnpRun_ {true}; // Variable to allow the thread to run.
std::thread pupnpThread_ {}; // PUPnP thread for non-blocking client registration.
+ std::shared_ptr<Task> searchForIgdTimer_ {};
+ unsigned int igdSearchCounter_ {0};
mutable std::mutex validIgdListMutex_;
std::list<std::shared_ptr<IGD>> validIgdList_; // List of valid IGDs.
diff --git a/src/upnp/protocol/upnp_protocol.h b/src/upnp/protocol/upnp_protocol.h
index 0b3e6bd181ec59107d0fc85abfbdb7a2c7af2f72..eed6cf00f6ed936e18160efa5291896acc44f90c 100644
--- a/src/upnp/protocol/upnp_protocol.h
+++ b/src/upnp/protocol/upnp_protocol.h
@@ -97,7 +97,7 @@ public:
virtual void getIgdList(std::list<std::shared_ptr<IGD>>& igdList) const = 0;
// Return true if it has at least one valid IGD.
- virtual bool hasValidIgd() const = 0;
+ virtual bool isReady() const = 0;
// Increment IGD errors counter.
virtual void incrementErrorsCounter(const std::shared_ptr<IGD>& igd) = 0;
@@ -121,10 +121,12 @@ public:
// Set the user callbacks.
virtual void setObserver(UpnpMappingObserver* obs) = 0;
- const IpAddr& getUserLocalIp() const { return userLocalIp_; }
+ // Get the host (local) address.
+ const IpAddr& getHostAddress() const { return hostAddress_; }
protected:
- IpAddr userLocalIp_ {};
+ // The host (local) address. Must be fully set before making any request.
+ IpAddr hostAddress_ {};
UpnpMappingObserver* observer_;
};
diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp
index 09ade02c30377b293c78457d613b5e207e4811c8..772ddc6bfb484ed98104d7711777a9ae7d3ebf58 100644
--- a/src/upnp/upnp_context.cpp
+++ b/src/upnp/upnp_context.cpp
@@ -181,7 +181,7 @@ UPnPContext::setPublicAddress(const IpAddr& addr)
}
bool
-UPnPContext::hasValidIGD() const
+UPnPContext::isReady() const
{
std::lock_guard<std::mutex> lock(mappingMutex_);
return not validIgdList_.empty();
@@ -365,9 +365,11 @@ UPnPContext::requestMapping(const std::shared_ptr<IGD>& igd, const Mapping::shar
CHECK_VALID_THREAD();
- JAMI_DBG("Request mapping %s using protocol [%s]",
+ JAMI_DBG("Request mapping %s using protocol [%s] IGD [%s %s]",
map->toString().c_str(),
- igd->getProtocolName());
+ igd->getProtocolName(),
+ igd->getUID().c_str(),
+ igd->getLocalIp().toString().c_str());
// Register mapping timeout callback and update the state if needed.
registerAddMappingTimeout(igd, map);
@@ -389,7 +391,7 @@ UPnPContext::requestMappingOnValidIgds(const Mapping::sharedPtr_t& map)
CHECK_VALID_THREAD();
- if (not hasValidIGD()) {
+ if (not isReady()) {
JAMI_WARN("Can not request mapping, no valid IGD available!");
return;
}
@@ -399,12 +401,12 @@ UPnPContext::requestMappingOnValidIgds(const Mapping::sharedPtr_t& map)
#if HAVE_LIBNATPMP
// Prefere NAT-PMP if available.
- if (protocolList_.at(NatProtocolType::NAT_PMP)->hasValidIgd())
+ if (protocolList_.at(NatProtocolType::NAT_PMP)->isReady())
proto = protocolList_.at(NatProtocolType::NAT_PMP);
#endif
// Request the mappings
- if (proto->hasValidIgd()) {
+ if (proto->isReady()) {
std::list<std::shared_ptr<IGD>> igdList;
proto->getIgdList(igdList);
for (auto const& igd : igdList)
@@ -496,19 +498,21 @@ UPnPContext::updateMappingList(bool async)
mappingListUpdateTimer_->cancel();
mappingListUpdateTimer_ = getScheduler()->scheduleIn([this] { updateMappingList(false); },
MAP_UPDATE_INTERVAL);
-
- bool hasValidIgd = false;
+ std::shared_ptr<IGD> prefIgd;
for (auto const& [_, protocol] : protocolList_) {
- auto valid = protocol->hasValidIgd();
- if (valid) {
- JAMI_DBG("Protocol status [%s] has valid IGD(s) %s",
- protocol->getProtocolName(),
- valid ? "YES" : "NO");
- hasValidIgd = true;
+ if (protocol->isReady()) {
+ std::list<std::shared_ptr<IGD>> igdList;
+ protocol->getIgdList(igdList);
+ assert(not igdList.empty());
+ auto const& igd = igdList.front();
+
+ // Perefer NAT-PMP if available.
+ if (not prefIgd or protocol->getProtocol() == NatProtocolType::NAT_PMP)
+ prefIgd = igd;
}
}
- if (not hasValidIgd) {
+ if (not prefIgd) {
JAMI_DBG("UPNP/NAT-PMP enabled, but no valid IGDs available");
std::lock_guard<std::mutex> lock(mappingMutex_);
// Invalidate the current IGDs.
@@ -517,6 +521,14 @@ UPnPContext::updateMappingList(bool async)
return;
}
+ JAMI_DBG("Current preferred protocol [%s] IGD [%s %s] ",
+ prefIgd->getProtocolName(),
+ prefIgd->getUID().c_str(),
+ prefIgd->getLocalIp().toString().c_str());
+
+ // Process pending requests if any.
+ processPendingRequests(prefIgd);
+
// Make new requests for mappings that failed and have
// the auto-update option enabled.
processMappingWithAutoUpdate();
@@ -567,10 +579,10 @@ UPnPContext::updateMappingList(bool async)
}
// Prune the mapping list if needed
- if (protocolList_.at(NatProtocolType::PUPNP)->hasValidIgd()) {
+ if (protocolList_.at(NatProtocolType::PUPNP)->isReady()) {
#if HAVE_LIBNATPMP
// Dont perform if NAT-PMP is valid.
- if (not protocolList_.at(NatProtocolType::NAT_PMP)->hasValidIgd())
+ if (not protocolList_.at(NatProtocolType::NAT_PMP)->isReady())
#endif
{
pruneMappingList();
@@ -579,7 +591,7 @@ UPnPContext::updateMappingList(bool async)
#if HAVE_LIBNATPMP
// Renew nat-pmp allocations
- if (protocolList_.at(NatProtocolType::NAT_PMP)->hasValidIgd())
+ if (protocolList_.at(NatProtocolType::NAT_PMP)->isReady())
renewAllocations();
#endif
}
@@ -809,12 +821,12 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event)
// Check if IGD has a valid addresses.
if (not igdLocalAddr) {
- JAMI_ERR("[%s] IGD has an invalid local address", protocolName);
+ JAMI_WARN("[%s] IGD has an invalid local address", protocolName);
return;
}
if (not igd->getPublicIp()) {
- JAMI_ERR("[%s] IGD has an invalid public address", protocolName);
+ JAMI_WARN("[%s] IGD has an invalid public address", protocolName);
return;
}
@@ -826,11 +838,12 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event)
knownPublicAddress_.toString().c_str());
}
- char const* IgdState = event == UpnpIgdEvent::ADDED ? "ADDED"
- : event == UpnpIgdEvent::REMOVED ? "REMOVED"
- : "INVALID";
+ char const* IgdState = event == UpnpIgdEvent::ADDED
+ ? "ADDED"
+ : event == UpnpIgdEvent::REMOVED ? "REMOVED" : "INVALID";
- JAMI_WARN("State of IGD %s [%s] changed to [%s]",
+ JAMI_WARN("State of IGD %s %s [%s] changed to [%s]",
+ igd->getUID().c_str(),
igdLocalAddr.toString(true, true).c_str(),
protocolName,
IgdState);
@@ -861,19 +874,6 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event)
}
}
- // Prune the IGD before making new requests.
- if (igd->getProtocol() == NatProtocolType::PUPNP) {
- auto protocol = protocolList_.at(igd->getProtocol());
- auto remoteMapList
- = protocol->getMappingsListByDescr(igd, Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX);
- if (not remoteMapList.empty()) {
- pruneUnMatchedMappings(igd, remoteMapList);
- }
- }
-
- // Process pending requests if any.
- processPendingRequests(igd);
-
// Update.
updateMappingList(false);
}
@@ -883,79 +883,56 @@ UPnPContext::onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& mapR
{
CHECK_VALID_THREAD();
- if (not mapRes.isValid() or mapRes.getState() == MappingState::FAILED) {
- JAMI_WARN("Mapping request %s failed on IGD %s [%s]",
- mapRes.toString().c_str(),
- igd->getLocalIp().toString().c_str(),
- mapRes.getProtocolName());
+ // Check if we have a pending request for this response.
+ 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->getLocalIp().toString().c_str(),
+ mapRes.getProtocolName());
return;
}
- Mapping::key_t key = 0;
-
- {
- std::lock_guard<std::mutex> lock(mappingMutex_);
- auto& mappingList = getMappingList(mapRes.getType());
- auto it = mappingList.find(mapRes.getMapKey());
- if (it == mappingList.end()) {
- // We may receive a response for a canceled request. Just
- // ignore it.
- JAMI_DBG("Mapping request %s from IGD %s [%s] does not match local request",
- mapRes.toString().c_str(),
- igd->getLocalIp().toString().c_str(),
- mapRes.getProtocolName());
- return;
- }
- key = it->second->getMapKey();
- assert(key);
- }
-
- auto map = getMappingWithKey(key);
// The mapping pointer must be valid at his point.
assert(map);
- // Update the state.
- if (map->isValid()) {
- // If both protocols grant the mapping, we prefere using NAT-PMP.
- if (map->getState() == MappingState::OPEN
- and map->getProtocol() == NatProtocolType::NAT_PMP) {
- JAMI_DBG("Mapping %s is already open on IGD %s [NAT-PMP]",
- map->toString().c_str(),
- igd->getLocalIp().toString().c_str());
- } else {
- // Set the attributes
- map->setExternalAddress(mapRes.getExternalAddress());
- map->setInternalAddress(mapRes.getInternalAddress());
- map->setIgd(igd);
-#if HAVE_LIBNATPMP
- if (mapRes.getProtocol() == NatProtocolType::NAT_PMP) {
- map->setRenewalTime(mapRes.getRenewalTime());
- }
-#endif
- updateMappingState(map, MappingState::OPEN);
+ // We have a response, so cancel the timer.
+ map->cancelTimeoutTimer();
- JAMI_DBG("Mapping %s (on IGD %s [%s]) successfully performed",
- map->toString().c_str(),
- igd->getLocalIp().toString().c_str(),
- map->getProtocolName());
+ // Update the mapping.
+ map->updateFrom(mapRes);
- if (not igd->isValid()) {
- std::lock_guard<std::mutex> lock(mappingMutex_);
- validIgdList_.emplace(igd);
- }
- // Call setValid() to reset the errors counter.
- igd->setValid(true);
- }
- } else {
- JAMI_ERR("Failed to perform mapping %s (on IGD %s)",
- map->toString().c_str(),
- igd->getLocalIp().toString().c_str());
+ // Clean-up if not valid.
+ if (not map->isValid()) {
+ JAMI_WARN("Mapping request %s on IGD %s failed or returned an invalid mapping!",
+ map->toString(true).c_str(),
+ igd->getLocalIp().toString().c_str());
updateMappingState(map, MappingState::FAILED);
unregisterMapping(map);
+ return;
}
- // We have a response, so cancel the time-out.
- map->cancelTimeoutTimer();
+ // Ignore if already open.
+ if (map->getState() == MappingState::OPEN) {
+ JAMI_DBG("Mapping %s is already open on IGD %s [%s]",
+ map->toString().c_str(),
+ map->getIgd()->getLocalIp().toString().c_str(),
+ map->getIgd()->getProtocolName());
+ return;
+ }
+
+ // The mapping request is new and successful. Update.
+ updateMappingState(map, MappingState::OPEN);
+
+ JAMI_DBG("Mapping %s (on IGD %s [%s]) successfully performed",
+ map->toString().c_str(),
+ igd->getLocalIp().toString().c_str(),
+ map->getProtocolName());
+
+ // Call setValid() to reset the errors counter. We need
+ // to reset the counter on each successful response.
+ igd->setValid(true);
}
#if HAVE_LIBNATPMP
@@ -1072,7 +1049,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 hasValidIGD()) {
+ if (not isReady()) {
JAMI_WARN("No IGD available. Mapping will be requested when an IGD becomes available");
} else {
requestMappingOnValidIgds(mapPtr);
@@ -1191,9 +1168,12 @@ UPnPContext::registerAddMappingTimeout(const std::shared_ptr<IGD>& igd,
auto timeout = igd->getProtocol() == NatProtocolType::NAT_PMP ? NAT_MAP_REQUEST_TIMEOUT_UNIT
: PUPNP_MAP_REQUEST_TIMEOUT_UNIT;
map->setTimeoutTimer(
- // The time-out is set proportional to the number of "in-progress" requests.
+ // The time-out is set proportional to the number of "in-progress"
+ // requests plus a bias.
+ // This rule is empirical and based on experimenting with few
+ // implementations.
Manager::instance().scheduler().scheduleIn([this, igd, map] { onRequestTimeOut(igd, map); },
- timeout * (status.inProgressCount_ + 1)));
+ timeout * (status.inProgressCount_ + 3)));
}
void
@@ -1237,7 +1217,7 @@ UPnPContext::updateMappingState(const Mapping::sharedPtr_t& map, MappingState ne
// Ignore if the state did not change.
if (newState == map->getState()) {
- JAMI_WARN("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_context.h b/src/upnp/upnp_context.h
index 566e87b61da7cf85844b28d52722a6b120d8e037..080dc83cd3045d4c7a322adcd9f922789d756ead 100644
--- a/src/upnp/upnp_context.h
+++ b/src/upnp/upnp_context.h
@@ -109,7 +109,7 @@ public:
void setPublicAddress(const IpAddr& addr);
// Check if there is a valid IGD in the IGD list.
- bool hasValidIGD() const;
+ bool isReady() const;
// Get external Ip of a chosen IGD.
IpAddr getExternalIP() const;
diff --git a/src/upnp/upnp_control.cpp b/src/upnp/upnp_control.cpp
index 72048aa23dffec5803dcf9d36426acef58f6c51e..652efe4486983d802fb1fb506c303847c1c362f8 100644
--- a/src/upnp/upnp_control.cpp
+++ b/src/upnp/upnp_control.cpp
@@ -57,17 +57,17 @@ Controller::~Controller()
}
bool
-Controller::hasValidIGD() const
+Controller::isReady() const
{
assert(upnpContext_);
- return upnpContext_->hasValidIGD();
+ return upnpContext_->isReady();
}
IpAddr
Controller::getExternalIP() const
{
assert(upnpContext_);
- if (upnpContext_->hasValidIGD()) {
+ if (upnpContext_->isReady()) {
return upnpContext_->getExternalIP();
}
return {};
diff --git a/src/upnp/upnp_control.h b/src/upnp/upnp_control.h
index 87714f12e8cbf353642d71994dee30811fda191c..67218d37a25cea92f2c2bdb38b28142d735d4da9 100644
--- a/src/upnp/upnp_control.h
+++ b/src/upnp/upnp_control.h
@@ -51,7 +51,7 @@ public:
~Controller();
// Checks if a valid IGD is available.
- bool hasValidIGD() const;
+ bool isReady() const;
// Gets the external ip of the first valid IGD in the list.
IpAddr getExternalIP() const;