Add basic IPv6 support #1
Conversation
|
I can't understand the diff because they are mixed with formatting changes. Could you separate the two? Actually, upstream probably won't want to reformat all their code so it's probably better to not touch the rest of their code and try to use the same style. |
LolloneS
force-pushed
the
add-ipv6-support
branch
6 times, most recently
from
fe37e7c to
ff906e0
Compare
LolloneS
changed the title
robot-detect
Outdated
| if timeout is not None: | ||
| s.settimeout(timeout) | ||
| return s | ||
| s = socket.socket(socket.AF_INET6) |
There was a problem hiding this comment.
Why is a different kind of socket required?
There was a problem hiding this comment.
AF_INET seems to be IPv4-only.
From https://docs.python.org/3/library/socket.html#socket-families, on AF_INET:
This behavior is not compatible with IPv6, therefore, you may want to avoid these if you intend to support IPv6 with your Python programs.
There was a problem hiding this comment.
I see. But I'm not sure we need that here. You could pass the parameters that you compute the very first time with getaddrinfo or create_connection/getpeername/getsockname to that function, and then pass them further to socket.socket and socket.connect. This way you don't need this extra if-then-else.
There was a problem hiding this comment.
Unfortunately, in the oracle function I need to create a socket but not a connection, in order to allow the fastopen if available. However I think I have managed to solve this issue in a polished way (line 193). In this case it is simply not possible to bypass the choice of IP type, since the sendto function used in case fastopen is available should not work on an open connection.
LolloneS
force-pushed
the
add-ipv6-support
branch
2 times, most recently
from
b6c9dee to
20b2750
Compare
LolloneS
force-pushed
the
add-ipv6-support
branch
2 times, most recently
from
b7cdb1e to
e368de4
Compare
robot-detect
Outdated
| ip = socket.gethostbyname(args.host) | ||
| connection = socket.create_connection(server, timeout=timeout) | ||
| ip = connection.getpeername()[0] | ||
| ip_family = socket.AF_INET if type(ipaddress.ip_address(ip)) == ipaddress.IPv4Address else socket.AF_INET6 |
There was a problem hiding this comment.
Probably easier to check ip_address.version == 4
LolloneS
force-pushed
the
add-ipv6-support
branch
from
e368de4 to
de033b1
Compare
Add basic IPv6 support.
The ROBOT test works fine for standard cases (e.g. it performs an IPv6 test when provided with an IPv6 address), but fails in corner cases (e.g. IPv6 address given, but the service only accepts IPv4 connections).
The default IP version is IPv4: this means that when checking example.com, if it is possible to resolve it to an IPv4 address, that will be the used one. In other words, in order to use IPv6 you can either pass an IPv6 address as host directly, or have a service that is only exposed on IPv6 and therefore does only resolve to it. I never seen a case such as the latter before, so I strongly suggest just passing the IPv6 address as host if you want to have it checked. Otherwise, the default behavior will be the standard IPv4 one.
@bbc2