From dbcde4005baadbf457456ab7305ff67a364f475b Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Tue, 28 Jul 2026 16:20:26 +0530 Subject: [PATCH 1/2] Fix undecodable function-code exception identity (#2990) When the server cannot decode a request function code, echo the framed transaction and device ids on the illegal-function exception response, and use function code 0x00 instead of the hardcoded 0x28. Fixes #2990 Signed-off-by: Sankalp Thakur --- pymodbus/framer/base.py | 9 +++++- pymodbus/server/requesthandler.py | 13 ++++++-- test/server/test_requesthandler.py | 51 ++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/pymodbus/framer/base.py b/pymodbus/framer/base.py index 773157797..8f0572da1 100644 --- a/pymodbus/framer/base.py +++ b/pymodbus/framer/base.py @@ -88,7 +88,14 @@ def handleFrame( ) continue if (pdu := self.decoder.decode(frame_data)) is None: - raise ModbusIOException("Unable to decode request") + # Preserve framing identity so the server can echo transaction/dev + # ids on the undecodable-function exception path (see #2990). + # Do not recover a function code from garbage payloads — noise on + # serial lines is the common cause of this path. + exc = ModbusIOException("Unable to decode request") + exc.transaction_id = tid + exc.dev_id = dev_id + raise exc pdu.dev_id = dev_id pdu.transaction_id = tid return used_len, pdu diff --git a/pymodbus/server/requesthandler.py b/pymodbus/server/requesthandler.py index ebaf52917..9928f9db5 100644 --- a/pymodbus/server/requesthandler.py +++ b/pymodbus/server/requesthandler.py @@ -62,8 +62,17 @@ def callback_data(self, data: bytes, addr: tuple | None = None) -> int: """Handle received data.""" try: used_len = super().callback_data(data, addr) - except ModbusIOException: - response = ExceptionResponse(40, exception_code=ExcCodes.ILLEGAL_FUNCTION) + except ModbusIOException as exc: + # Undecodable function codes (and frame garbage) land here. last_pdu + # is cleared before framing runs, so identity comes from the framer + # exception attributes when available. Use function code 0x00 rather + # than a hardcoded unrelated value (was 40 / 0x28) — see #2990. + response = ExceptionResponse( + 0x00, + exception_code=ExcCodes.ILLEGAL_FUNCTION, + device_id=getattr(exc, "dev_id", 0) or 0, + transaction=getattr(exc, "transaction_id", 0) or 0, + ) self.server_send(response, 0) return len(data) if self.last_pdu: diff --git a/test/server/test_requesthandler.py b/test/server/test_requesthandler.py index ebd171354..436b8a961 100755 --- a/test/server/test_requesthandler.py +++ b/test/server/test_requesthandler.py @@ -53,6 +53,57 @@ async def test_rh_callback_data(self, requesthandler): data = b"012" assert len(data) == requesthandler.callback_data(data, None) + async def test_rh_callback_data_undecodable_echoes_identity(self, requesthandler): + """Undecodable FC exception must echo framing tid/dev_id (#2990).""" + with mock.patch( + "pymodbus.transaction.TransactionManager.callback_data" + ) as cb_data: + exc = ModbusIOException("Unable to decode request") + exc.transaction_id = 0x000A + exc.dev_id = 7 + cb_data.side_effect = exc + data = b"\x00\x0a\x00\x00\x00\x06\x07\x0a\x00\x00\x00\x01" + assert len(data) == requesthandler.callback_data(data, None) + + requesthandler.pdu_send.assert_called_once() + response = requesthandler.pdu_send.call_args.args[0] + assert isinstance(response, ExceptionResponse) + assert response.transaction_id == 0x000A + assert response.dev_id == 7 + # 0x00 | 0x80 — not the previous hardcoded 0x28 | 0x80 + assert response.function_code == 0x80 + assert response.exception_code == 0x01 # ILLEGAL_FUNCTION + + async def test_rh_callback_data_undecodable_without_framing_attrs( + self, requesthandler + ): + """Missing framing attrs fall back to zeros rather than crashing.""" + with mock.patch( + "pymodbus.transaction.TransactionManager.callback_data" + ) as cb_data: + cb_data.side_effect = ModbusIOException("Unable to decode request") + data = b"garbage" + assert len(data) == requesthandler.callback_data(data, None) + + response = requesthandler.pdu_send.call_args.args[0] + assert response.transaction_id == 0 + assert response.dev_id == 0 + assert response.function_code == 0x80 + + async def test_rh_callback_data_undecodable_real_socket_frame(self, requesthandler): + """End-to-end: undecodable socket FC echoes MBAP transaction id.""" + # MBAP: tid=0x1234, pid=0, len=6, uid=1, PDU: FC=0x09 + 4 data bytes + frame = b"\x12\x34\x00\x00\x00\x06\x01\x09\x00\x00\x00\x01" + assert len(frame) == requesthandler.callback_data(frame, None) + + requesthandler.pdu_send.assert_called_once() + response = requesthandler.pdu_send.call_args.args[0] + assert isinstance(response, ExceptionResponse) + assert response.transaction_id == 0x1234 + assert response.dev_id == 1 + assert response.function_code == 0x80 + assert response.exception_code == 0x01 + async def test_rh_handle_request(self, requesthandler): """Test __init__.""" requesthandler.last_pdu = None From cdccdf128e4f8bf60423e7c335b82c682ea6e6f6 Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Tue, 28 Jul 2026 16:21:49 +0530 Subject: [PATCH 2/2] fix: declare framing identity on ModbusIOException Add transaction_id and dev_id as first-class fields so the type checker accepts the undecodable-function exception path for #2990. Signed-off-by: Sankalp Thakur --- pymodbus/exceptions.py | 14 +++++++++++++- pymodbus/framer/base.py | 9 +++++---- pymodbus/server/requesthandler.py | 4 ++-- test/server/test_requesthandler.py | 8 +++++--- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/pymodbus/exceptions.py b/pymodbus/exceptions.py index 8d8392d00..d96211e3d 100644 --- a/pymodbus/exceptions.py +++ b/pymodbus/exceptions.py @@ -36,12 +36,24 @@ def isError(self): class ModbusIOException(ModbusException): """Error resulting from data i/o.""" - def __init__(self, string="", function_code=None): + def __init__( + self, + string="", + function_code=None, + *, + transaction_id: int = 0, + dev_id: int = 0, + ): """Initialize the exception. :param string: The message to append to the error + :param function_code: Optional function code associated with the error + :param transaction_id: Optional framing transaction id (e.g. MBAP TID) + :param dev_id: Optional framing device / unit id """ self.fcode = function_code + self.transaction_id = transaction_id + self.dev_id = dev_id self.message = f"[Input/Output] {string}" ModbusException.__init__(self, self.message) diff --git a/pymodbus/framer/base.py b/pymodbus/framer/base.py index 8f0572da1..df908d90a 100644 --- a/pymodbus/framer/base.py +++ b/pymodbus/framer/base.py @@ -92,10 +92,11 @@ def handleFrame( # ids on the undecodable-function exception path (see #2990). # Do not recover a function code from garbage payloads — noise on # serial lines is the common cause of this path. - exc = ModbusIOException("Unable to decode request") - exc.transaction_id = tid - exc.dev_id = dev_id - raise exc + raise ModbusIOException( + "Unable to decode request", + transaction_id=tid, + dev_id=dev_id, + ) pdu.dev_id = dev_id pdu.transaction_id = tid return used_len, pdu diff --git a/pymodbus/server/requesthandler.py b/pymodbus/server/requesthandler.py index 9928f9db5..170391a1b 100644 --- a/pymodbus/server/requesthandler.py +++ b/pymodbus/server/requesthandler.py @@ -70,8 +70,8 @@ def callback_data(self, data: bytes, addr: tuple | None = None) -> int: response = ExceptionResponse( 0x00, exception_code=ExcCodes.ILLEGAL_FUNCTION, - device_id=getattr(exc, "dev_id", 0) or 0, - transaction=getattr(exc, "transaction_id", 0) or 0, + device_id=exc.dev_id, + transaction=exc.transaction_id, ) self.server_send(response, 0) return len(data) diff --git a/test/server/test_requesthandler.py b/test/server/test_requesthandler.py index 436b8a961..668f60c89 100755 --- a/test/server/test_requesthandler.py +++ b/test/server/test_requesthandler.py @@ -58,9 +58,11 @@ async def test_rh_callback_data_undecodable_echoes_identity(self, requesthandler with mock.patch( "pymodbus.transaction.TransactionManager.callback_data" ) as cb_data: - exc = ModbusIOException("Unable to decode request") - exc.transaction_id = 0x000A - exc.dev_id = 7 + exc = ModbusIOException( + "Unable to decode request", + transaction_id=0x000A, + dev_id=7, + ) cb_data.side_effect = exc data = b"\x00\x0a\x00\x00\x00\x06\x07\x0a\x00\x00\x00\x01" assert len(data) == requesthandler.callback_data(data, None)