From 1b635766572aded2d54fe49a3cde95d99fa786b6 Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Tue, 30 Jun 2026 09:42:33 +0200 Subject: [PATCH] Don't mutate caller's registers list in convert_from_registers The STRING/BITS branch of convert_from_registers reversed the registers list in place when word_order="little", corrupting the caller's input so a second identical call decoded garbage (non-deterministic output). The numeric branch already avoids this by reversing a slice copy; reverse a copy here too so decoding is side-effect free. --- pymodbus/client/mixin.py | 4 +++- test/client/test_client.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/pymodbus/client/mixin.py b/pymodbus/client/mixin.py index a5c9d46d7..60859ada1 100644 --- a/pymodbus/client/mixin.py +++ b/pymodbus/client/mixin.py @@ -927,7 +927,9 @@ def convert_from_registers( # noqa: C901 if not (data_len := data_type.value[1]): byte_list = bytearray() if word_order == "little": - registers.reverse() + # Reverse a copy; the numeric branch below also works on a + # slice copy, so decoding must not mutate the caller's list. + registers = list(reversed(registers)) for x in registers: byte_list.extend(int.to_bytes(x, 2, "big")) if data_type == cls.DATATYPE.STRING: diff --git a/test/client/test_client.py b/test/client/test_client.py index faf791521..1b33a4dd0 100755 --- a/test/client/test_client.py +++ b/test/client/test_client.py @@ -366,6 +366,41 @@ def test_client_mixin_convert_1234(self, datatype, registers, value): assert result == value assert regs == registers + @pytest.mark.parametrize("word_order", ["big", "little"]) + @pytest.mark.parametrize( + ("datatype", "value"), + [ + (ModbusClientMixin.DATATYPE.STRING, "Hello"), + (ModbusClientMixin.DATATYPE.STRING, "abcdefghij"), + (ModbusClientMixin.DATATYPE.BITS, [True, False, True] + [False] * 17), + (ModbusClientMixin.DATATYPE.INT32, 123456), + (ModbusClientMixin.DATATYPE.INT32, [1, 2, 3, 4]), + ], + ) + def test_client_mixin_convert_from_registers_no_mutation( + self, datatype, value, word_order + ): + """convert_from_registers must not mutate input and stay deterministic. + + Regression: for STRING/BITS with word_order="little" the registers list + was reversed in place, corrupting the caller's data so a second identical + call decoded garbage (non-deterministic output). + """ + registers = ModbusClientMixin.convert_to_registers( + value, datatype, word_order=word_order + ) + snapshot = list(registers) + first = ModbusClientMixin.convert_from_registers( + registers, datatype, word_order=word_order + ) + # Decoding must leave the caller's list untouched ... + assert registers == snapshot + # ... so repeating the identical call yields the identical result. + second = ModbusClientMixin.convert_from_registers( + registers, datatype, word_order=word_order + ) + assert first == second + def test_client_mixin_convert_fail(self): """Test convert fail.""" with pytest.raises(TypeError):