PythonRemoteServer icon indicating copy to clipboard operation
PythonRemoteServer copied to clipboard

Documentation error for all interfaces

Open timcoote opened this issue 5 years ago • 6 comments

README states:

"Address to listen. Use '0.0.0.0' to listen to all available interfaces."

However, this value for "host" only listens on IPv4 addresses. A minimal program, such as this: """ from robotremoteserver import RobotRemoteServer as rs

class DummyClass (): def init (self): pass

rs (DummyClass (), host = "0.0.0.0") """

only listens on 0.0.0.0, which is an ipv4 address, rather than ::, which would include ipv4 and ipv6. The listening ports can be spotted, on linux, with netstat -anp |grep python, in a separate terminal, which produces something like this: """ tcp 0 0 0.0.0.0:8270 0.0.0.0:* LISTEN 10921/python3
"""

timcoote avatar May 05 '20 21:05 timcoote

It is possible to get RobotRemoteServer to listen on "::". It requires modifying the class variable of socketserver.TCPServer.address_family before the class is loaded by robotremoteserver, eg: """ import socketserver import socket

socketserver.TCPServer.address_family = socket.AF_INET6

from robotremoteserver import RobotRemoteServer as rs

class DummyClass (): def init (self): pass

#rs (DummyClass (), host = "0.0.0.0") server = rs (DummyClass (), host = "::", serve=False) server.serve (log=False) """

Note that logging needs to be turned off, though as the method RobotRemoteServer._log() assumes that the addr of the server is a tuple of size 2, whereas for an IPv6 address it has a size of 4.

timcoote avatar May 06 '20 12:05 timcoote

It's true that the documentation is misleading and 0.0.0.0 covers only IPv4. Fixing that is easy but enhancing IPv6 support would be great as well. Anything that could be done in generic manner? Interested to provide a PR?

pekkaklarck avatar May 06 '20 12:05 pekkaklarck

I don't know how to create a PR, nor what regression tests I'd need. I think that the code example is sufficient for a more general ipv6 support. Additionally, this change seems to be sufficient to sort out _log in robotremoteserver.py (based on minimal testing), ie just use the first two elements of the server_address tuple:

diff robotremoteserver.py*
151c151
<             address = '%s:%s' % self.server_address [:2]
---
>             address = '%s:%s' % self.server_address

There could well be other assumptions of the size of this variable.

If you can point me at where you'd like the doc amending (to include the example) and how to do regression testing, I'll have a crack at a PR.

timcoote avatar May 06 '20 13:05 timcoote

GitHub has pretty good instructions related to PRs: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests

In this case there should be two PRs, one for the doc updated and another for IPv6 support. The latter should actually get its own issue as well. Probably better to start from the doc change as it's easier and requires no tests. The doc is here: https://github.com/robotframework/PythonRemoteServer/blob/master/src/robotremoteserver.py#L57

pekkaklarck avatar May 06 '20 13:05 pekkaklarck

ok. I'll have a go.

timcoote avatar May 06 '20 13:05 timcoote

I found that I also needed to edit README.rst - both included in the PR. Let's see if I got the process correct. I created a new issue for IPv6 support.

timcoote avatar May 07 '20 08:05 timcoote

I'm looking at this project after a long break to fix some critical Python 3.10 and 3.11 compatibility issues. I'll fix the documentation related to "all interfaces" at the same time but I unfortunately won't have time to look the actual IPv6 support PR. Hopefully Robot Framework Foundation has more resources develop the remote server further next year.

pekkaklarck avatar Dec 11 '22 15:12 pekkaklarck