Skip to content

Commit f005049

Browse files
authored
Fix 1850 (#1862)
* start new dev branch; add audit file * 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).
1 parent b66b854 commit f005049

5 files changed

Lines changed: 110 additions & 1 deletion

File tree

.audit/oberstet_fix_1850.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
- [ ] I did **not** use any AI-assistance tools to help create this pull request.
2+
- [x] I **did** use AI-assistance tools to *help* create this pull request.
3+
- [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.
4+
5+
Submitted by: @oberstet
6+
Date: 2026-06-17
7+
Related issue(s): #1850
8+
Branch: oberstet:fix_1850

docs/changelog.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ Changelog
88
26.6.1
99
------
1010

11+
**WAMP RawSocket**
12+
13+
* 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)
14+
1115
**WAMP Serialization**
1216

1317
* ``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)

src/autobahn/asyncio/test/test_aio_rawsocket.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,3 +265,47 @@ def fact_client():
265265
proto.data_received(d)
266266
assert client.onMessage.called
267267
assert isinstance(client.onMessage.call_args[0][0], message.Abort)
268+
269+
270+
@pytest.mark.skipif(
271+
not os.environ.get("USE_ASYNCIO", False), reason="test runs on asyncio only"
272+
)
273+
def test_wamp_server_bad_magic_byte_aborts_cleanly():
274+
"""
275+
A WAMP server receiving an invalid magic byte in the opening handshake
276+
closes the transport cleanly and starts no session (cross-backend parity
277+
with the Twisted fix for #1850).
278+
"""
279+
transport = Mock(spec_set=("abort", "close", "write", "get_extra_info"))
280+
server = Mock(spec=["onOpen", "onMessage"])
281+
282+
proto = WampRawSocketServerFactory(lambda: server)()
283+
proto.connection_made(transport)
284+
285+
# any non-0x7f first octet is an invalid magic byte; this must not raise
286+
proto.data_received(b"GET ")
287+
288+
transport.close.assert_called_once_with()
289+
server.onOpen.assert_not_called()
290+
291+
292+
@pytest.mark.skipif(
293+
not os.environ.get("USE_ASYNCIO", False), reason="test runs on asyncio only"
294+
)
295+
def test_wamp_client_bad_magic_byte_aborts_cleanly():
296+
"""
297+
A WAMP client receiving an invalid magic byte in the opening handshake
298+
closes the transport cleanly and starts no session (cross-backend parity
299+
with the Twisted fix for #1850).
300+
"""
301+
transport = Mock(spec_set=("abort", "close", "write", "get_extra_info"))
302+
client = Mock(spec=["onOpen", "onMessage"])
303+
304+
proto = WampRawSocketClientFactory(lambda: client)()
305+
proto.connection_made(transport)
306+
307+
# any non-0x7f first octet is an invalid magic byte; this must not raise
308+
proto.data_received(b"GET ")
309+
310+
transport.close.assert_called_once_with()
311+
client.onOpen.assert_not_called()

src/autobahn/twisted/rawsocket.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,12 @@ def abort(self):
295295
"""
296296
Implements :func:`autobahn.wamp.interfaces.ITransport.abort`
297297
"""
298-
if self.isOpen():
298+
# Guard on the transport, not isOpen(): isOpen() requires an attached
299+
# WAMP session, but abort() must also tear down the transport when
300+
# called before the session is established - e.g. on a failed opening
301+
# handshake (bad magic byte / no suitable serializer), see #1850.
302+
# Only a genuinely missing transport is a lost transport.
303+
if self.transport is not None:
299304
if hasattr(self.transport, "abortConnection"):
300305
# ProcessProtocol lacks abortConnection()
301306
self.transport.abortConnection()
@@ -338,6 +343,7 @@ def dataReceived(self, data):
338343
magic=_magic,
339344
)
340345
self.abort()
346+
return
341347
else:
342348
self.log.debug(
343349
"WampRawSocketServerProtocol: correct magic byte received"
@@ -367,6 +373,7 @@ def dataReceived(self, data):
367373
serializers=self.factory._serializers.keys(),
368374
)
369375
self.abort()
376+
return
370377

371378
# we request the client to send message of maximum length 2**reply_max_len_exp
372379
#
@@ -460,6 +467,7 @@ def dataReceived(self, data):
460467
magic=_LazyHexFormatter(self._handshake_bytes[0]),
461468
)
462469
self.abort()
470+
return
463471

464472
# peer requests us to _send_ messages of maximum length 2**max_len_exp
465473
#
@@ -479,6 +487,7 @@ def dataReceived(self, data):
479487
serializers=self._serializer.RAWSOCKET_SERIALIZER_ID,
480488
)
481489
self.abort()
490+
return
482491

483492
self._handshake_complete = True
484493

src/autobahn/twisted/test/test_tx_rawsocket.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,47 @@ def test_handshake_succeeds(self):
6969
# onOpen is called on the session
7070
session_mock.onOpen.assert_called_once_with(p)
7171
server_session_mock.onOpen.assert_called_once_with(sp)
72+
73+
def test_server_bad_magic_byte_aborts_cleanly(self):
74+
"""
75+
A server receiving an invalid magic byte in the opening handshake
76+
aborts the transport cleanly instead of raising ``TransportLost`` out
77+
of ``dataReceived`` (regression test for #1850).
78+
"""
79+
server_session_mock = Mock()
80+
st = FakeTransport()
81+
sf = WampRawSocketServerFactory(lambda: server_session_mock)
82+
sp = WampRawSocketServerProtocol()
83+
sp.transport = st
84+
sp.factory = sf
85+
86+
sp.connectionMade()
87+
88+
# any non-0x7f first octet is an invalid magic byte; this must not raise
89+
sp.dataReceived(b"GET ")
90+
91+
# the transport was aborted and no WAMP session was started
92+
self.assertTrue(st.abort_called())
93+
server_session_mock.onOpen.assert_not_called()
94+
95+
def test_client_bad_magic_byte_aborts_cleanly(self):
96+
"""
97+
A client receiving an invalid magic byte in the opening handshake
98+
aborts the transport cleanly instead of raising ``TransportLost`` out
99+
of ``dataReceived`` (regression test for #1850).
100+
"""
101+
session_mock = Mock()
102+
t = FakeTransport()
103+
f = WampRawSocketClientFactory(lambda: session_mock)
104+
p = WampRawSocketClientProtocol()
105+
p.transport = t
106+
p.factory = f
107+
108+
p.connectionMade()
109+
110+
# any non-0x7f first octet is an invalid magic byte; this must not raise
111+
p.dataReceived(b"GET ")
112+
113+
# the transport was aborted and no WAMP session was started
114+
self.assertTrue(t.abort_called())
115+
session_mock.onOpen.assert_not_called()

0 commit comments

Comments
 (0)