From 6e5c35220c1b80c6f0bf7684c1f081804576a4c5 Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Wed, 21 Oct 2026 11:10:16 +0400 Subject: [PATCH 1/2] Report bad file record frames as undecodable The file record PDUs (read and write file record, read fifo queue) raise ModbusException when the byte or fifo count in a frame points past its end. DecodePDU.decode only turned ValueError, IndexError and struct.error into ModbusIOException, so these escaped the framer without transaction and device id, and the server could not answer them with an exception response like any other undecodable request. Convert ModbusException there as well, and keep ModbusIOException as is. --- pymodbus/pdu/decoders.py | 10 ++++++++-- test/framer/test_framer.py | 12 ++++++++++++ test/pdu/test_decoders.py | 13 +++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/pymodbus/pdu/decoders.py b/pymodbus/pdu/decoders.py index ba7d5b00c..cf810bb4b 100644 --- a/pymodbus/pdu/decoders.py +++ b/pymodbus/pdu/decoders.py @@ -5,7 +5,11 @@ import copy import struct -from ..exceptions import MessageRegisterException, ModbusIOException +from ..exceptions import ( + MessageRegisterException, + ModbusException, + ModbusIOException, +) from ..logging import Log from .exceptionresponse import ExceptionResponse from .pdu import ModbusPDU @@ -84,7 +88,9 @@ def decode(self, frame: bytes) -> ModbusPDU: str(pdu), ) return pdu - except (ValueError, IndexError, struct.error) as exc: + except ModbusIOException: + raise + except (ValueError, IndexError, struct.error, ModbusException) as exc: raise ModbusIOException( f"Unable to decode frame {exc}", function_code=function_code ) from exc diff --git a/test/framer/test_framer.py b/test/framer/test_framer.py index 88663ec4c..ece10034c 100644 --- a/test/framer/test_framer.py +++ b/test/framer/test_framer.py @@ -4,6 +4,7 @@ import pytest +from pymodbus.exceptions import ModbusIOException from pymodbus.framer import ( FramerAscii, FramerBase, @@ -956,6 +957,17 @@ def test_framer_encode(self, test_framer, msg, mockPDU): actual = test_framer.buildFrame(message) assert msg == actual + def test_bad_file_record_request_for_framer_socket(self): + """Test a bad file record request keeps the ids the server reply needs.""" + framer = FramerSocket(DecodePDU(True)) + # transaction 1, protocol 0, length 3, device 1, read file record, byte count 16 + data = b"\x00\x01\x00\x00\x00\x03\x01\x14\x10" + with pytest.raises(ModbusIOException) as exc: + framer.handleFrame(data, 0, 0) + assert exc.value.transaction_id == 1 + assert exc.value.dev_id == 1 + assert exc.value.used_len == len(data) + def test_invalid_protocol_id_for_framer_socket(self): """Test that ModbusSocketFramer rejects an invalid Protocol ID.""" framer = FramerSocket(DecodePDU(False)) diff --git a/test/pdu/test_decoders.py b/test/pdu/test_decoders.py index eccc25390..7aa9f2290 100644 --- a/test/pdu/test_decoders.py +++ b/test/pdu/test_decoders.py @@ -139,6 +139,19 @@ def test_decode_bad_frame(self, decoder, frame): with pytest.raises(ModbusIOException): decoder.decode(frame) + @pytest.mark.parametrize( + ("decoder", "frame"), + [ + (server, b"\x14\x10"), # read file record, byte count past the frame + (server, b"\x15\x10"), # write file record, byte count past the frame + (client, b"\x18\x00\x06\x00\x05\x00\x01"), # fifo count past the frame + ], + ) + def test_decode_bad_file_record_frame(self, decoder, frame): + """Test bad file record frames fail like other undecodable frames.""" + with pytest.raises(ModbusIOException): + decoder.decode(frame) + def test_decode_unknown_sub(self): """Test for unknown sub code.""" with pytest.raises(ModbusIOException): From 513365158d3b4c0949e9d421e6b4c1b691a3bbdb Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Thu, 22 Oct 2026 07:29:45 +0400 Subject: [PATCH 2/2] Raise ModbusIOException for bad file record frames ModbusException signals a problem with the use of the API, so decode() is back to converting only ValueError, IndexError and struct.error. The three checks that reject a frame while decoding (the read and write file record byte counts and the read fifo queue count) now raise ModbusIOException themselves, with the function code. The constructor checks in file_message.py still raise ModbusException. --- pymodbus/pdu/decoders.py | 10 ++-------- pymodbus/pdu/file_message.py | 14 ++++++++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pymodbus/pdu/decoders.py b/pymodbus/pdu/decoders.py index cf810bb4b..ba7d5b00c 100644 --- a/pymodbus/pdu/decoders.py +++ b/pymodbus/pdu/decoders.py @@ -5,11 +5,7 @@ import copy import struct -from ..exceptions import ( - MessageRegisterException, - ModbusException, - ModbusIOException, -) +from ..exceptions import MessageRegisterException, ModbusIOException from ..logging import Log from .exceptionresponse import ExceptionResponse from .pdu import ModbusPDU @@ -88,9 +84,7 @@ def decode(self, frame: bytes) -> ModbusPDU: str(pdu), ) return pdu - except ModbusIOException: - raise - except (ValueError, IndexError, struct.error, ModbusException) as exc: + except (ValueError, IndexError, struct.error) as exc: raise ModbusIOException( f"Unable to decode frame {exc}", function_code=function_code ) from exc diff --git a/pymodbus/pdu/file_message.py b/pymodbus/pdu/file_message.py index 5041ba013..fa2fa2689 100644 --- a/pymodbus/pdu/file_message.py +++ b/pymodbus/pdu/file_message.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from ..datastore import ModbusServerContext -from ..exceptions import ModbusException +from ..exceptions import ModbusException, ModbusIOException from .decoders import DecodePDU from .pdu import ModbusPDU @@ -68,7 +68,9 @@ def x_decode(self, data: bytes) -> None: """Decode the incoming request.""" self.records = [] if (byte_count := int(data[0])) > len(data) - 1: - raise ModbusException(f"Invalid byte count: {byte_count}") + raise ModbusIOException( + f"Invalid byte count: {byte_count}", function_code=self.function_code + ) for count in range(1, byte_count, 7): decoded = struct.unpack(">BHHH", data[count : count + 7]) record = FileRecord( @@ -176,7 +178,9 @@ def x_encode(self) -> bytes: def x_decode(self, data: bytes) -> None: """Decode the incoming request.""" if (byte_count := int(data[0])) > len(data) - 1: - raise ModbusException(f"Invalid byte count: {byte_count}") + raise ModbusIOException( + f"Invalid byte count: {byte_count}", function_code=self.function_code + ) count = 1 self.records.clear() while count < byte_count: @@ -323,7 +327,9 @@ def x_decode(self, data: bytes) -> None: self.values = [] _, count = struct.unpack(">HH", data[0:4]) if 4 + count * 2 > len(data): - raise ModbusException(f"Invalid fifo count: {count}") + raise ModbusIOException( + f"Invalid fifo count: {count}", function_code=self.function_code + ) for index in range(0, count): idx = 4 + index * 2 self.values.append(struct.unpack(">H", data[idx : idx + 2])[0])