From 50f0f51604b5e8acc8db23b26c98a76c3688a705 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Mon, 15 Mar 2021 14:38:13 +0100 Subject: [PATCH] [Net] Fix miniupnpc when no interface is specified This is a tricky one, it used to work, but it was wrong, because in such a scenario instead of passing NULL as required by the API, it would pass a buffer containing the `\0` terminator. This stopped working on a specific miniupnpc version, when they fixed some network endianess issue on Windows, to which we made a workaround, which in turn would probably result in failures when the interface is specified. This commit address the issue properly, by checking the specified interface string size, and correctly passing NULL instead of the empty string when necessary. Also reverts the commit that introduced the bogus workaround: 388adac94714d7bbae212d4df9a16221d697d6f8 One of those PR when the explanation is much longer then code changes :). --- modules/upnp/upnp.cpp | 6 ++++-- thirdparty/README.md | 1 - thirdparty/miniupnpc/miniupnpc/minissdpc.c | 4 ++++ thirdparty/miniupnpc/windows_fix.diff | 16 ---------------- 4 files changed, 8 insertions(+), 19 deletions(-) delete mode 100644 thirdparty/miniupnpc/windows_fix.diff diff --git a/modules/upnp/upnp.cpp b/modules/upnp/upnp.cpp index 618b42e9e25..d110bbcf19a 100644 --- a/modules/upnp/upnp.cpp +++ b/modules/upnp/upnp.cpp @@ -53,10 +53,12 @@ int UPNP::discover(int timeout, int ttl, const String &device_filter) { int error = 0; struct UPNPDev *devlist; + CharString cs = discover_multicast_if.utf8(); + const char *m_if = cs.length() ? cs.get_data() : NULL; if (is_common_device(device_filter)) { - devlist = upnpDiscover(timeout, discover_multicast_if.utf8().get_data(), NULL, discover_local_port, discover_ipv6, ttl, &error); + devlist = upnpDiscover(timeout, m_if, NULL, discover_local_port, discover_ipv6, ttl, &error); } else { - devlist = upnpDiscoverAll(timeout, discover_multicast_if.utf8().get_data(), NULL, discover_local_port, discover_ipv6, ttl, &error); + devlist = upnpDiscoverAll(timeout, m_if, NULL, discover_local_port, discover_ipv6, ttl, &error); } if (error != UPNPDISCOVER_SUCCESS) { diff --git a/thirdparty/README.md b/thirdparty/README.md index 6f5b186aed9..9bc7588b464 100644 --- a/thirdparty/README.md +++ b/thirdparty/README.md @@ -292,7 +292,6 @@ Files extracted from upstream source: - All `*.c` and `*.h` files from `miniupnpc` to `thirdparty/miniupnpc/miniupnpc` - Remove `test*`, `minihttptestserver.c` and `wingenminiupnpcstrings.c` -The patch `windows_fix.diff` is applied to `minissdpc.c` to fix an upstream issue. The only modified file is miniupnpcstrings.h, which was created for Godot (it is usually autogenerated by cmake). diff --git a/thirdparty/miniupnpc/miniupnpc/minissdpc.c b/thirdparty/miniupnpc/miniupnpc/minissdpc.c index 36244dedecc..a3b1283da59 100644 --- a/thirdparty/miniupnpc/miniupnpc/minissdpc.c +++ b/thirdparty/miniupnpc/miniupnpc/minissdpc.c @@ -683,7 +683,11 @@ ssdpDiscoverDevices(const char * const deviceTypes[], #endif } else { struct in_addr mc_if; +#if defined(_WIN32) && (_WIN32_WINNT >= _WIN32_WINNT_VISTA) + InetPtonA(AF_INET, multicastif, &mc_if); +#else mc_if.s_addr = inet_addr(multicastif); /* ex: 192.168.x.x */ +#endif if(mc_if.s_addr != INADDR_NONE) { ((struct sockaddr_in *)&sockudp_r)->sin_addr.s_addr = mc_if.s_addr; diff --git a/thirdparty/miniupnpc/windows_fix.diff b/thirdparty/miniupnpc/windows_fix.diff deleted file mode 100644 index 460b596888e..00000000000 --- a/thirdparty/miniupnpc/windows_fix.diff +++ /dev/null @@ -1,16 +0,0 @@ -diff --git a/thirdparty/miniupnpc/miniupnpc/minissdpc.c b/thirdparty/miniupnpc/miniupnpc/minissdpc.c -index 29f8110155..ea9af02e1f 100644 ---- a/thirdparty/miniupnpc/miniupnpc/minissdpc.c -+++ b/thirdparty/miniupnpc/miniupnpc/minissdpc.c -@@ -683,11 +683,7 @@ ssdpDiscoverDevices(const char * const deviceTypes[], - #endif - } else { - struct in_addr mc_if; --#if defined(_WIN32) && (_WIN32_WINNT >= _WIN32_WINNT_VISTA) -- InetPtonA(AF_INET, multicastif, &mc_if); --#else - mc_if.s_addr = inet_addr(multicastif); /* ex: 192.168.x.x */ --#endif - if(mc_if.s_addr != INADDR_NONE) - { - ((struct sockaddr_in *)&sockudp_r)->sin_addr.s_addr = mc_if.s_addr;