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]) 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):