-
Notifications
You must be signed in to change notification settings - Fork 231
fix(websocket): handle IPv6 4-tuple in listener getsockname() #1317
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,26 @@ | |
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def _extract_host_port_from_sockname( | ||
| sock_name: object, | ||
| ) -> tuple[str, int] | None: | ||
| """ | ||
| Return ``(host, port)`` from a ``socket.getsockname()`` return value, or | ||
| ``None`` if the value is not recognised. | ||
|
|
||
| ``socket.getsockname()`` yields a 2-tuple ``(host, port)`` for IPv4 and a | ||
| 4-tuple ``(host, port, flowinfo, scopeid)`` for IPv6. Only the first two | ||
| fields are meaningful for multiaddr reconstruction, so we accept any | ||
| tuple of length ≥ 2 where the first two elements are a string and an int. | ||
| """ | ||
| if not isinstance(sock_name, tuple) or len(sock_name) < 2: | ||
| return None | ||
| host, port = sock_name[0], sock_name[1] | ||
| if not isinstance(host, str) or not isinstance(port, int): | ||
| return None | ||
| return host, port | ||
|
|
||
|
|
||
| @dataclass | ||
| class WebsocketListenerConfig: | ||
| """Configuration for WebSocket listener.""" | ||
|
|
@@ -297,9 +317,19 @@ async def _run_server() -> None: | |
| if hasattr(server_info, "socket"): | ||
| sock = server_info.socket | ||
| if hasattr(sock, "getsockname"): | ||
| sock_addr, sock_port = sock.getsockname() | ||
| actual_host = sock_addr | ||
| actual_port = sock_port | ||
| sock_name = sock.getsockname() | ||
| host_port = _extract_host_port_from_sockname(sock_name) | ||
| if host_port is not None: | ||
| actual_host, actual_port = host_port | ||
| else: | ||
| # Explicitly restore the requested host/port so the | ||
| # fallback mentioned in the warning is self-contained. | ||
| actual_host, actual_port = host, port | ||
| logger.warning( | ||
| "Unexpected getsockname() result %r; " | ||
| "falling back to requested host/port", | ||
| sock_name, | ||
| ) | ||
|
Comment on lines
+320
to
+332
|
||
| elif hasattr(server_info, "port"): | ||
| # If we can't get socket, at least get the port | ||
| actual_port = server_info.port | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| Fix ``WebsocketListener.listen()`` binding IPv6 addresses. ``socket.getsockname()`` | ||
| returns a 4-tuple ``(host, port, flowinfo, scopeid)`` for IPv6 sockets, which | ||
| previously failed to unpack into ``(sock_addr, sock_port)`` and surfaced as | ||
| ``OpenConnectionError("Failed to listen on ...")``. The listener now accepts | ||
| any 2-or-more-tuple with ``(host: str, port: int)`` in the first two slots and | ||
| falls back to the requested host/port on unknown shapes. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| """ | ||
| Unit tests for ``_extract_host_port_from_sockname``. | ||
|
|
||
| Historically ``WebsocketListener.listen`` unpacked ``sock.getsockname()`` | ||
| as a 2-tuple, which worked for IPv4 sockets but failed with | ||
| ``ValueError: too many values to unpack`` for IPv6 sockets (whose | ||
| ``getsockname()`` returns a 4-tuple ``(host, port, flowinfo, scopeid)``). | ||
| This regression test pins the supported shapes for the extractor so the | ||
| IPv6 path doesn't re-break. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from libp2p.transport.websocket.listener import _extract_host_port_from_sockname | ||
|
|
||
|
|
||
| def test_ipv4_two_tuple() -> None: | ||
| assert _extract_host_port_from_sockname(("127.0.0.1", 12345)) == ( | ||
| "127.0.0.1", | ||
| 12345, | ||
| ) | ||
|
|
||
|
|
||
| def test_ipv6_four_tuple() -> None: | ||
| # (host, port, flowinfo, scopeid) | ||
| assert _extract_host_port_from_sockname(("::1", 23456, 0, 0)) == ("::1", 23456) | ||
|
|
||
|
|
||
| def test_ipv6_four_tuple_with_nonzero_scopeid() -> None: | ||
| assert _extract_host_port_from_sockname(("fe80::1", 34567, 0, 3)) == ( | ||
| "fe80::1", | ||
| 34567, | ||
| ) | ||
|
|
||
|
|
||
| def test_unexpected_shape_returns_none() -> None: | ||
| # Not a tuple, single element, wrong element types — all graceful. | ||
| assert _extract_host_port_from_sockname(None) is None | ||
| assert _extract_host_port_from_sockname(("127.0.0.1",)) is None | ||
| assert _extract_host_port_from_sockname((12345, "127.0.0.1")) is None | ||
| # A raw string (e.g. what an AF_UNIX socket's getsockname returns) is | ||
| # not a tuple and must be rejected. | ||
| assert _extract_host_port_from_sockname("socket-path") is None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
Anyin a runtime-evaluated annotation can introduce an unnecessary runtime dependency ontyping.Anybeing in scope (depending on whether postponed evaluation of annotations is enabled in this module). Prefersock_name: objecthere (since you’re doing runtime type checks anyway), or ensureAnyis imported and annotations are postponed consistently across the module.