Skip to content

Commit

Permalink
Trim down getaddrinfo edge cases in the highlevel interface
Browse files Browse the repository at this point in the history
No more port=None or port="" or port=bytestring -- they're not useful
and they can have weird/non-portable behavior.

Also don't accept service names (port="http") -- it would work, but
they aren't necessarily portable, I can't find evidence that anyone
uses them, and they violate TOOWTDI. I'd rather not support things I
wouldn't recommend using.

Also don't allow host=None in open_tcp_stream -- this is redundant
with the loopback address. I guess this *could* be useful in some
weird case, but we can always add it back later if someone wants it.
  • Loading branch information
njsmith committed Aug 15, 2017
1 parent 59365a2 commit 80905c8
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 18 deletions.
40 changes: 26 additions & 14 deletions trio/_highlevel_open_tcp_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,32 @@ async def open_tcp_listeners(port, *, host=None, backlog=None):
Args:
port (int): The port to listen on. If you pass 0, the kernel will
automatically pick an arbitrary open port. But be careful: if you
use ``port=0`` when binding to multiple IP address, then each IP
address will be assigned a different port. An example of when this
happens is when ``host=None``, which means to bind to both the IPv4
wildcard address (``0.0.0.0``) and also the IPv6 wildcard address
(``::``).
port (int): The port to listen on.
host (str or None): The local interface to bind to. This is passed to
:func:`~socket.getaddrinfo` with the ``AI_PASSIVE`` flag set.
If you use 0 as your port, then the kernel will automatically pick
an arbitrary open port. But be careful: if you use this feature when
binding to multiple IP addresses, then each IP address will get its
own random port, and the returned listeners will probably be
listening on different ports. In particular, this will happen if you
use ``host=None`` – which is the default – because in this case
:func:`open_tcp_listeners` will bind to both the IPv4 wildcard
address (``0.0.0.0``) and also the IPv6 wildcard address (``::``).
host (str, bytes-like, or None): The local interface to bind to. This is
passed to :func:`~socket.getaddrinfo` with the ``AI_PASSIVE`` flag
set.
If you want to bind to bind to the wildcard address on both IPv4 and
IPv6, in order to accept connections on all available interfaces,
then pass ``None``. This is the default.
If you have a specific interface you want to bind to, pass its IP
address or hostname here. If a hostname resolves to multiple IP
addresses, this function will bind one listener to each of them.
If you want to bind to all available interfaces (the wildcard
address) for both IPv4 and IPv6, pass ``None`` (the default).
addresses, this function will open one listener on each of them.
If you want to use only IPv4, or only IPv6, but want to accept on
all interfaces, pass the family-specific wildcard address:
``"0.0.0.0"`` or ``"::"``.
``"0.0.0.0"`` for IPv4-only and ``"::"`` for IPv6-only.
backlog (int or None): The listen backlog to use. If you leave this as
``None`` then Trio will pick a good default.
Expand All @@ -77,6 +82,13 @@ async def open_tcp_listeners(port, *, host=None, backlog=None):
list of :class:`SocketListener`
"""
# getaddrinfo sometimes allows port=None, sometimes not (depending on
# whether host=None). And on some systems it treats "" as 0, others it
# doesn't:
# http://klickverbot.at/blog/2012/01/getaddrinfo-edge-case-behavior-on-windows-linux-and-osx/
if not isinstance(port, int):
raise TypeError("port must be an int or str, not {!r}".format(port))

backlog = _compute_backlog(backlog)

addresses = await tsocket.getaddrinfo(
Expand Down
10 changes: 9 additions & 1 deletion trio/_highlevel_open_tcp_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ async def open_tcp_stream(
<https://tools.ietf.org/html/rfc6555>`__ for more details.
Args:
host (bytes or str): The host to connect to. Can be an IPv4 address,
host (str or bytes): The host to connect to. Can be an IPv4 address,
IPv6 address, or a hostname.
port (int): The port to connect to.
happy_eyeballs_delay (float): How many seconds to wait for each
Expand All @@ -216,6 +216,14 @@ async def open_tcp_stream(
open_ssl_over_tcp_stream
"""
# To keep our public API surface smaller, rule out some cases that
# getaddrinfo will accept in some circumstances, but that act weird or
# have non-portable behavior or are just plain not useful.
# No type check on host though b/c we want to allow bytes-likes.
if host is None:
raise ValueError("host cannot be None")
if not isinstance(port, int):
raise TypeError("port must be int, not {!r}".format(port))

if happy_eyeballs_delay is None:
happy_eyeballs_delay = DEFAULT_DELAY
Expand Down
7 changes: 4 additions & 3 deletions trio/_highlevel_ssl_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,12 @@ async def open_ssl_over_tcp_listeners(
"""Start listening for SSL/TLS-encrypted TCP connections to the given port.
Args:
port (int): The port to listen on. See :func:`open_tcp_listeners`.
port (int or str): The port to listen on. See
:func:`open_tcp_listeners`.
ssl_context (~ssl.SSLContext): The SSL context to use for all incoming
connections.
host (str or None): The address to bind to; use ``None`` to bind to the
wildcard address. See :func:`open_tcp_listeners`.
host (str, bytes, or None): The address to bind to; use ``None`` to bind
to the wildcard address. See :func:`open_tcp_listeners`.
https_compatible (bool): See :class:`~trio.ssl.SSLStream` for details.
backlog (int or None): See :class:`~trio.ssl.SSLStream` for details.
Expand Down
10 changes: 10 additions & 0 deletions trio/tests/test_highlevel_open_tcp_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,3 +210,13 @@ async def test_open_tcp_listeners_multiple_host_cleanup_on_error():
assert len(fsf.sockets) == 3
for sock in fsf.sockets:
assert sock.closed


async def test_open_tcp_listeners_port_checking():
for host in ["127.0.0.1", None]:
with pytest.raises(TypeError):
await open_tcp_listeners(None, host=host)
with pytest.raises(TypeError):
await open_tcp_listeners(b"80", host=host)
with pytest.raises(TypeError):
await open_tcp_listeners("http", host=host)
8 changes: 8 additions & 0 deletions trio/tests/test_highlevel_open_tcp_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ async def test_open_tcp_stream_real_socket_smoketest():
await client_stream.aclose()
server_sock.close()

listen_sock.close()

async def test_open_tcp_stream_input_validation():
with pytest.raises(ValueError):
await open_tcp_stream(None, 80)
with pytest.raises(TypeError):
await open_tcp_stream("127.0.0.1", b"80")


# Now, thorough tests using fake sockets

Expand Down

0 comments on commit 80905c8

Please sign in to comment.