diff --git a/src/upnp/protocol/natpmp/nat_pmp.cpp b/src/upnp/protocol/natpmp/nat_pmp.cpp
index 30a339195b2c36302b5c30900596839a339153d8..42aad9a9b6bf56566522df7b1ab59c359c3f968e 100644
--- a/src/upnp/protocol/natpmp/nat_pmp.cpp
+++ b/src/upnp/protocol/natpmp/nat_pmp.cpp
@@ -97,7 +97,7 @@ NatPmp::~NatPmp()
}
void
-NatPmp::clearIGDs()
+NatPmp::clearIgds()
{
// Lock valid IGD.
std::lock_guard<std::mutex> lock(validIgdMutex_);
@@ -113,7 +113,7 @@ NatPmp::clearIGDs()
}
void
-NatPmp::searchForIGD()
+NatPmp::searchForIgd()
{
// Lock valid IGD.
std::lock_guard<std::mutex> lock(validIgdMutex_);
diff --git a/src/upnp/protocol/natpmp/nat_pmp.h b/src/upnp/protocol/natpmp/nat_pmp.h
index f463c9fa6240832620defeacc934bfef426c22a1..ab3b409d881a26ea6d38df007d7d664078cbdabc 100644
--- a/src/upnp/protocol/natpmp/nat_pmp.h
+++ b/src/upnp/protocol/natpmp/nat_pmp.h
@@ -55,10 +55,10 @@ public:
Type getType() const override { return Type::NAT_PMP; }
// Notifies a change in network.
- void clearIGDs() override;
+ void clearIgds() override;
// Renew pmp_igd.
- void searchForIGD() override;
+ void searchForIgd() override;
// Tries to add mapping. Assumes mutex is already locked.
Mapping addMapping(IGD* igd, uint16_t port_external, uint16_t port_internal, PortType type, UPnPProtocol::UpnpError& upnp_error) override;
diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp
index 0b4b00659d3cdbd91d4f95327724695c3e9c9a93..59490b16c9ce84f5c672b2fda0b9d5c687cdb731 100644
--- a/src/upnp/protocol/pupnp/pupnp.cpp
+++ b/src/upnp/protocol/pupnp/pupnp.cpp
@@ -2,7 +2,8 @@
* Copyright (C) 2004-2019 Savoir-faire Linux Inc.
*
* Author: Stepan Salenikovich <stepan.salenikovich@savoirfairelinux.com>
- * Author: Eden Abitbol <eden.abitbol@savoirfairelinux.com>
+ * Author: Eden Abitbol <eden.abitbol@savoirfairelinux.com>
+ * Author: Adrien Béraud <adrien.beraud@savoirfairelinux.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -21,6 +22,8 @@
#include "pupnp.h"
+#include <opendht/thread_pool.h>
+
namespace jami { namespace upnp {
// Helper functions for xml parsing.
@@ -80,7 +83,64 @@ errorOnResponse(IXML_Document* doc)
PUPnP::PUPnP()
{
- pupnpThread_ = std::thread([this] { registerClientAsync(); });
+ pupnpThread_ = std::thread([this] {
+ std::unique_lock<std::mutex> lk(ctrlptMutex_);
+ while (pupnpRun_) {
+ pupnpCv_.wait(lk);
+
+ if (not clientRegistered_) {
+ // 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));
+ else
+ clientRegistered_ = true;
+ }
+
+ if (not pupnpRun_)
+ break;
+
+ if (clientRegistered_ and searchForIgd_) {
+ // Send out search for multiple types of devices, as some routers may possibly only reply to one.
+ UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_ROOT_DEVICE, this);
+ UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_IGD_DEVICE, this);
+ UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_WANIP_SERVICE, this);
+ UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_WANPPP_SERVICE, this);
+
+ // Reset variable.
+ searchForIgd_ = false;
+ }
+
+ if (clientRegistered_) {
+ auto xmlList = std::move(dwnldlXmlList_);
+ decltype(xmlList) finished {};
+
+ // Wait on futures asynchronously
+ lk.unlock();
+ for (auto it = xmlList.begin(); it != xmlList.end();) {
+ if (it->wait_for(std::chrono::seconds(1)) == std::future_status::ready) {
+ finished.splice(finished.end(), xmlList, it++);
+ } else {
+ JAMI_WARN("PUPnP: XML download timed out");
+ ++it;
+ }
+ }
+ lk.lock();
+
+ // Move back failed items to end of list
+ dwnldlXmlList_.splice(dwnldlXmlList_.end(), xmlList);
+ // Handle successful downloads
+ for (auto& item : finished) {
+ auto result = item.get();
+ if (not result.document or not validateIgd(result)) {
+ cpDeviceList_.erase(result.location);
+ }
+ }
+ }
+ }
+
+ UpnpFinish();
+ });
int upnp_err = UPNP_E_SUCCESS;
char* ip_address = nullptr;
@@ -109,11 +169,10 @@ PUPnP::PUPnP()
ip_address6 = UpnpGetServerIp6Address();
port6 = UpnpGetServerPort6();
#endif
- if (ip_address6 and port6) {
+ if (ip_address6 and port6)
JAMI_DBG("PUPnP: Initialiazed on %s:%u | %s:%u", ip_address, port, ip_address6, port6);
- } else {
+ else
JAMI_DBG("PUPnP: Initialiazed on %s:%u", ip_address, port);
- }
// Relax the parser to allow malformed XML text.
ixmlRelaxParser(1);
@@ -122,47 +181,133 @@ PUPnP::PUPnP()
PUPnP::~PUPnP()
{
+ pupnpCv_.notify_all();
// Clear all the lists.
{
std::lock_guard<std::mutex> lk(validIgdMutex_);
for(auto const &it : validIgdList_) {
- if (auto igd = dynamic_cast<UPnPIGD*>(it.second.get()))
+ if (auto igd = std::dynamic_pointer_cast<UPnPIGD>(it.second))
actionDeletePortMappingsByDesc(*igd, Mapping::UPNP_DEFAULT_MAPPING_DESCRIPTION);
}
validIgdList_.clear();
cpDeviceList_.clear();
+ dwnldlXmlList_.clear();
}
// Notify thread to terminate. UpnpFinish function will get called.
pupnpRun_ = false;
pupnpCv_.notify_all();
- if (pupnpThread_.joinable()) {
+ if (pupnpThread_.joinable())
pupnpThread_.join();
- }
}
void
-PUPnP::clearIGDs()
+PUPnP::clearIgds()
{
// Lock internal IGD list.
std::lock_guard<std::mutex> lk(validIgdMutex_);
- // Clear internal IGD list.
+ // Clear all internal lists.
+ dwnldlXmlList_.clear();
validIgdList_.clear();
cpDeviceList_.clear();
}
void
-PUPnP::searchForIGD()
+PUPnP::searchForIgd()
{
- // Notify registerClientAsync function running in thread to execute in non-blocking fashion.
+ // Notify thread to execute in non-blocking fashion.
{
std::lock_guard<std::mutex> lk(ctrlptMutex_);
- pupnpRun_ = true;
+ searchForIgd_ = true;
}
pupnpCv_.notify_one();
}
+bool
+PUPnP::validateIgd(const IGDInfo& info)
+{
+ auto descDoc = info.document.get();
+ // Check device type.
+ std::string deviceType = getFirstDocItem(descDoc, "deviceType");
+ if (deviceType.empty()) {
+ // No device type.
+ return false;
+ }
+
+ if (deviceType.compare(UPNP_IGD_DEVICE) != 0) {
+ // Device type not IGD.
+ return false;
+ }
+
+ auto igd_candidate = parseIgd(descDoc, info.location);
+ if (not igd_candidate) {
+ // No valid IGD candidate.
+ return false;
+ }
+
+ JAMI_DBG("PUPnP: Validating IGD candidate\n"
+ " UDN: %s\n"
+ " Base URL: %s\n"
+ " Name: %s\n"
+ " serviceType: %s\n"
+ " serviceID: %s\n"
+ " locationURL: %s\n"
+ " controlURL: %s\n"
+ " eventSubURL: %s",
+ igd_candidate->getUDN().c_str(),
+ igd_candidate->getBaseURL().c_str(),
+ igd_candidate->getFriendlyName().c_str(),
+ igd_candidate->getServiceType().c_str(),
+ igd_candidate->getServiceId().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->getUDN().c_str());
+ return false;
+ }
+
+ // Validate external Ip.
+ igd_candidate->publicIp_ = actionGetExternalIP(*igd_candidate);
+ if (igd_candidate->publicIp_.toString().empty()) {
+ JAMI_WARN("PUPnP: IGD candidate %s has no valid external Ip", igd_candidate->getUDN().c_str());
+ return false;
+ }
+
+ // Validate internal Ip.
+ igd_candidate->localIp_ = ip_utils::getLocalAddr(pj_AF_INET());
+ if (igd_candidate->localIp_.toString().empty()) {
+ JAMI_WARN("PUPnP: No valid internal Ip.");
+ return false;
+ }
+
+ JAMI_DBG("PUPnP: Found device with external IP %s", igd_candidate->publicIp_.toString().c_str());
+
+ // Store info for subscription.
+ std::string eventSub = igd_candidate->getEventSubURL();
+ std::string udn = igd_candidate->getUDN();
+
+ // Remove any local mappings that may be left over from last time used.
+ removeAllLocalMappings(igd_candidate.get());
+
+ // Add the igd to the upnp context class list.
+ if (updateIgdListCb_(this, std::move(igd_candidate.get()), std::move(igd_candidate.get()->publicIp_), true))
+ JAMI_DBG("PUPnP: IGD with public IP %s was added to the list", igd_candidate->publicIp_.toString().c_str());
+
+ // Keep local IGD list internally.
+ validIgdList_.emplace(igd_candidate->getUDN(), std::move(igd_candidate)).first;
+
+ // Subscribe to IGD events.
+ int upnp_err = UpnpSubscribeAsync(ctrlptHandle_, eventSub.c_str(), SUBSCRIBE_TIMEOUT, subEventCallback, this);
+ if (upnp_err != UPNP_E_SUCCESS)
+ JAMI_WARN("PUPnP: Error when trying to request subscription for %s -> %s", udn.c_str(), UpnpGetErrorMessage(upnp_err));
+
+ return true;
+}
+
Mapping
PUPnP::addMapping(IGD* igd, uint16_t port_external, uint16_t port_internal, PortType type, UPnPProtocol::UpnpError& upnp_error)
{
@@ -179,17 +324,17 @@ PUPnP::addMapping(IGD* igd, uint16_t port_external, uint16_t port_internal, Port
auto globalMappings = type == PortType::UDP ? &igd->udpMappings : &igd->tcpMappings;
auto iter = globalMappings->find(port_external);
if (iter != globalMappings->end()) {
- /* mapping exists with same external port */
+ // Mapping exists with same external port.
GlobalMapping* mapping_ptr = &iter->second;
if (*mapping_ptr == mapping) {
- /* the same mapping, so nothing needs to be done */
+ // The same mapping, so nothing needs to be done.
upnp_error = UPnPProtocol::UpnpError::ERROR_OK;
++(mapping_ptr->users);
JAMI_DBG("PUPnP: Mapping already exists, incrementing number of users: %d",
iter->second.users);
return mapping;
} else {
- /* this port is already used by a different mapping */
+ // This port is already used by a different mapping.
JAMI_WARN("PUPnP: Cannot add a mapping with an external port which is already used by another:\n\tcurrent: %s\n\ttrying to add: %s",
mapping_ptr->toString().c_str(), mapping.toString().c_str());
upnp_error = UPnPProtocol::UpnpError::CONFLICT_IN_MAPPING;
@@ -197,7 +342,7 @@ PUPnP::addMapping(IGD* igd, uint16_t port_external, uint16_t port_internal, Port
}
}
- if (auto pupnp_igd = dynamic_cast<const UPnPIGD*>(igd)) {
+ if (auto pupnp_igd = dynamic_cast<const UPnPIGD*>(igd)) {
if (actionAddPortMapping(*pupnp_igd, mapping, upnp_error)) {
JAMI_WARN("PUPnP: Opened port %s", mapping.toString().c_str());
globalMappings->emplace(port_external, GlobalMapping{mapping});
@@ -216,17 +361,15 @@ PUPnP::removeMapping(const Mapping& igdMapping)
// Iterate over all IGDs in internal list and try to remove selected mapping.
for (auto const& item : validIgdList_) {
- if (not item.second) {
+ if (not item.second)
continue;
- }
// Get mappings of IGD depending on type.
PortMapGlobal* globalMappings;
- if (igdMapping.getType() == PortType::UDP) {
+ if (igdMapping.getType() == PortType::UDP)
globalMappings = &item.second->udpMappings;
- } else {
+ else
globalMappings = &item.second->tcpMappings;
- }
// Check if the mapping we want to remove is present in the IGD.
auto mapToRemove = globalMappings->find(igdMapping.getPortExternal());
@@ -258,52 +401,16 @@ PUPnP::removeMapping(const Mapping& igdMapping)
void
PUPnP::removeAllLocalMappings(IGD* igd)
{
- if (auto igd_del_map = dynamic_cast<UPnPIGD*>(igd)) {
+ if (auto igd_del_map = dynamic_cast<UPnPIGD*>(igd))
actionDeletePortMappingsByDesc(*igd_del_map, Mapping::UPNP_DEFAULT_MAPPING_DESCRIPTION);
- }
-}
-
-void
-PUPnP::registerClientAsync()
-{
- std::unique_lock<std::mutex> lk(ctrlptMutex_);
-
- while (pupnpRun_) {
-
- pupnpCv_.wait(lk);
-
- if (not clientRegistered_) {
- // 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));
- } else {
- clientRegistered_ = true;
- }
- }
-
- if (not pupnpRun_) {
- break;
- }
-
- if (clientRegistered_) {
- // Send out search for multiple types of devices, as some routers may possibly only reply to one.
- UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_ROOT_DEVICE, this);
- UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_IGD_DEVICE, this);
- UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_WANIP_SERVICE, this);
- UpnpSearchAsync(ctrlptHandle_, SEARCH_TIMEOUT, UPNP_WANPPP_SERVICE, this);
- }
- }
-
- UpnpFinish();
}
int
PUPnP::ctrlPtCallback(Upnp_EventType event_type, const void* event, void* user_data)
{
- if (auto pupnp = static_cast<PUPnP*>(user_data)) {
+ if (auto pupnp = static_cast<PUPnP*>(user_data))
return pupnp->handleCtrlPtUPnPEvents(event_type, event);
- }
+
JAMI_WARN("PUPnP: Control point callback without PUPnP");
return 0;
}
@@ -320,122 +427,38 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
case UPNP_DISCOVERY_SEARCH_RESULT:
{
const UpnpDiscovery* d_event = (const UpnpDiscovery*)event;
- int upnp_err;
// First check the error code.
- if (UpnpDiscovery_get_ErrCode(d_event) != UPNP_E_SUCCESS) {
+ if (UpnpDiscovery_get_ErrCode(d_event) != UPNP_E_SUCCESS)
break;
- }
// Check if this device ID is already in the list.
std::string cpDeviceId = UpnpDiscovery_get_DeviceID_cstr(d_event);
std::lock_guard<std::mutex> lk(validIgdMutex_);
- if (cpDeviceList_.count(cpDeviceId) > 0) {
+ if (not cpDeviceList_.emplace(cpDeviceId).second)
break;
- }
- cpDeviceList_.emplace(std::move(cpDeviceId), std::string{});
-
- /*
- * NOTE: This thing will block until success for the system socket timeout
- * unless libupnp is compile with '-disable-blocking-tcp-connections', in
- * which case it will block for the libupnp specified timeout.
- */
- IXML_Document* doc_container_ptr = nullptr;
- std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&> doc_desc_ptr(nullptr, ixmlDocument_free);
- upnp_err = UpnpDownloadXmlDoc(UpnpDiscovery_get_Location_cstr(d_event), &doc_container_ptr);
- if (doc_container_ptr) {
- doc_desc_ptr.reset(doc_container_ptr);
- }
-
- if (upnp_err != UPNP_E_SUCCESS or not doc_desc_ptr) {
- JAMI_WARN("PUPnP: Error downloading device XML document -> %s", UpnpGetErrorMessage(upnp_err));
- break;
- }
-
- // Check device type.
- std::string deviceType = getFirstDocItem(doc_desc_ptr.get(), "deviceType");
- if (deviceType.empty()) {
- // No device type. Exit.
- break;
- }
-
- if (deviceType.compare(UPNP_IGD_DEVICE) != 0) {
- // Device type not IGD. Exit.
- break;
- }
-
- std::unique_ptr<UPnPIGD> igd_candidate;
- igd_candidate = parseIGD(doc_desc_ptr.get(), d_event);
- if (not igd_candidate) {
- // No valid IGD candidate. Exit.
- break;
- }
-
- JAMI_DBG("PUPnP: Validating IGD candidate.\n\tUDN: %s\n\tBase URL: %s\n\tName: %s\n\tserviceType: %s\n\tserviceID: %s\n\tcontrolURL: %s\n\teventSubURL: %s",
- igd_candidate->getUDN().c_str(),
- igd_candidate->getBaseURL().c_str(),
- igd_candidate->getFriendlyName().c_str(),
- igd_candidate->getServiceType().c_str(),
- igd_candidate->getServiceId().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->getUDN().c_str());
- break;
- }
-
- // Validate external Ip.
- igd_candidate->publicIp_ = actionGetExternalIP(*igd_candidate);
- if (igd_candidate->publicIp_.toString().empty()) {
- JAMI_WARN("PUPnP: IGD candidate %s has no valid external Ip", igd_candidate->getUDN().c_str());
- break;
- }
-
- // Validate internal Ip.
- igd_candidate->localIp_ = ip_utils::getLocalAddr(pj_AF_INET());
- if (igd_candidate->localIp_.toString().empty()) {
- JAMI_WARN("PUPnP: No valid internal Ip.");
- break;
- }
-
- JAMI_DBG("PUPnP: Found device with external IP %s", igd_candidate->publicIp_.toString().c_str());
-
- // Store public IP.
- std::string publicIpStr(std::move(igd_candidate->publicIp_.toString()));
-
- // Store info for subscription.
- std::string eventSub = igd_candidate->getEventSubURL();
- std::string udn = igd_candidate->getUDN();
-
- // Remove any local mappings that may be left over from last time used.
- removeAllLocalMappings(igd_candidate.get());
- // Add the igd to the upnp context class list.
- if (updateIgdListCb_(this, std::move(igd_candidate.get()), std::move(igd_candidate.get()->publicIp_), true)) {
- JAMI_DBG("PUPnP: IGD with public IP %s was added to the list", publicIpStr.c_str());
- } else {
- JAMI_DBG("PUPnP: IGD with public IP %s is already in the list", publicIpStr.c_str());
- }
+ // Check if we already downloaded the xml doc based on the igd location string.
+ std::string igdLocationUrl {UpnpDiscovery_get_Location_cstr(d_event)};
+ dwnldlXmlList_.emplace_back(dht::ThreadPool::io().get<IGDInfo>([this, location = std::move(igdLocationUrl)]{
+ IXML_Document* doc_container_ptr = nullptr;
+ XMLDocument doc_desc_ptr(nullptr, ixmlDocument_free);
+ int upnp_err = UpnpDownloadXmlDoc(location.c_str(), &doc_container_ptr);
+ if (doc_container_ptr)
+ doc_desc_ptr.reset(doc_container_ptr);
+ pupnpCv_.notify_all();
+ if (upnp_err != UPNP_E_SUCCESS or not doc_desc_ptr)
+ JAMI_WARN("PUPnP: Error downloading device XML document -> %s", UpnpGetErrorMessage(upnp_err));
+ else
+ return IGDInfo {std::move(location), std::move(doc_desc_ptr)};
+ return IGDInfo {std::move(location), XMLDocument(nullptr, ixmlDocument_free)};
+ }));
- // Keep local IGD list internally.
- if (cpDeviceList_.count(udn) > 0) {
- cpDeviceList_[udn] = eventSub;
- }
- validIgdList_.emplace(std::move(igd_candidate->getUDN()), std::move(igd_candidate));
-
- // Subscribe to IGD events.
- upnp_err = UpnpSubscribeAsync(ctrlptHandle_, eventSub.c_str(), SUBSCRIBE_TIMEOUT, subEventCallback, this);
- if (upnp_err != UPNP_E_SUCCESS) {
- JAMI_WARN("PUPnP: Error when trying to request subscription for %s -> %s", udn.c_str(), UpnpGetErrorMessage(upnp_err));
- }
break;
}
case UPNP_DISCOVERY_ADVERTISEMENT_BYEBYE:
{
const UpnpDiscovery *d_event = (const UpnpDiscovery *)event;
-
std::lock_guard<std::mutex> lk(validIgdMutex_);
// Remvoe device Id from list.
@@ -444,7 +467,6 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
IGD* igd_to_remove = nullptr;
for (auto it = validIgdList_.find(cpDeviceId); it != validIgdList_.end(); it++) {
-
// Store igd to remove.
igd_to_remove = it->second.get();
@@ -471,32 +493,17 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
case UPNP_EVENT_SUBSCRIPTION_EXPIRED: // This event will occur only if autorenewal is disabled.
{
const UpnpEventSubscribe *es_event = (const UpnpEventSubscribe *)event;
-
std::string eventSubUrl(UpnpEventSubscribe_get_PublisherUrl_cstr(es_event));
- std::pair<std::string, std::string> foundDevice = std::make_pair("", "");
- bool foundEventSubUrl = false;
- auto it = cpDeviceList_.begin();
- while(it != cpDeviceList_.end()) {
- if(it->second == eventSubUrl) {
- foundEventSubUrl = true;
- foundDevice = std::make_pair(it->first, it->second);
- break;
- }
- it++;
- }
-
- if (not foundEventSubUrl) {
- // If we don't find event subscription url then exit.
- break;
+ auto it = validIgdList_.begin();
+ for (; it != validIgdList_.end(); it++) {
+ if (auto igd = std::dynamic_pointer_cast<UPnPIGD>(it->second))
+ if (igd->getEventSubURL() == eventSubUrl) {
+ UpnpSubscribeAsync(ctrlptHandle_, eventSubUrl.c_str(), SUBSCRIBE_TIMEOUT, subEventCallback, this);
+ break;
+ }
}
- std::string udn = foundDevice.first;
- std::string eventSub = foundDevice.second;
-
- // Renew subscriptons to IGD events.
- UpnpSubscribeAsync(ctrlptHandle_, eventSub.c_str(), SUBSCRIBE_TIMEOUT, subEventCallback, this);
-
break;
}
case UPNP_EVENT_SUBSCRIBE_COMPLETE:
@@ -524,15 +531,14 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
int
PUPnP::subEventCallback(Upnp_EventType event_type, const void* event, void* user_data)
{
- if (auto pupnp = static_cast<PUPnP*>(user_data)) {
+ if (auto pupnp = static_cast<PUPnP*>(user_data))
return pupnp->handleSubscriptionUPnPEvent(event_type, event);
- }
JAMI_WARN("PUPnP: Subscription callback without service Id string");
return 0;
}
int
-PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType event_type, const void* event)
+PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType /*event_type */, const void* event)
{
std::lock_guard<std::mutex> lk(ctrlptMutex_);
@@ -550,9 +556,9 @@ PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType event_type, const void* event)
}
std::unique_ptr<UPnPIGD>
-PUPnP::parseIGD(IXML_Document* doc, const UpnpDiscovery* d_event)
+PUPnP::parseIgd(IXML_Document* doc, std::string locationUrl)
{
- if (not doc or not d_event)
+ if (not (doc and locationUrl.c_str()))
return nullptr;
// Check the UDN to see if its already in our device list.
@@ -576,9 +582,8 @@ PUPnP::parseIGD(IXML_Document* doc, const UpnpDiscovery* d_event)
// Get base URL.
std::string baseURL = getFirstDocItem(doc, "URLBase");
- if (baseURL.empty()) {
- baseURL = std::string(UpnpDiscovery_get_Location_cstr(d_event));
- }
+ if (baseURL.empty())
+ baseURL = locationUrl;
// Get list of services defined by serviceType.
std::unique_ptr<IXML_NodeList, decltype(ixmlNodeList_free)&> serviceList(nullptr, ixmlNodeList_free);
@@ -627,11 +632,11 @@ PUPnP::parseIGD(IXML_Document* doc, const UpnpDiscovery* d_event)
char* absolute_control_url = nullptr;
upnp_err = UpnpResolveURL2(baseURL.c_str(), controlURL.c_str(), &absolute_control_url);
- if (upnp_err == UPNP_E_SUCCESS) {
+ if (upnp_err == UPNP_E_SUCCESS)
controlURL = absolute_control_url;
- } else {
+ else
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.
@@ -643,11 +648,11 @@ PUPnP::parseIGD(IXML_Document* doc, const UpnpDiscovery* d_event)
char* absolute_event_sub_url = nullptr;
upnp_err = UpnpResolveURL2(baseURL.c_str(), eventSubURL.c_str(), &absolute_event_sub_url);
- if (upnp_err == UPNP_E_SUCCESS) {
+ if (upnp_err == UPNP_E_SUCCESS)
eventSubURL = absolute_event_sub_url;
- } else {
+ else
JAMI_WARN("PUPnP: Error resolving absolute eventSubURL -> %s", UpnpGetErrorMessage(upnp_err));
- }
+
std::free(absolute_event_sub_url);
new_igd.reset(new UPnPIGD(std::move(UDN),
@@ -655,6 +660,7 @@ PUPnP::parseIGD(IXML_Document* doc, const UpnpDiscovery* d_event)
std::move(friendlyName),
std::move(serviceType),
std::move(serviceId),
+ std::move(locationUrl),
std::move(controlURL),
std::move(eventSubURL)));
@@ -667,9 +673,8 @@ PUPnP::parseIGD(IXML_Document* doc, const UpnpDiscovery* d_event)
bool
PUPnP::actionIsIgdConnected(const UPnPIGD& igd)
{
- if (not clientRegistered_) {
+ if (not clientRegistered_)
return false;
- }
// Action and response pointers.
std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&> action(nullptr, ixmlDocument_free); // Action pointer.
@@ -701,9 +706,8 @@ PUPnP::actionIsIgdConnected(const UPnPIGD& igd)
// Parse response.
std::string status = getFirstDocItem(response.get(), "NewConnectionStatus");
- if (status.compare("Connected") != 0) {
+ if (status.compare("Connected") != 0)
return false;
- }
return true;
}
@@ -711,9 +715,8 @@ PUPnP::actionIsIgdConnected(const UPnPIGD& igd)
IpAddr
PUPnP::actionGetExternalIP(const UPnPIGD& igd)
{
- if (not clientRegistered_) {
+ if (not clientRegistered_)
return {};
- }
// Action and response pointers.
std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&> action(nullptr, ixmlDocument_free); // Action pointer.
@@ -749,13 +752,8 @@ PUPnP::actionGetExternalIP(const UPnPIGD& igd)
void
PUPnP::actionDeletePortMappingsByDesc(const UPnPIGD& igd, const std::string& description)
{
- if (not clientRegistered_) {
- return;
- }
-
- if (!igd.localIp_) {
+ if (not (clientRegistered_ and igd.localIp_))
return;
- }
// Set action name.
std::string action_name { "GetGenericPortMappingEntry" };
@@ -821,9 +819,8 @@ PUPnP::actionDeletePortMappingsByDesc(const UPnPIGD& igd, const std::string& des
bool
PUPnP::actionDeletePortMapping(const UPnPIGD& igd, const std::string& port_external, const std::string& protocol)
{
- if (not clientRegistered_) {
+ if (not clientRegistered_)
return false;
- }
// Action and response pointers.
std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&> action(nullptr, ixmlDocument_free); // Action pointer.
@@ -869,9 +866,8 @@ PUPnP::actionDeletePortMapping(const UPnPIGD& igd, const std::string& port_exter
bool
PUPnP::actionAddPortMapping(const UPnPIGD& igd, const Mapping& mapping, UPnPProtocol::UpnpError& error_code)
{
- if (not clientRegistered_) {
+ if (not clientRegistered_)
return false;
- }
error_code = UPnPProtocol::UpnpError::ERROR_OK;
diff --git a/src/upnp/protocol/pupnp/pupnp.h b/src/upnp/protocol/pupnp/pupnp.h
index 52de8fa94690d39de1f2b3a3baf5894b6eb578f2..ca76786f22080b6fb0524bdec76cd8ac93db1830 100644
--- a/src/upnp/protocol/pupnp/pupnp.h
+++ b/src/upnp/protocol/pupnp/pupnp.h
@@ -2,7 +2,7 @@
* Copyright (C) 2004-2019 Savoir-faire Linux Inc.
*
* Author: Stepan Salenikovich <stepan.salenikovich@savoirfairelinux.com>
- * Author: Eden Abitbol <eden.abitbol@savoirfairelinux.com>
+ * Author: Eden Abitbol <eden.abitbol@savoirfairelinux.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -50,6 +50,12 @@
#include <atomic>
#include <thread>
+#include <list>
+#include <map>
+#include <set>
+#include <string>
+#include <memory>
+#include <future>
namespace jami {
class IpAddr;
@@ -68,6 +74,12 @@ constexpr static unsigned int SUBSCRIBE_TIMEOUT {300};
class PUPnP : public UPnPProtocol
{
public:
+ using XMLDocument = std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&>;
+ struct IGDInfo {
+ std::string location;
+ XMLDocument document;
+ };
+
PUPnP();
~PUPnP();
@@ -75,10 +87,10 @@ public:
Type getType() const override { return Type::PUPNP; }
// Notifies a change in network.
- void clearIGDs() override;
+ void clearIgds() override;
// Sends out async search for IGD.
- void searchForIGD() override;
+ void searchForIgd() override;
// Tries to add mapping. Assumes mutex is already locked.
Mapping addMapping(IGD* igd, uint16_t port_external, uint16_t port_internal, PortType type, UPnPProtocol::UpnpError& upnp_error) override;
@@ -90,8 +102,8 @@ public:
void removeAllLocalMappings(IGD* igd) override;
private:
- // Register client in an async manner using a thread.
- void registerClientAsync();
+ // Validate IGD from the xml document received from the router.
+ bool validateIgd(const IGDInfo&);
// Control point callback.
static int ctrlPtCallback(Upnp_EventType event_type, const void* event, void* user_data);
@@ -116,7 +128,7 @@ private:
int handleSubscriptionUPnPEvent(Upnp_EventType event_type, const void* event);
// Parses the IGD candidate.
- std::unique_ptr<UPnPIGD> parseIGD(IXML_Document* doc, const UpnpDiscovery* d_event);
+ std::unique_ptr<UPnPIGD> parseIgd(IXML_Document* doc, std::string locationUrl);
// These functions directly create UPnP actions and make synchronous UPnP control point calls. Assumes mutex is already locked.
bool actionIsIgdConnected(const UPnPIGD& igd);
@@ -133,11 +145,14 @@ private:
std::thread pupnpThread_ {}; // PUPnP thread for non-blocking client registration.
std::map<std::string, std::shared_ptr<IGD>> validIgdList_; // Map of valid IGDs with their UDN (universal Id).
- std::map<std::string, std::string> cpDeviceList_; // Control point device list containing the device ID and device subscription event url.
+ std::set<std::string> cpDeviceList_; // Control point device list containing the device ID and device subscription event url.
+ std::list<std::future<IGDInfo>> dwnldlXmlList_; // List of shared_futures for blocking xml download function calls.
std::mutex ctrlptMutex_; // Mutex for client handle protection.
UpnpClient_Handle ctrlptHandle_ {-1}; // Control point handle.
+
std::atomic_bool clientRegistered_ { false }; // Indicates of the client is registered.
+ std::atomic_bool searchForIgd_ { false }; // Variable to signal thread for a search.
};
}} // namespace jami::upnp
\ No newline at end of file
diff --git a/src/upnp/protocol/pupnp/upnp_igd.cpp b/src/upnp/protocol/pupnp/upnp_igd.cpp
index a355ed1ce422a7969f2e5e54517e2657c114718b..3323799d40f450bac419faf24a15a097042764bd 100644
--- a/src/upnp/protocol/pupnp/upnp_igd.cpp
+++ b/src/upnp/protocol/pupnp/upnp_igd.cpp
@@ -23,15 +23,19 @@
namespace jami { namespace upnp {
-UPnPIGD::UPnPIGD(std::string&& UDN, std::string&& baseURL, std::string&& friendlyName, std::string&& serviceType, std::string&& serviceId, std::string&& controlURL, std::string&& eventSubURL,
- IpAddr&& localIp, IpAddr&& publicIp):
- IGD(std::move(localIp), std::move(publicIp))
+UPnPIGD::UPnPIGD(std::string&& UDN, std::string&& baseURL,
+ std::string&& friendlyName, std::string&& serviceType,
+ std::string&& serviceId, std::string && locationURL,
+ std::string&& controlURL, std::string&& eventSubURL,
+ IpAddr&& localIp, IpAddr&& publicIp):
+ IGD(std::move(localIp), std::move(publicIp))
{
UDN_ = std::move(UDN);
baseURL_ = std::move(baseURL);
friendlyName_ = std::move(friendlyName);
serviceType_ = std::move(serviceType);
serviceId_ = std::move(serviceId);
+ locationURL_ = std::move(locationURL);
controlURL_ = std::move(controlURL);
eventSubURL_ = std::move(eventSubURL);
}
@@ -56,6 +60,7 @@ UPnPIGD::operator==(UPnPIGD& other) const
friendlyName_ == other.friendlyName_ and
serviceType_ == other.serviceType_ and
serviceId_ == other.serviceId_ and
+ locationURL_ == other.locationURL_ and
controlURL_ == other.controlURL_ and
eventSubURL_ == other.eventSubURL_;
}
diff --git a/src/upnp/protocol/pupnp/upnp_igd.h b/src/upnp/protocol/pupnp/upnp_igd.h
index 01638dcd1518166b5acb10926747b12986a1e056..22e51066296d3bf2520b3bdaece6e918e3905932 100644
--- a/src/upnp/protocol/pupnp/upnp_igd.h
+++ b/src/upnp/protocol/pupnp/upnp_igd.h
@@ -47,18 +47,20 @@ public:
std::string&& friendlyName,
std::string&& serviceType,
std::string&& serviceId,
+ std::string&& locationURL,
std::string&& controlURL,
std::string&& eventSubURL,
IpAddr&& localIp = {},
IpAddr&& publicIp = {});
~UPnPIGD(){}
- const std::string& getUDN() const { return UDN_; };
- const std::string& getBaseURL() const { return baseURL_; };
- const std::string& getFriendlyName() const { return friendlyName_; };
- const std::string& getServiceType() const { return serviceType_; };
- const std::string& getServiceId() const { return serviceId_; };
- const std::string& getControlURL() const { return controlURL_; };
- const std::string& getEventSubURL() const { return eventSubURL_; };
+ const std::string& getUDN() const { return UDN_; }
+ const std::string& getBaseURL() const { return baseURL_; }
+ const std::string& getFriendlyName() const { return friendlyName_; }
+ const std::string& getServiceType() const { return serviceType_; }
+ const std::string& getServiceId() const { return serviceId_; }
+ const std::string& getLocationURL() const { return locationURL_; }
+ const std::string& getControlURL() const { return controlURL_; }
+ const std::string& getEventSubURL() const { return eventSubURL_; }
bool operator==(IGD& other) const;
bool operator==(UPnPIGD& other) const;
@@ -69,6 +71,7 @@ private:
std::string friendlyName_ {};
std::string serviceType_ {};
std::string serviceId_ {};
+ std::string locationURL_ {};
std::string controlURL_ {};
std::string eventSubURL_ {};
};
diff --git a/src/upnp/protocol/upnp_protocol.h b/src/upnp/protocol/upnp_protocol.h
index b258163411e68e2d0fc0987e609a58661faf682e..e5ec27f7ccb17bdbf75771ae74d2f3b0e0684d16 100644
--- a/src/upnp/protocol/upnp_protocol.h
+++ b/src/upnp/protocol/upnp_protocol.h
@@ -73,10 +73,10 @@ public:
virtual Type getType() const = 0;
// Clear all known IGDs.
- virtual void clearIGDs() = 0;
+ virtual void clearIgds() = 0;
// Search for IGD.
- virtual void searchForIGD() = 0;
+ virtual void searchForIgd() = 0;
// Tries to add mapping. Assumes mutex is already locked.
virtual Mapping addMapping(IGD* igd, uint16_t port_external, uint16_t port_internal, PortType type, UPnPProtocol::UpnpError& upnp_error) = 0;
diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp
index 0afafeefc0585bc16d4465b4871b5118a61dfab9..51f112e6333a9a30265fb42bb9c2a0d216bd7150 100644
--- a/src/upnp/upnp_context.cpp
+++ b/src/upnp/upnp_context.cpp
@@ -48,13 +48,13 @@ UPnPContext::UPnPContext()
#if HAVE_LIBNATPMP
auto natPmp = std::make_unique<NatPmp>();
natPmp->setOnIgdChanged(std::bind(&UPnPContext::igdListChanged, this, _1, _2, _3, _4));
- natPmp->searchForIGD();
+ natPmp->searchForIgd();
protocolList_.push_back(std::move(natPmp));
#endif
#if HAVE_LIBUPNP
auto pupnp = std::make_unique<PUPnP>();
pupnp->setOnIgdChanged(std::bind(&UPnPContext::igdListChanged, this, _1, _2, _3, _4));
- pupnp->searchForIGD();
+ pupnp->searchForIgd();
protocolList_.push_back(std::move(pupnp));
#endif
}
@@ -70,7 +70,7 @@ UPnPContext::connectivityChanged()
{
std::lock_guard<std::mutex> lock(igdListMutex_);
for (auto const& protocol : protocolList_)
- protocol->clearIGDs();
+ protocol->clearIgds();
if (not igdList_.empty()) {
// Clear main IGD list.
igdList_.clear();
@@ -81,7 +81,7 @@ UPnPContext::connectivityChanged()
}
for (auto const& protocol : protocolList_)
- protocol->searchForIGD();
+ protocol->searchForIgd();
}
bool
@@ -306,6 +306,7 @@ UPnPContext::addIgdToList(UPnPProtocol* protocol, IGD* igd)
}
if (isIgdInList(igd->publicIp_)) {
+ JAMI_DBG("UPnPContext: IGD with public IP %s is already in the list", igd->publicIp_.toString().c_str());
return false;
}