Fix adding port forwardings using UPnP

The UPnP implementation included a combination of two subtle bugs, which
resulted in failure to create port forwardings every time after the
first one.

When using UPnP, the internet gateway to create the port forwardings at
needs to discovered. As that takes a while, the its root descriptor URL
was supposed to be cached after successful discovery in the user config
in "network.upnprootdescurl". However, instead of caching the root
descriptor URL, the control URL got cached. That caused following
requests to the root descriptor URL to fail, as they ended up at the
control URL instead. As such requests might also fail when the network
topology changed, the code was supposed to fall back to discovering the
internet gateway again when the cached one didn't work. However, due to
the inner workings of miniupnpc the request using the cached root
descriptor URL didn't result in an error, so the new discovery was never
triggered. As the wrong value was persisted in the user config there was
also no way to get out of this situation again.

This commit fixes both of these bugs.

As far as I can tell these bugs existed since the introduction of the
caching of the root descriptor URL in 0ba25e9968, which means creating
port forwardings using UPnP has been broken since Alpha 15.

(cherry picked from commit 18d7746c84)
Signed-off-by: Itms <itms@wildfiregames.com>
This commit is contained in:
Dunedan 2025-02-24 08:23:07 +01:00 committed by Itms
parent 5012d1f1c0
commit a7694ef8b3
No known key found for this signature in database
GPG key ID: C7E52BD14CE14E09

View file

@ -242,7 +242,7 @@ void CNetServerWorker::SetupUPnP()
int ret = 0;
// Try a cached URL first
if (!rootDescURL.empty() && UPNP_GetIGDFromUrl(rootDescURL.c_str(), &urls, &data, internalIPAddress, sizeof(internalIPAddress)))
if (!rootDescURL.empty() && UPNP_GetIGDFromUrl(rootDescURL.c_str(), &urls, &data, internalIPAddress, sizeof(internalIPAddress)) && strlen(data.first.controlurl) != 0)
{
LOGMESSAGE("Net server: using cached IGD = %s", urls.controlURL);
ret = 1;
@ -339,9 +339,9 @@ void CNetServerWorker::SetupUPnP()
externalIPAddress, psPort, protocall, intClient, intPort, duration);
// Cache root descriptor URL to try to avoid discovery next time.
g_ConfigDB.SetValueString(CFG_USER, "network.upnprootdescurl", urls.controlURL);
g_ConfigDB.WriteValueToFile(CFG_USER, "network.upnprootdescurl", urls.controlURL);
LOGMESSAGE("Net server: cached UPnP root descriptor URL as %s", urls.controlURL);
g_ConfigDB.SetValueString(CFG_USER, "network.upnprootdescurl", urls.rootdescURL);
g_ConfigDB.WriteValueToFile(CFG_USER, "network.upnprootdescurl", urls.rootdescURL);
LOGMESSAGE("Net server: cached UPnP root descriptor URL as %s", urls.rootdescURL);
freeUPnP();
}