From a19a639ddf8008b5671ab6e441989936c37c1fb1 Mon Sep 17 00:00:00 2001 From: goodboy Date: Tue, 18 Aug 2026 04:50:00 -0400 Subject: [PATCH] Tighten `TIPC` address-shape dispatch Restrict proto-key matching to numeric 3- or 4-element descriptors so a UDS directory named `tipc` stays UDS. Route `/tipc` parsing through `TIPCAddress.from_addr()` to normalize zone scope and report malformed input clearly. Also align UDS unwrapped metadata with its actual `(str, str)` shape. Keep the TIPC test module portable by importing `SOL_TIPC` from the backend's UAPI fallback instead of the host `socket` module. Review: PR #493 (copilot-pull-request-reviewer[bot],goodboy) https://github.com/goodboy/tractor/pull/493 (this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`)) --- tests/ipc/test_tipc.py | 33 ++++++++++++++++++++++++++++++++- tractor/discovery/_addr.py | 6 +++++- tractor/discovery/_multiaddr.py | 21 +++++++++++++++------ tractor/ipc/_uds.py | 4 ++-- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/tests/ipc/test_tipc.py b/tests/ipc/test_tipc.py index 88ab4971..0a82ee3e 100644 --- a/tests/ipc/test_tipc.py +++ b/tests/ipc/test_tipc.py @@ -9,11 +9,11 @@ the pure address-algebra cases run everywhere. from __future__ import annotations import errno import struct +from types import SimpleNamespace from socket import ( SOCK_STREAM, SOL_SOCKET, SO_ACCEPTCONN, - SOL_TIPC, ) import pytest @@ -27,8 +27,10 @@ from tractor.discovery._multiaddr import ( parse_maddr, ) from tractor.ipc import _tipc +from tractor.ipc._uds import UDSAddress from tractor.ipc._tipc import ( AF_TIPC, + SOL_TIPC, TIPC_ADDR_ID, TIPC_ADDR_NAME, TIPC_CLUSTER_SCOPE, @@ -114,6 +116,29 @@ def test_zone_scope_normalized_to_cluster(): assert addr.is_valid +def test_maddr_parse_normalizes_and_reports_bad_input(): + ''' + The interim `/tipc` parser must route through + `TIPCAddress.from_addr()` so deprecated zone scope is normalized + exactly like every other unwrapped-address entrypoint. + + A malformed segment count previously leaked the tuple-unpacking + `ValueError`, which gave callers no indication that the TIPC + multiaddr grammar itself was invalid. + + ''' + addr: TIPCAddress = parse_maddr( + f'/tipc/{TRACTOR_STYPE}/7/{TIPC_ZONE_SCOPE}' + ) + assert addr._scope == TIPC_CLUSTER_SCOPE + + with pytest.raises( + ValueError, + match='Invalid TIPC multiaddr', + ): + parse_maddr('/tipc/not-enough-segments') + + def test_addr_from_bare_port_id_raises(): ''' A `TIPC_ADDR_ID` 5-tuple carries no service-name so it can @@ -235,6 +260,12 @@ def test_wrap_address_dispatches_on_the_proto_key(): 'tipc', TRACTOR_STYPE, 1616, TIPC_CLUSTER_SCOPE, ) + # A UDS directory named `tipc` is still a valid classic + # 2-element address, not a malformed proto-keyed TIPC one. + uds: UDSAddress = wrap_address(('tipc', 'actor.sock')) + assert isinstance(uds, UDSAddress) + assert UDSAddress.unwrapped_type == tuple[str, str] + def test_maddr_roundtrip(): ''' diff --git a/tractor/discovery/_addr.py b/tractor/discovery/_addr.py index 94b922fb..4297a2d0 100644 --- a/tractor/discovery/_addr.py +++ b/tractor/discovery/_addr.py @@ -265,7 +265,11 @@ def wrap_address( # # NOTE, a bare seq-pattern matches `list` too, which is # what `msgpack` decodes our tuples back to. - case ('tipc', *_): + case ( + ('tipc', int(), int()) + | + ('tipc', int(), int(), int()) + ): cls = TIPCAddress # classic network socket-address as tuple/list diff --git a/tractor/discovery/_multiaddr.py b/tractor/discovery/_multiaddr.py index cc9eb9db..15b96cf0 100644 --- a/tractor/discovery/_multiaddr.py +++ b/tractor/discovery/_multiaddr.py @@ -70,6 +70,9 @@ def mk_maddr( dispatching on the `.proto_key` to build the correct multiaddr-spec-compliant protocol path. + Return a `Multiaddr` for registered protocols. TIPC remains + an interim `str` until its upstream multiaddr protocol lands. + ''' proto_key: str = addr.proto_key maddr_proto: str|None = _tpt_proto_to_maddr.get(proto_key) @@ -132,12 +135,18 @@ def parse_maddr( # XXX MUST come before `Multiaddr()` which rejects the # not-yet-registered `/tipc` proto name outright. if maddr_str.startswith(_tipc_maddr_prefix): - _, _, stype, instance, scope = maddr_str.split('/') - return TIPCAddress( - _stype=int(stype), - _instance=int(instance), - _scope=int(scope), - ) + try: + _, _, stype, instance, scope = maddr_str.split('/') + return TIPCAddress.from_addr(( + 'tipc', + int(stype), + int(instance), + int(scope), + )) + except (TypeError, ValueError) as src_err: + raise ValueError( + f'Invalid TIPC multiaddr: {maddr_str!r}' + ) from src_err maddr = Multiaddr(maddr_str) proto_names: list[str] = [ diff --git a/tractor/ipc/_uds.py b/tractor/ipc/_uds.py index bd1a44dd..4a4c45f1 100644 --- a/tractor/ipc/_uds.py +++ b/tractor/ipc/_uds.py @@ -114,7 +114,7 @@ class UDSAddress( # -[ ] need to check what other mult-transport frameworks do # like zmq, nng, uri-spec et al! proto_key: ClassVar[str] = 'uds' - unwrapped_type: ClassVar[type] = tuple[str, int] + unwrapped_type: ClassVar[type] = tuple[str, str] def_bindspace: ClassVar[Path] = get_rt_dir() # NOTE, `getsockname()` answers the sock-file path as a `str` @@ -176,7 +176,7 @@ class UDSAddress( f'{addr!r}\n' ) - def unwrap(self) -> tuple[str, int]: + def unwrap(self) -> tuple[str, str]: # XXX NOTE, since this gets passed DIRECTLY to # `.ipc._uds.open_unix_socket_w_passcred()` return (