From 9239976bd4e378782621d313054f54a27f5c0b6c Mon Sep 17 00:00:00 2001 From: Tobias Oberstein Date: Wed, 17 Jun 2026 10:12:23 +0200 Subject: [PATCH 1/2] start new dev branch; add audit file --- .audit/oberstet_fix_1850.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .audit/oberstet_fix_1850.md diff --git a/.audit/oberstet_fix_1850.md b/.audit/oberstet_fix_1850.md new file mode 100644 index 000000000..bf4c5d8f2 --- /dev/null +++ b/.audit/oberstet_fix_1850.md @@ -0,0 +1,8 @@ +- [ ] I did **not** use any AI-assistance tools to help create this pull request. +- [x] I **did** use AI-assistance tools to *help* create this pull request. +- [x] I have read, understood and followed the projects' [AI Policy](https://github.com/crossbario/autobahn-python/blob/main/AI_POLICY.md) when creating code, documentation etc. for this pull request. + +Submitted by: @oberstet +Date: 2026-06-17 +Related issue(s): #1850 +Branch: oberstet:fix_1850 From 3b5ec0b2dc8cfef90150a0a2209756fe7209d3f1 Mon Sep 17 00:00:00 2001 From: Tobias Oberstein Date: Wed, 17 Jun 2026 10:28:01 +0200 Subject: [PATCH 2/2] Fix RawSocket TransportLost leak on failed opening handshake (#1850) The Twisted WampRawSocketProtocol.abort() guarded on isOpen(), which requires an attached WAMP session. During the opening handshake no session exists yet, so a handshake failure (invalid magic byte from a port scanner / service probe, or no suitable serializer) made abort() take the else-branch and raise TransportLost. That exception escaped through dataReceived into the Twisted reactor and was logged as an "Unhandled Error" with a full stack trace, while the TCP connection stayed open. Fix: - abort() now guards on `self.transport is not None` instead of isOpen(): aborting a transport does not require an open WAMP session, only a transport. A genuinely missing transport still raises TransportLost. Post-handshake callers (stringReceived error paths, _on_handshake_complete) are unaffected, since an open session always has a transport. - The four handshake-failure sites (server/client x bad-magic/ no-serializer) now `return` after abort(). Previously they relied on abort() raising to stop processing; without the return they would fall through and continue the handshake (e.g. write a reply and start a session) on an aborted connection. The asyncio backend already handled this correctly (parse_handshake raises HandshakeError, caught in data_received -> protocol_error closes the transport and returns). Added cross-backend regression tests for both Twisted and asyncio, server and client. Fixes #1850. Note: abort() is part of ITransport; this changes its behavior in the pre-session state from raising TransportLost to closing the transport. Flagging for careful human review per AI_POLICY.md. Note: This work was completed with AI assistance (Claude Code). --- docs/changelog.rst | 4 ++ .../asyncio/test/test_aio_rawsocket.py | 44 +++++++++++++++++++ src/autobahn/twisted/rawsocket.py | 11 ++++- .../twisted/test/test_tx_rawsocket.py | 44 +++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2e4c55e2c..d8d935e4b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,6 +8,10 @@ Changelog 26.6.1 ------ +**WAMP RawSocket** + +* Fix the Twisted ``WampRawSocketProtocol`` raising ``TransportLost`` out of ``dataReceived`` when the opening handshake fails before a WAMP session is attached (e.g. an invalid magic byte from a port scanner). ``abort()`` now tears down the transport whenever a transport is present - rather than only when a session is open - so a failed handshake closes the connection cleanly with a single warning instead of an "Unhandled Error" stack trace, and handshake processing stops instead of continuing past the abort. The asyncio backend already behaved correctly; cross-backend regression tests were added for both. Thanks to @karel-un for the report (#1850) + **WAMP Serialization** * ``py-ubjson`` (unmaintained, sdist-only) is no longer an unconditional dependency. A base ``pip install autobahn`` — and the wheels-only / cross-arch case from #1849 (``pip download --only-binary :all: --platform ...``) — now resolves entirely from binary wheels (#1849) diff --git a/src/autobahn/asyncio/test/test_aio_rawsocket.py b/src/autobahn/asyncio/test/test_aio_rawsocket.py index 0a92f76d7..618e94b22 100644 --- a/src/autobahn/asyncio/test/test_aio_rawsocket.py +++ b/src/autobahn/asyncio/test/test_aio_rawsocket.py @@ -265,3 +265,47 @@ def fact_client(): proto.data_received(d) assert client.onMessage.called assert isinstance(client.onMessage.call_args[0][0], message.Abort) + + +@pytest.mark.skipif( + not os.environ.get("USE_ASYNCIO", False), reason="test runs on asyncio only" +) +def test_wamp_server_bad_magic_byte_aborts_cleanly(): + """ + A WAMP server receiving an invalid magic byte in the opening handshake + closes the transport cleanly and starts no session (cross-backend parity + with the Twisted fix for #1850). + """ + transport = Mock(spec_set=("abort", "close", "write", "get_extra_info")) + server = Mock(spec=["onOpen", "onMessage"]) + + proto = WampRawSocketServerFactory(lambda: server)() + proto.connection_made(transport) + + # any non-0x7f first octet is an invalid magic byte; this must not raise + proto.data_received(b"GET ") + + transport.close.assert_called_once_with() + server.onOpen.assert_not_called() + + +@pytest.mark.skipif( + not os.environ.get("USE_ASYNCIO", False), reason="test runs on asyncio only" +) +def test_wamp_client_bad_magic_byte_aborts_cleanly(): + """ + A WAMP client receiving an invalid magic byte in the opening handshake + closes the transport cleanly and starts no session (cross-backend parity + with the Twisted fix for #1850). + """ + transport = Mock(spec_set=("abort", "close", "write", "get_extra_info")) + client = Mock(spec=["onOpen", "onMessage"]) + + proto = WampRawSocketClientFactory(lambda: client)() + proto.connection_made(transport) + + # any non-0x7f first octet is an invalid magic byte; this must not raise + proto.data_received(b"GET ") + + transport.close.assert_called_once_with() + client.onOpen.assert_not_called() diff --git a/src/autobahn/twisted/rawsocket.py b/src/autobahn/twisted/rawsocket.py index fff4681d5..05766e883 100644 --- a/src/autobahn/twisted/rawsocket.py +++ b/src/autobahn/twisted/rawsocket.py @@ -295,7 +295,12 @@ def abort(self): """ Implements :func:`autobahn.wamp.interfaces.ITransport.abort` """ - if self.isOpen(): + # Guard on the transport, not isOpen(): isOpen() requires an attached + # WAMP session, but abort() must also tear down the transport when + # called before the session is established - e.g. on a failed opening + # handshake (bad magic byte / no suitable serializer), see #1850. + # Only a genuinely missing transport is a lost transport. + if self.transport is not None: if hasattr(self.transport, "abortConnection"): # ProcessProtocol lacks abortConnection() self.transport.abortConnection() @@ -338,6 +343,7 @@ def dataReceived(self, data): magic=_magic, ) self.abort() + return else: self.log.debug( "WampRawSocketServerProtocol: correct magic byte received" @@ -367,6 +373,7 @@ def dataReceived(self, data): serializers=self.factory._serializers.keys(), ) self.abort() + return # we request the client to send message of maximum length 2**reply_max_len_exp # @@ -460,6 +467,7 @@ def dataReceived(self, data): magic=_LazyHexFormatter(self._handshake_bytes[0]), ) self.abort() + return # peer requests us to _send_ messages of maximum length 2**max_len_exp # @@ -479,6 +487,7 @@ def dataReceived(self, data): serializers=self._serializer.RAWSOCKET_SERIALIZER_ID, ) self.abort() + return self._handshake_complete = True diff --git a/src/autobahn/twisted/test/test_tx_rawsocket.py b/src/autobahn/twisted/test/test_tx_rawsocket.py index 517a71518..8577a8cf1 100644 --- a/src/autobahn/twisted/test/test_tx_rawsocket.py +++ b/src/autobahn/twisted/test/test_tx_rawsocket.py @@ -69,3 +69,47 @@ def test_handshake_succeeds(self): # onOpen is called on the session session_mock.onOpen.assert_called_once_with(p) server_session_mock.onOpen.assert_called_once_with(sp) + + def test_server_bad_magic_byte_aborts_cleanly(self): + """ + A server receiving an invalid magic byte in the opening handshake + aborts the transport cleanly instead of raising ``TransportLost`` out + of ``dataReceived`` (regression test for #1850). + """ + server_session_mock = Mock() + st = FakeTransport() + sf = WampRawSocketServerFactory(lambda: server_session_mock) + sp = WampRawSocketServerProtocol() + sp.transport = st + sp.factory = sf + + sp.connectionMade() + + # any non-0x7f first octet is an invalid magic byte; this must not raise + sp.dataReceived(b"GET ") + + # the transport was aborted and no WAMP session was started + self.assertTrue(st.abort_called()) + server_session_mock.onOpen.assert_not_called() + + def test_client_bad_magic_byte_aborts_cleanly(self): + """ + A client receiving an invalid magic byte in the opening handshake + aborts the transport cleanly instead of raising ``TransportLost`` out + of ``dataReceived`` (regression test for #1850). + """ + session_mock = Mock() + t = FakeTransport() + f = WampRawSocketClientFactory(lambda: session_mock) + p = WampRawSocketClientProtocol() + p.transport = t + p.factory = f + + p.connectionMade() + + # any non-0x7f first octet is an invalid magic byte; this must not raise + p.dataReceived(b"GET ") + + # the transport was aborted and no WAMP session was started + self.assertTrue(t.abort_called()) + session_mock.onOpen.assert_not_called()