is_valid_ip: Do not raise exceptions on too-long input.

[alexmv]

Calling socket.getaddrinfo with a potential hostname attempts to
encode the "hostname" with the idna encoding. Per RFC, each part of
the hostname cannot be longer than 63 characters; and getaddrinfo
enforces this by raising a UnicodeError if the length is too long.

Catch these UnicodeErrors and return false for too-long names, rather
than raising an exception.

[bdarnell]

Thanks for the contribution! This looks good to me, but I'd like to have a comment in the code explaining why UnicodeError might be thrown.

I also think this may be a bug in the standard library - I don't think it should be trying to do the idna encoding if AI_NUMERICHOST is passed.

[ploxiln]

hmm yeah weird:

>>> socket.getaddrinfo("a" * 32, 0, socket.AF_UNSPEC, socket.SOCK_STREAM, 0, socket.AI_NUMERICHOST)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.7/socket.py", line 748, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -2] Name or service not known

>>> socket.getaddrinfo("a" * 70, 0, socket.AF_UNSPEC, socket.SOCK_STREAM, 0, socket.AI_NUMERICHOST)
Traceback (most recent call last):
  File "/usr/lib/python3.7/encodings/idna.py", line 167, in encode
    raise UnicodeError("label too long")
UnicodeError: label too long

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.7/socket.py", line 748, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
UnicodeError: encoding with 'idna' codec failed (UnicodeError: label too long)

[alexmv]

Yeah, I think I agree that this is really a python bug -- it looks like this was already filed as https://bugs.python.org/issue32958. Up to you if it's worth papering over in Tornado!

Happy to fix this up with a longer comment pointing to the core Python bug if you're interested in taking it.

[bdarnell]

I think this is a good change since the upstream fix will be rolled out slowly. As I said, it just needs a comment about what's going on, and perhaps a test case.

[alexmv]

I already had a simple test case (that too-long values don't raise, but return false) but I've added a comment, and fixed up the commit message to also point to the Python bug report.

Happy to add more tests, but AFAICT this gives coverage of the relevant codepath.

[bdarnell]

Thanks!