Skip to content

Commit

Permalink
Fix UPnP error detection
Browse files Browse the repository at this point in the history
  • Loading branch information
@anr2me
anr2me committed Jul 15, 2020
1 parent 06f125a commit 6bb15b8
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 21 deletions.
56 changes: 35 additions & 21 deletions Core/Util/PortManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ PortManager::PortManager():
urls(0),
datas(0),
m_InitState(UPNP_INITSTATE_NONE),
m_LocalPort(UPNP_LOCAL_PORT_ANY) {
m_LocalPort(UPNP_LOCAL_PORT_ANY),
m_leaseDuration("43200") {
}

PortManager::~PortManager() {
Expand All @@ -63,6 +64,7 @@ void PortManager::Deinit() {
m_portList.clear(); m_portList.shrink_to_fit();
m_lanip.clear();
m_defaultDesc.clear();
m_leaseDuration.clear();
m_LocalPort = UPNP_LOCAL_PORT_ANY;
m_InitState = UPNP_INITSTATE_DONE;
}
Expand Down Expand Up @@ -102,6 +104,7 @@ bool PortManager::Init(const unsigned int timeout) {
break;
}
}
m_leaseDuration = "43200"; // 12 hours
m_InitState = UPNP_INITSTATE_BUSY;
urls = (UPNPUrls*)malloc(sizeof(struct UPNPUrls));
datas = (IGDdatas*)malloc(sizeof(struct IGDdatas));
Expand Down Expand Up @@ -131,7 +134,7 @@ bool PortManager::Init(const unsigned int timeout) {
GetUPNPUrls(urls, datas, dev->descURL, dev->scope_id);
}

// Get LAN IP address
// Get LAN IP address that connects to the router
char lanaddr[64] = "unset";
int status = UPNP_GetValidIGD(devlist, urls, datas, lanaddr, sizeof(lanaddr)); //possible "status" values, 0 = NO IGD found, 1 = A valid connected IGD has been found, 2 = A valid IGD has been found but it reported as not connected, 3 = an UPnP device has been found but was not recognized as an IGD
m_lanip = std::string(lanaddr);
Expand Down Expand Up @@ -162,7 +165,7 @@ bool PortManager::Init(const unsigned int timeout) {
RefreshPortList();
return true;
}
ERROR_LOG(SCENET, "PortManager - upnpDiscover failed (error: %d) or No UPnP device detected", error);
ERROR_LOG(SCENET, "PortManager - upnpDiscover failed (error: %i) or No UPnP device detected", error);
auto n = GetI18NCategory("Networking");
host->NotifyUserMessage(n->T("Unable to find UPnP device"), 6.0f, 0x0000ff);
m_InitState = UPNP_INITSTATE_NONE;
Expand All @@ -175,7 +178,6 @@ int PortManager::GetInitState() {

bool PortManager::Add(unsigned short port, const char* protocol) {
char port_str[16];
std::string leaseDuration = "43200"; // range(0-604800) in seconds (0 = Indefinite/permanent). Some routers doesn't support non-zero value
int r;

INFO_LOG(SCENET, "PortManager::Add(%d, %s)", port, protocol);
Expand All @@ -196,14 +198,21 @@ bool PortManager::Add(unsigned short port, const char* protocol) {
r = UPNP_DeletePortMapping(urls->controlURL, datas->first.servicetype, port_str, protocol, NULL);
}
r = UPNP_AddPortMapping(urls->controlURL, datas->first.servicetype,
port_str, port_str, m_lanip.c_str(), m_defaultDesc.c_str(), protocol, NULL, leaseDuration.c_str());
port_str, port_str, m_lanip.c_str(), m_defaultDesc.c_str(), protocol, NULL, m_leaseDuration.c_str());
if (r == 725 && m_leaseDuration != "0") {
m_leaseDuration = "0";
r = UPNP_AddPortMapping(urls->controlURL, datas->first.servicetype,
port_str, port_str, m_lanip.c_str(), m_defaultDesc.c_str(), protocol, NULL, m_leaseDuration.c_str());
}
if (r != 0)
{
ERROR_LOG(SCENET, "PortManager - AddPortMapping failed (error: %d)", r);
auto n = GetI18NCategory("Networking");
host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff);
Deinit(); // Most of the time errors occurred because the router is no longer reachable (ie. changed networks) so we should invalidate the state to prevent further lags due to timeouts
return false;
ERROR_LOG(SCENET, "PortManager - AddPortMapping failed (error: %i)", r);
if (r == UPNPCOMMAND_HTTP_ERROR) {
auto n = GetI18NCategory("Networking");
host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff);
Deinit(); // Most of the time errors occurred because the router is no longer reachable (ie. changed networks) so we should invalidate the state to prevent further lags due to timeouts
return false;
}
}
m_portList.push_front({ port_str, protocol });
// Keep tracks of it to be restored later if it belongs to others
Expand All @@ -225,11 +234,13 @@ bool PortManager::Remove(unsigned short port, const char* protocol) {
int r = UPNP_DeletePortMapping(urls->controlURL, datas->first.servicetype, port_str, protocol, NULL);
if (r != 0)
{
ERROR_LOG(SCENET, "PortManager - DeletePortMapping failed (error: %d)", r);
auto n = GetI18NCategory("Networking");
host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff);
Deinit(); // Most of the time errors occurred because the router is no longer reachable (ie. changed networks) so we should invalidate the state to prevent further lags due to timeouts
return false;
ERROR_LOG(SCENET, "PortManager - DeletePortMapping failed (error: %i)", r);
if (r == UPNPCOMMAND_HTTP_ERROR) {
auto n = GetI18NCategory("Networking");
host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff);
Deinit(); // Most of the time errors occurred because the router is no longer reachable (ie. changed networks) so we should invalidate the state to prevent further lags due to timeouts
return false;
}
}
for (auto it = m_portList.begin(); it != m_portList.end(); ) {
(it->first == port_str && it->second == protocol) ? it = m_portList.erase(it) : ++it;
Expand Down Expand Up @@ -258,8 +269,9 @@ bool PortManager::Restore() {
m_portList.erase(el_it);
}
else {
ERROR_LOG(SCENET, "PortManager::Restore - DeletePortMapping failed (error: %d)", r);
return false; // Might be better not to exit here, but exiting a loop will avoid long timeouts in the case the router is no longer reachable
ERROR_LOG(SCENET, "PortManager::Restore - DeletePortMapping failed (error: %i)", r);
if (r == UPNPCOMMAND_HTTP_ERROR)
return false; // Might be better not to exit here, but exiting a loop will avoid long timeouts in the case the router is no longer reachable
}
}
// Add the original owner back
Expand All @@ -269,8 +281,9 @@ bool PortManager::Restore() {
it->taken = false;
}
else {
ERROR_LOG(SCENET, "PortManager::Restore - AddPortMapping failed (error: %d)", r);
return false; // Might be better not to exit here, but exiting a loop will avoid long timeouts in the case the router is no longer reachable
ERROR_LOG(SCENET, "PortManager::Restore - AddPortMapping failed (error: %i)", r);
if (r == UPNPCOMMAND_HTTP_ERROR)
return false; // Might be better not to exit here, but exiting a loop will avoid long timeouts in the case the router is no longer reachable
}
}
}
Expand Down Expand Up @@ -314,8 +327,9 @@ bool PortManager::Clear() {
int r2 = UPNP_DeletePortMapping(urls->controlURL, datas->first.servicetype, extPort, protocol, rHost);
if (r2 != 0)
{
ERROR_LOG(SCENET, "PortManager::Clear - DeletePortMapping(%s, %s) failed (error: %d)", extPort, protocol, r2);
return false;
ERROR_LOG(SCENET, "PortManager::Clear - DeletePortMapping(%s, %s) failed (error: %i)", extPort, protocol, r2);
if (r2 == UPNPCOMMAND_HTTP_ERROR)
return false;
}
else {
i--;
Expand Down
1 change: 1 addition & 0 deletions Core/Util/PortManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class PortManager {
int m_LocalPort = UPNP_LOCAL_PORT_ANY;
std::string m_lanip;
std::string m_defaultDesc;
std::string m_leaseDuration = "43200"; // range(0-604800) in seconds (0 = Indefinite/permanent). Some routers doesn't support non-zero value
std::deque<std::pair<std::string, std::string>> m_portList;
std::deque<PortMap> m_otherPortList;
};
Expand Down

0 comments on commit 6bb15b8

Please sign in to comment.