UPNP add_port_mapping() returns error 26 even though discover() returns 0 beforehand

[Anotic]

Godot version

4.0 beta 9

System information

Windows 10, Ryzen 5 3600, RTX 2060. Godot 4 beta 9 Forward+

Issue description

When I use the code on the latest version of the Godot Docs the upnp.discover() function returns 0 saying that it was successfull. But upnp.add_port_mapping() returns 26 stating that I need to use upnp.discover() first. Even though I already have and it was successful.

Steps to reproduce

I simply ran this code from the Godot Latest Docs and tried to find what was going wrong by just printing it.

func _upnp_setup(port):
    var upnp = UPNP.new()
    var err = upnp.discover()

    print(err) #Prints 0 for me showing it was a success

    if err != OK:
        push_error(str(err))
        emit_signal("upnp_completed", err)
        print(err)
        return

    print(upnp.get_gateway()) #Prints 26 saying I need to run discover first

    if upnp.get_gateway() and upnp.get_gateway().is_valid_gateway():
        upnp.add_port_mapping(port, port, ProjectSettings.get_setting("application/config/name"), "UDP")
        upnp.add_port_mapping(port, port, ProjectSettings.get_setting("application/config/name"), "TCP")
        emit_signal("upnp_completed", OK)
        print(OK)
        upnp.delete_port_mapping(port)

Minimal reproduction project

N/A

[Calinou]

Can you reproduce this on 3.5.1?

[Anotic]

using the same code in 3.5.1 retuns 0 for upnp.discover() and returns [Object:null] for upnp.get_gateway()

[hcoura]

I took a look on this one, I don't think this is a regression, nor a bug necessarily for that matter.

upnp.discover() may return 0 (success) if it found any device even if their igd_status is invalid thus, upnp.get_gateway will return null and upnp.add_port_mapping will return 26 and display this message:

No gateway available. You may need to call [method discover] first, or discovery didn't detect any valid IGDs (InternetGatewayDevices).

suggesting one may need run upnp.discover()...

There is no significant change between 3.5.1 and 4.x on the related source files and using the same code in 3.5.1 retuns 0 for upnp.discover() and returns [Object:null] for upnp.get_gateway() pretty much says the behaviour is the same there

@Citonaa I think this line in your steps to reproduce is not correct print(upnp.get_gateway()) #Prints 26 saying I need to run discover first this function should return either a device or null in either 4.x or 3.5.1

---

I guess the only question is if this situation should be made more clear and if discover should return success in the case of devices found that fail is_valid_gateway() check.

[Anotic]

Tried again on latest Godot 4 and print(upnp.get_gateway()) does return null. Not sure what was happening before as I was getting 0 and then 26 printed one after the other when I made the issue and assumed that print(upnp.get_gateway()) was what printed it.

[ywmaa]

I have this problem too in Godot 4.0.1.

[ywmaa]

I have one device that I can get using :

upnp.get_device(0)

if I execute add_port_mapping on it I get error code 16, which says : UPNP_RESULT_INVALID_GATEWAY = 16 Invalid gateway.

very strange, because if I get the device description using this command

print(upnp.get_device(0).description_url)

I get this line : http://192.168.1.1:37215/upnpdev.xml

which should mean that this is the router, because it has the IP 192.168.1.1

[mhilbrunner]

@ywmaa Could you post your router model (and possibly firmware version)?

Note that just because the router is found when looking for UPnP devices, that doesn't mean its a valid UPnP internet gateway device, or that it's compatible with MiniUPnPc, which is what Godot uses under the hood.

Could you also verify adding ports via UPnP is enabled in the router settings?

[ywmaa]

@mhilbrunner yeah I am very sure that upnp option is enabled in the router, and just in case I disabled firewall in the router.

the router is huawei DG8045

I don't know how to get firmware version. is it the same as "Software Version" ?

[mhilbrunner]

Software Version sounds right, yeah.

There seem to be various reports around the web of people having UPnP trouble with Huawei routers in general and Huawei routers and MiniUPnP specifically.

We even got a report for Godot in 2019 here which seems similar, so maybe this is a specific problem with (some?) Huawei routers and/or MiniUPnPc.

As we don't maintain MiniUPnP, this problem seems unlikely to be fixable on our side.

[ywmaa]

This is solved for me by manually port forwarding in the router, now upnp works, and gets the router, also can fetch the global IP, but I think this makes it useless for my router, after all I had to port forward myself, but my router is from huawei which can be the cause of the problem.

[Anotic]

This is solved for me by manually port forwarding in the router, now upnp works, and gets the router, also can fetch the global IP, but I think this makes it useless for my router, after all I had to port forward myself, but my router is from huawei which can be the cause of the problem.

i can manually port suff but the whole point of upnp is a semi automated way of doing it :/

[Calinou]

In general, UPnP in 2023 is a lot less worthwhile than it was back in 2010-2012. The fact that it's disabled by default in most routers now and the rise of mobile connections (and home CGNAT connections) make UPnP rarely usable, since there is simply no way to directly reach an user's IP address in a lot of cases.

Something like https://github.com/godotengine/godot-proposals/issues/434 should be implemented to replace it entirely at some point.

[Anotic]

using a NAT server in godot would be great

[chrom007]

I have same problem with TPLink Archer C6 v2 :cry:

[FeralBytes]

May I request that we update miniUPnP to the latest version 2.2.4. I am also having issues with a TP Link Archer AX20 router, but it works fine in Transmission and seems that there was an issue fixed late last year for TP Link routers that may fix some of the UPnP issues. I also see that we are currently using version 2.2.3 last updated on 19NOV2021. Thank you all for the amazing game engine that is Godot.

Update: I used pip3 to install the latest version of miniupnpc bindings for Python3. I then ran this code:

import miniupnpc
import socket
try:
    from http.server import BaseHTTPRequestHandler, HTTPServer
except ImportError:
    from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer

# function definition
def list_redirections():
    i = 0
    while True:
        p = u.getgenericportmapping(i)
        if p==None:
            break
        print(i, p)
        i = i + 1

#define the handler class for HTTP connections
class handler_class(BaseHTTPRequestHandler):
    def do_GET(self):
        self.send_response(200)
        self.end_headers()
        self.wfile.write(b"OK MON GARS")

# create the object
u = miniupnpc.UPnP()
#print 'inital(default) values :'
#print ' discoverdelay', u.discoverdelay
#print ' lanaddr', u.lanaddr
#print ' multicastif', u.multicastif
#print ' minissdpdsocket', u.minissdpdsocket
u.discoverdelay = 200;

try:
    print('Discovering... delay=%ums' % u.discoverdelay)
    ndevices = u.discover()
    print(ndevices, 'device(s) detected')

    # select an igd
    u.selectigd()
    # display information about the IGD and the internet connection
    print('local ip address :', u.lanaddr)
    externalipaddress = u.externalipaddress()
    print('external ip address :', externalipaddress)
    #print(u.statusinfo(), u.connectiontype())
    #print(u.statusinfo())
    print(u.connectiontype())

    #instanciate a HTTPd object. The port is assigned by the system.
    httpd = HTTPServer((u.lanaddr, 0), handler_class)
    eport = httpd.server_port

    # find a free port for the redirection
    r = u.getspecificportmapping(eport, 'TCP')
    while r != None and eport < 65536:
        eport = eport + 1
        r = u.getspecificportmapping(eport, 'TCP')

    print('trying to redirect %s port %u TCP => %s port %u TCP' % (externalipaddress, eport, u.lanaddr, httpd.server_port))

    b = u.addportmapping(eport, 'TCP', u.lanaddr, httpd.server_port,
                        'UPnP IGD Tester port %u' % eport, '')
    if b:
        print('Success. Now waiting for some HTTP request on http://%s:%u' % (externalipaddress ,eport))
        try:
            httpd.handle_request()
            httpd.server_close()
        except KeyboardInterrupt as details:
            print("CTRL-C exception!", details)
        b = u.deleteportmapping(eport, 'TCP')
        if b:
            print('Successfully deleted port mapping')
        else:
            print('Failed to remove port mapping')
    else:
        print('Failed')

    httpd.server_close()

except Exception as e:
    print('Exception :', e)

It worked and I could connect to the web server on a external port on my router to get the status of "OK MON GARS" as the example code produces. Additionally I could see it as an active UPnP Client in my router.

IMPORTANT NOTE: If I ran the code with this line:

print(u.statusinfo())

I would get an exception that stated "Exception : Invalid Action", perhaps this is what is going wrong in the Godot code on these routers too. I hope this all helps. Thank you all again.

[akien-mga]

May I request that we update miniUPnP to the latest version 2.2.4. I am also having issues with a TP Link Archer AX20 router, but it works fine in Transmission and seems that there was an issue fixed late last year for TP Link routers that may fix some of the UPnP issues. I also see that we are currently using version 2.2.3 last updated on 19NOV2021.

We already provide miniupnpc 2.2.4, since #69393.

[FeralBytes]

@akien-mga thanks for pointing that out, sorry I missed it. Is it possible then that we are running into the same statusinfo() exception that I was encountering in Python, and perhaps that is why we are not able to use the IGD correctly in Godot?

[mhilbrunner]

Thanks for that testing, I'll investigate it (althought thats a bit hard without a TP device for testing) - at least its a direction to look into.

[FeralBytes]

@mhilbrunner if there is anything I can do to help test please let me know. I have experience in the past with re-compiling the engine but it has been a long time since I did that. So I think a valid test would be to perform basic discovery then without doing anything additional we select the first IGD, without verifying it, and see if the TP devices start to work. I do think this is probably not up to spec nor standard, but if it allows more devices to work with UPnP in Godot then that would still be a good thing. I think in documentation we would still keep everything normal, but put a note that a second attempt could be tried without any form of validation on the IGD or at least to avoid the statusinfo call when attempting to port forward on some IGDs specifically we can call out TP Link for their issue.

[FeralBytes]

@mhilbrunner is there any thing that I can do to test a fix, or do you know what code I could adjust to test my theory about the issue of calling statusinfo()?

[mhilbrunner]

@FeralBytes The code for UPnP lives in upnp.cpp and upnp_device.cpp.

If I understand it correctly, the Python upnpc.statusinfo() function is a wrapper around this C function in the MiniUPnP client, which is basically a call to UPNP_GetStatusInfo plus some setup/teardown. If I'm not missing anything, we do not seem to call UPNP_GetStatusInfo in any of our code.

Now, what we do do is after finding devices in UPNP.discover(), we add them to our device list and then call parse_igd() on their description URL (which in turn calls load_description(), which is just a small wrapper using miniwget to download the XML from that URL).

If I were to hazard a guess, this fails for some reason (maybe for the same reason that statusinfo() does, as it probably does something similar) - maybe the router doesn't return the XML as expected, or does not correctly identify itself as an IGD (InternetGatewayDevice), or something else is going on during that HTTP request.

Now as said, this is kinda hard to debug without having such a router around to actually look at the traffic. What could be potentially interesting is if you could insert a debug print statement in load_description and log the status code, the result body/XML and such and see what is actually received or if there are errors during UPNP.discover(). Maybe you should also try to set the device_filter parameter of UPNP.discover() to something other than InternetGatewayDevice (maybe empty string).

If you can find out your router's description URL for UPNP, building with this PR and manually setting the description URL to see if that works could also be interesting to skip the whole discovery process and pinpoint where the issue occurs.

If you are new to modifying the Godot source and building it, the documentation to get setup and started is around here, but I'm also happy to help you debug this if you want to join contributors chat and ping me on the #networking channel to talk! (Responses may take some time though :))

I could try to see if I can modify the code to do a test build that allows you to just call add_port_mapping() on any device without checking if its a gateway, to see if that helps - but then I'm not sure that would be a good fix for us to ship, even if it works.

[drautb]

FWIW, I also have a TP-link router and have faced similar issues. I tracked it down to the fact that TP-link routers don't correctly implement the GetStatusInfo action (bug report), despite the router-provided XML declaring that it is supported. When miniupnp tries to invoke that action, it receives an HTTP 500 response, and treats the device as disconnected.

Edit: So after parsing the XML, Godot calls UPNP_GetValidIGD, which eventually calls UPNPIGD_IsConnected. This invokes the GetStatusInfo action, which responds with HTTP 500 on TP-link devices.

miniupnpc provides the -i argument to work around issues like this. (Example) Perhaps Godot could implement something similar to accommodate some non-compliant devices.