fix(websocket): handle IPv6 4-tuple in listener getsockname()#1317
fix(websocket): handle IPv6 4-tuple in listener getsockname()#1317yashksaini-coder wants to merge 2 commits into
Conversation
WebsocketListener.listen() unpacked sock.getsockname() directly into (sock_addr, sock_port), which raised ValueError on IPv6 sockets whose getsockname() returns (host, port, flowinfo, scopeid). The ValueError was caught by the outer except and re-raised as OpenConnectionError, so IPv6 WebSocket listeners could not start. Pull the parse into a small _extract_host_port_from_sockname helper that accepts any tuple of length >= 2 with (str, int) in the first two slots, and falls back gracefully on unknown shapes. Add unit tests covering IPv4 2-tuple, IPv6 4-tuple (zero and nonzero scopeid), and unexpected shapes — no actual IPv6 socket needed. Fixes libp2p#1316.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes IPv6 WebSocket listener binding by correctly handling the 4-tuple shape returned from socket.getsockname() and preventing the listener from failing with a masked ValueError.
Changes:
- Added
_extract_host_port_from_sockname()to safely parse(host, port)from IPv4/IPv6getsockname()results. - Updated
WebsocketListener.listen()to use the helper and warn+fallback on unexpected shapes. - Added focused unit tests and a changelog entry for the IPv6 binding fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| libp2p/transport/websocket/listener.py | Adds a helper to parse IPv4/IPv6 getsockname() return values and uses it during listener setup. |
| tests/core/transport/websocket/test_extract_host_port.py | Introduces unit tests covering IPv4/IPv6 tuple shapes and unknown inputs. |
| newsfragments/1316.bugfix.rst | Documents the IPv6 listener binding fix and the new fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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: | ||
| logger.warning( | ||
| "Unexpected getsockname() result %r; " | ||
| "falling back to requested host/port", | ||
| sock_name, | ||
| ) |
There was a problem hiding this comment.
The warning says the listener is 'falling back to requested host/port', but the code doesn’t explicitly apply a fallback here. This makes the behavior dependent on actual_host/actual_port being pre-initialized elsewhere and is harder to reason about. Consider explicitly setting actual_host/actual_port to the requested values in the else branch to match the log message and keep the control flow self-contained.
| def _extract_host_port_from_sockname( | ||
| sock_name: Any, | ||
| ) -> tuple[str, int] | None: |
There was a problem hiding this comment.
Using Any in a runtime-evaluated annotation can introduce an unnecessary runtime dependency on typing.Any being in scope (depending on whether postponed evaluation of annotations is enabled in this module). Prefer sock_name: object here (since you’re doing runtime type checks anyway), or ensure Any is imported and annotations are postponed consistently across the module.
- _extract_host_port_from_sockname now takes sock_name: object rather than Any, matching the runtime isinstance checks and avoiding an unnecessary typing.Any dependency - The sockname-unrecognised warning branch now explicitly restores actual_host/actual_port from the requested host/port, so the 'falling back to requested host/port' log line is self-contained and the intent is obvious from the code flow
|
Addressed Copilot feedback in cc5813b:
|
Summary
Fixes #1316 —
WebsocketListener.listen()could not bind any IPv6 address because it unpackedsocket.getsockname()directly into a 2-tuple, which fails for IPv6 (which returns a 4-tuple(host, port, flowinfo, scopeid)).Context
The
ValueErrorfrom the bad unpack was caught by the outerexcept Exceptioninlisten()and re-raised asOpenConnectionError("Failed to listen on ..."), so IPv6 WebSocket listeners failed silently-but-hard rather than exposing the real cause. Flagged as a follow-up from the PR #1308 review (inline comment onlibp2p/transport/websocket/listener.py:289).Approach
Extract the socket-name parsing into
_extract_host_port_from_sockname(), a small helper that accepts any tuple of length ≥ 2 where the first two elements are a(host: str, port: int)pair. Any other shape returnsNone, and the listener logs a warning and falls back to the originally requested host/port. Because the helper is a pure function, it can be unit-tested without binding an actual IPv6 socket (which is not portable across all CI environments).Files changed
libp2p/transport/websocket/listener.py— add_extract_host_port_from_sockname, use it inlisten()tests/core/transport/websocket/test_extract_host_port.py— new unit tests (IPv4 2-tuple, IPv6 4-tuple with zero and nonzero scopeid, unrecognised shapes)newsfragments/1316.bugfix.rst— changelog entryTesting
pytest tests/core/transport/websocket/test_extract_host_port.py tests/core/transport/websocket/test_websocket.py— 52 passed (including 4 new cases)make lintclean (ruff, ruff-format, mypy, pyrefly, path audit)References