Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading documentation related to listening to "all interfaces" #60

Closed
timcoote opened this issue May 5, 2020 · 7 comments
Closed

Comments

@timcoote
Copy link

timcoote commented May 5, 2020

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
Copy link
Author

timcoote commented May 6, 2020

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.

@pekkaklarck
Copy link
Member

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?

@timcoote
Copy link
Author

timcoote commented May 6, 2020

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.

@pekkaklarck
Copy link
Member

pekkaklarck commented May 6, 2020

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

@timcoote
Copy link
Author

timcoote commented May 6, 2020

ok. I'll have a go.

@timcoote
Copy link
Author

timcoote commented May 7, 2020

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.

@pekkaklarck pekkaklarck added this to the v1.1.1 milestone Dec 11, 2022
@pekkaklarck
Copy link
Member

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 pekkaklarck changed the title Documentation error for all interfaces Misleading documentation related to listening to "all interfaces" Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants