-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Issue #610 -- Tests fallback to IPv4 if IPv6 fails #611
Conversation
@ddriddle Any ideas what conditions are required for this to happen? (ie. has_ipv6 false-positive) @TomasTomecek Any thoughts on this? (You wrote the ipv6 detection code in our test suite before) |
I am not sure why I am receiving a false positive for socket.has_ipv6. I am running on RHEL6. I receive this error with Python 2.6.6 and Python 2.7.5 on the same system. |
@ddriddle Does your system support IPv6 but simply not have the module loaded? |
I talked with my sysadmin and the system supports IPv6. The module is loaded. The problem is that localhost is an unknown host because the IPv6 address for localhost is not listed in /etc/hosts. I don't think it is a good idea to use localhost for IPv6 since even my stock Debian box does not support localhost as an alias for ::1. If I hardcode ::1 in dummyserver/server.py then the server will start but I receive a bunch of errors from failed unit tests here is the first one.
I will keep working on this to see if I can figure anything else out. Please tell me what you think. |
Why is that? Are you able to supply a different alias for your ::1? Maybe we could override the default hostname with an environment variable for the test suite if there are other people in your situation.
I believe that's because our code is assuming that you're passing in a hostname, rather than an IP, so it attempts to resolve it. @Lukasa @sigmavirus24 I think this is a bug? HTTP should work with an IP as a hostname, no? |
I don't think so @shazow. That's because the server is only starting up for IPv6 and if the tests try to use IPv4, then you'll get the gaierror that the address family we're trying to open the socket with is not supported. You would get the same error if you bound using only IPv6 and tried to open another socket connecting to it using IPv4. If the server doesn't support IPv6 or IPv4 and we try to connect using the one it doesn't support, this seems like something that we would expect to see.
I just spun up a Debian VM and it had 127.0.0.1 and ::1 both in |
@sigmavirus24 is correct, |
You can even edit it without restarting and it should work. The question should be, why does your sysadmin remove it?Sent from my Android device with K-9 Mail. Please excuse my brevity. |
@sigmavirus24 Does the Debian VM have an IPv6 address assigned? My Debian system does not. When I look at my /etc/hosts it has a line with
But on the RHEL6 system I receive
I ran the urllib3 unit tests on my Debian system and I received the same errors as on the RHEL6 system:
Note that on the Debian system
even though ifconfig clearly shows I have no IPv6 address:
I went into my
@sigmavirus24 Can you retry your VM test but make sure the Debian system does not have an IPv6 address? Either way it is clear that |
As to the reliability of socket.has_ipv6, the best thing to do might be to try to create an IPv6 socket and see if it fails. |
I looked at the documentation for
This is a bit cryptic but I would assume by that it means that IPv6 is supported by the system and the cPython compiler but looking at the C code it appears that is not the case. Take a look at ( #ifdef ENABLE_IPV6
has_ipv6 = Py_True;
#else
has_ipv6 = Py_False;
#endif
Py_INCREF(has_ipv6);
PyModule_AddObject(m, "has_ipv6", has_ipv6); And the headers contain the following relevant code ( /* VC6 is shipped with old platform headers, and does not have MSTcpIP.h
* Separate SDKs have all the functions we want, but older ones don't have
* any version information.
* I use SIO_GET_MULTICAST_FILTER to detect a decent SDK.
*/
# ifdef SIO_GET_MULTICAST_FILTER
# include <MSTcpIP.h> /* for SIO_RCVALL */
# define HAVE_ADDRINFO
# define HAVE_SOCKADDR_STORAGE
# define HAVE_GETADDRINFO
# define HAVE_GETNAMEINFO
# define ENABLE_IPV6
# else Looking at the headers and the C code it looks to me that def _is_ipv6_enabled():
"""Check whether IPv6 is enabled on this host."""
if socket.has_ipv6:
sock = None
try:
sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
sock.bind((HOSTv6, 0))
return True
except OSError:
pass
finally:
if sock:
sock.close()
return False
IPV6_ENABLED = _is_ipv6_enabled() I also found the following bit of information in cPython's docs (
Looking at the ticket it reads to me that |
@Lukasa completely agree. That is what this patches does. |
On systems with IPv6 disabled all IPv6 unit tests will now be skipped, instead of throwing an exception. On some IPv6 enabled systems with badly configured DNS, where the IPv6 loopback address ::1 is not assigned to localhost, the unit tests will fallback to using IPv4 for tests that bind to localhost instead of throwing an socket.gaierror exception. IPv6 tests that bind directly to ::1 will continue to be run.
I made changes to the patch and added comments now that I better understand the problem. When you have a chance please review it and tell me if it needs any further improvements. |
Tests fallback to IPv4 if IPv6 fails
Thanks @ddriddle! |
On some systems binding to an IPv6 address will fail with a socket.gaierror exception even though socket.has_ipv6 is set to True. Added code to catch these exceptions and fallback to using IPv4.