From 40ac6bd8ff67f695898c2a3484b5614b64d0f807 Mon Sep 17 00:00:00 2001 From: Kaiser <1727168348@qq.com> Date: Fri, 16 Oct 2026 10:35:31 +0800 Subject: [PATCH] fix: preserve Modbus target identity and explicit TCP unit zero Reject target aliasing at configuration, encoding and response boundaries. Preserve explicit TCP unit 0 in configured and discovery probes. AI-assisted implementation and regression tests; 275 local tests passed. --- core/config_manager.py | 8 ++- core/generic_detector.py | 8 +-- core/protocol_engine.py | 19 ++++-- docs/modbus-targets.md | 15 ++++ tests/test_modbus_identity.py | 125 ++++++++++++++++++++++++++++++++++ 5 files changed, 164 insertions(+), 11 deletions(-) create mode 100644 docs/modbus-targets.md create mode 100644 tests/test_modbus_identity.py diff --git a/core/config_manager.py b/core/config_manager.py index 2d02710..3e01b57 100644 --- a/core/config_manager.py +++ b/core/config_manager.py @@ -5,7 +5,7 @@ import re from typing import Dict, Iterable, List, Optional -from .protocol_engine import ProtocolConfigError, decode_hex, validate_modbus_read +from .protocol_engine import ProtocolConfigError, decode_hex, validate_modbus_read, validate_modbus_target DEFAULT_CONFIG = os.path.join( @@ -392,16 +392,18 @@ def _validate_protocol(self, protocol: Dict[str, object], errors: List[str], dev if protocol_type in ("modbus_rtu", "modbus_tcp"): try: values = (probe.get("function_code", protocol.get("function_code", 3)), - probe.get("address", 0), probe.get("count", 1)) + probe.get("address", 0), probe.get("count", 1), + protocol.get("unit_id" if protocol_type == "modbus_tcp" else "slave_id", 1)) if any(isinstance(value, bool) or isinstance(value, float) and (not math.isfinite(value) or not value.is_integer()) for value in values): raise ValueError("Non-integral protocol parameter") - function_code, address, count = (int(value) for value in values) + function_code, address, count, target = (int(value) for value in values) except (TypeError, ValueError, OverflowError): errors.append(f"{device_id} 第{index}个Modbus规则参数必须是整数") continue try: validate_modbus_read(function_code, address, count) + validate_modbus_target(target, tcp=protocol_type == "modbus_tcp") except ProtocolConfigError as exc: errors.append(f"{device_id} 第{index}个Modbus规则: {exc}") operation = str(probe.get("operation") or protocol.get("operation") or "read").lower() diff --git a/core/generic_detector.py b/core/generic_detector.py index 7a935ad..d029807 100644 --- a/core/generic_detector.py +++ b/core/generic_detector.py @@ -839,7 +839,7 @@ def _test_modbus_rtu_configured(self, port: str, serial_cfg: dict, protocol: dic return {"ok": False, **test} def _configured_modbus_rtu_frames(self, protocol: dict) -> List[tuple]: - slave_id = int(protocol.get("slave_id", 1) or 1) + slave_id = int(protocol.get("slave_id", 1)) registers = protocol.get("probes", []) or protocol.get("test_registers", []) or [{"address": 0, "count": 1, "description": "默认只读寄存器"}] frames = [] default_function = int(protocol.get("function_code", 3) or 3) @@ -1202,7 +1202,7 @@ def _test_tcp_protocol_configured(self, ip: str, port: int, iface: str, def _test_modbus_tcp_configured(self, ip: str, port: int, iface: str, protocol: dict, profile: Optional[dict] = None) -> dict: profile = profile or self._network_profile({"protocol": protocol}, {"network_config": {"transport": "tcp", "device_role": "server"}}) - unit_id = int(protocol.get("unit_id", 1) or 1) + unit_id = int(protocol.get("unit_id", 1)) registers = protocol.get("probes", []) or protocol.get("test_registers", []) or [{"address": 0, "count": 1, "description": "默认只读寄存器"}] start = time.time() match_mode = str(protocol.get("match_mode") or "any").lower() @@ -2085,7 +2085,7 @@ def _probe_modbus_rtu(self, device: str) -> dict: def _modbus_rtu_probe_frames(self) -> List[tuple]: frames = [] for probe in self.config.protocol_probes("modbus_rtu"): - slave_id = int(probe.get("slave_id", 1) or 1) + slave_id = int(probe.get("slave_id", 1)) registers = probe.get("test_registers", []) or [{"address": 0, "count": 1, "description": "默认只读探测"}] for reg in registers: address = int(reg.get("address", 0) or 0) @@ -2335,7 +2335,7 @@ def _modbus_tcp_standard_probe(self, ip: str, port: int) -> Optional[dict]: }] transaction_id = 2 for probe in probes[:8]: - unit_id = int(probe.get("unit_id", 1) or 1) + unit_id = int(probe.get("unit_id", 1)) registers = probe.get("test_registers", []) or [{"address": 0, "count": 1, "description": "默认只读探测"}] for reg in registers[:2]: address = int(reg.get("address", 0) or 0) diff --git a/core/protocol_engine.py b/core/protocol_engine.py index d33db6f..2ace73d 100644 --- a/core/protocol_engine.py +++ b/core/protocol_engine.py @@ -194,10 +194,18 @@ def validate_modbus_read(function_code: int, address: int, count: int) -> None: raise ProtocolConfigError(f"Modbus读取地址/数量超出范围：功能码{function_code}数量须为1到{maximum}且不能跨越65535地址") +def validate_modbus_target(identifier: int, tcp: bool = False) -> None: + """Refuse target aliasing; RTU read probes require a unicast responder.""" + low, high = (0, 255) if tcp else (1, 247) + if type(identifier) is not int or not low <= identifier <= high: + raise ProtocolConfigError(f"Modbus设备标识必须是{low}到{high}之间的整数") + + def build_modbus_rtu_read(slave_id: int, function_code: int, address: int, count: int) -> bytes: + validate_modbus_target(slave_id) validate_modbus_read(function_code, address, count) payload = bytes([ - slave_id & 0xFF, + slave_id, function_code & 0xFF, (address >> 8) & 0xFF, address & 0xFF, @@ -209,10 +217,11 @@ def build_modbus_rtu_read(slave_id: int, function_code: int, address: int, count def find_modbus_rtu_response(data: bytes, slave_id: int, function_code: int, accept_exception: bool = False, expected_count: Optional[int] = None) -> Tuple[bool, str, bytes]: + validate_modbus_target(slave_id) if not data: return False, "未收到响应", b"" for start in range(max(0, len(data) - 4)): - if data[start] != (slave_id & 0xFF) or start + 4 >= len(data): + if data[start] != slave_id or start + 4 >= len(data): continue actual_function = data[start + 1] is_exception = actual_function == (function_code | 0x80) @@ -246,6 +255,7 @@ def find_modbus_rtu_response(data: bytes, slave_id: int, function_code: int, def build_modbus_tcp_read(transaction_id: int, unit_id: int, function_code: int, address: int, count: int) -> bytes: + validate_modbus_target(unit_id, tcp=True) validate_modbus_read(function_code, address, count) pdu = bytes([ function_code & 0xFF, @@ -258,7 +268,7 @@ def build_modbus_tcp_read(transaction_id: int, unit_id: int, function_code: int, transaction_id.to_bytes(2, "big") + b"\x00\x00" + (len(pdu) + 1).to_bytes(2, "big") - + bytes([unit_id & 0xFF]) + + bytes([unit_id]) + pdu ) @@ -266,6 +276,7 @@ def build_modbus_tcp_read(transaction_id: int, unit_id: int, function_code: int, def validate_modbus_tcp_response(data: bytes, transaction_id: int, unit_id: int, function_code: int, accept_exception: bool = False, expected_count: Optional[int] = None) -> Tuple[bool, str, bytes]: + validate_modbus_target(unit_id, tcp=True) if len(data) < 9: return False, "Modbus TCP响应长度不足", b"" if int.from_bytes(data[0:2], "big") != transaction_id or data[2:4] != b"\x00\x00": @@ -275,7 +286,7 @@ def validate_modbus_tcp_response(data: bytes, transaction_id: int, unit_id: int, if declared_length < 3 or declared_length > 254 or len(data) < frame_length: return False, "Modbus TCP长度字段不正确", b"" frame = data[:frame_length] - if frame[6] != (unit_id & 0xFF): + if frame[6] != unit_id: return False, "Modbus TCP单元标识不匹配", frame actual_function = frame[7] if actual_function == (function_code | 0x80): diff --git a/docs/modbus-targets.md b/docs/modbus-targets.md new file mode 100644 index 0000000..2a2380d --- /dev/null +++ b/docs/modbus-targets.md @@ -0,0 +1,15 @@ +# Modbus read target identity / Modbus 只读目标标识 + +Configured response-bearing RTU reads require `slave_id` in **1–247**. Address 0 is serial broadcast; this workbench does not perform broadcast read acceptance. Reserved addresses 248–255 are rejected for RTU reads. + +Modbus TCP `unit_id` supports **0–255**, including explicit numeric `0` and `255`. Omitting either target field still defaults to `1`. A configured value is never reduced modulo 256 or replaced merely because it is zero. + +RTU 只读验收使用 1–247 的单播站号；串口广播 0 与保留站号不能用于需要响应的验收。TCP 单元标识支持 0–255，明确配置的 0 保持为 0，字段缺失仍默认 1。 + +Configuration accepts existing numeric-string and exact integral-float forms, such as `"247"`, `247.0` and `"0"`. Booleans, fractional or nonfinite numbers, missing-type values such as null, and out-of-range targets are invalid. Pure byte encoders/response helpers require actual Python integers and raise `ProtocolConfigError` for invalid expected targets. Configuration validation returns an input error before an adapter runs. + +配置层保留整数形式字符串及整数值浮点数的兼容性，但拒绝布尔值、小数、非有限数、null 和越界值。底层字节 API 要求真正的整数。响应检查不会把错误目标掩码成另一个合法设备。 + +This is a compatibility correction for invalid targets and explicit TCP zero, not physical device certification. Raw custom request bytes still need operator review. Valid nonzero read packets keep their original encoding. + +Reference: [Modbus TCP/IP Implementation Guide V1.0b](https://www.modbus.org/file/secure/messagingimplementationguide.pdf), MBAP fields pp5–6 and target routing p23. Automated evidence uses pure packets, FakeSocket and loopback HTTP only; no hardware was contacted. Engineering and regression additions are AI-assisted. diff --git a/tests/test_modbus_identity.py b/tests/test_modbus_identity.py new file mode 100644 index 0000000..8c771aa --- /dev/null +++ b/tests/test_modbus_identity.py @@ -0,0 +1,125 @@ +"""Target identity regressions: pure bytes, fake sockets and loopback only.""" +import copy + +import pytest + +from core.config_manager import StandardDeviceConfig +from core.protocol_engine import ( + ProtocolConfigError, append_checksum, build_modbus_rtu_read, + build_modbus_tcp_read, find_modbus_rtu_response, validate_modbus_tcp_response, +) +from inspection.adapters import demo_config +from test_http import api + + +def configured(tcp, value): + config = demo_config() + config['devices'][int(tcp)]['protocol']['unit_id' if tcp else 'slave_id'] = value + return config + + +def build(tcp, value): + return build_modbus_tcp_read(1, value, 3, 0, 1) if tcp else build_modbus_rtu_read(value, 3, 0, 1) + + +def response(tcp, value): + if tcp: + return b'\x00\x01\x00\x00\x00\x05' + bytes([value, 3, 2, 0, 42]) + return append_checksum(bytes([value, 3, 2, 0, 42]), 'crc16_modbus') + + +def receive(tcp, packet, expected): + return (validate_modbus_tcp_response(packet, 1, expected, 3, expected_count=1) + if tcp else find_modbus_rtu_response(packet, expected, 3, expected_count=1)) + + +@pytest.mark.parametrize('tcp', [False, True], ids=['rtu', 'tcp']) +@pytest.mark.parametrize('value', [-1, 256, 257, True, False, 1.9, None, '1', float('nan'), float('inf')]) +def test_byte_encoder_rejects_invalid_target_instead_of_wrapping(tcp, value): + with pytest.raises(ProtocolConfigError): + build(tcp, value) + + +@pytest.mark.parametrize('tcp', [False, True], ids=['rtu', 'tcp']) +@pytest.mark.parametrize('value', [-1, 256, 257, True, False, 1.9, None, '1.9', float('nan'), float('inf')]) +def test_configuration_rejects_invalid_target_before_any_adapter(tcp, value): + with pytest.raises(ValueError): + StandardDeviceConfig(data=configured(tcp, value)) + + +@pytest.mark.parametrize('value', [0, 248, 255]) +def test_response_bearing_rtu_read_refuses_broadcast_and_reserved_targets(value): + with pytest.raises(ValueError): + StandardDeviceConfig(data=configured(False, value)) + with pytest.raises(ProtocolConfigError): + build(False, value) + + +@pytest.mark.parametrize('tcp', [False, True], ids=['rtu', 'tcp']) +@pytest.mark.parametrize('expected', [257, -255, True]) +def test_unit_one_response_never_satisfies_invalid_expected_identity(tcp, expected): + with pytest.raises(ProtocolConfigError): + receive(tcp, response(tcp, 1), expected) + + +@pytest.mark.parametrize('tcp', [False, True], ids=['rtu', 'tcp']) +def test_every_legal_target_roundtrips_without_aliasing(tcp): + targets = range(256) if tcp else range(1, 248) + for target in targets: + request = build(tcp, target) + assert request[6 if tcp else 0] == target + assert receive(tcp, response(tcp, target), target)[0] + other = (target + 1) % 256 if tcp else target % 247 + 1 + assert not receive(tcp, response(tcp, other), target)[0] + + +@pytest.mark.parametrize('tcp,value,target', [(False, '247', 247), (False, 247.0, 247), (True, '0', 0), (True, 255.0, 255)]) +def test_config_keeps_existing_integral_numeric_compatibility(tcp, value, target): + config = configured(tcp, value) + before = copy.deepcopy(config) + StandardDeviceConfig(data=config) + assert config == before + assert build(tcp, int(value))[6 if tcp else 0] == target + + +@pytest.mark.parametrize('path', ['configured', 'discovery']) +@pytest.mark.parametrize('unit', [0, 0.0]) +def test_fake_socket_flow_preserves_explicit_tcp_zero(monkeypatch, path, unit): + from core import generic_detector + sent = [] + + class FakeSocket: + def settimeout(self, timeout): + pass + def connect(self, endpoint): + assert endpoint == ('fixture.invalid', 1502) + def sendall(self, data): + self.request = data + sent.append(data) + def recv(self, size): + return self.request[:2] + b'\x00\x00\x00\x05\x00\x03\x02\x00\x2a' + def close(self): + pass + + monkeypatch.setattr(generic_detector.socket, 'socket', lambda *args: FakeSocket()) + detector = object.__new__(generic_detector.GenericDetector) + detector.config = StandardDeviceConfig(data=configured(True, unit)) + detector.connection_tests = [] + detector._bind_network_socket = lambda *args: None + detector._socket_interface = lambda *args: 'SIM-ETH0' + if path == 'configured': + protocol = detector.config.data['devices'][1]['protocol'] + result = detector._test_modbus_tcp_configured('fixture.invalid', 1502, 'SIM-ETH0', protocol) + assert result['ok'] + else: + assert detector._modbus_tcp_standard_probe('fixture.invalid', 1502) + assert sent and all(packet[6] == 0 for packet in sent) + + +@pytest.mark.parametrize('tcp', [False, True], ids=['rtu', 'tcp']) +def test_http_config_validation_rejects_target_alias(api, tcp): + service, server, request = api + status, body = request('POST', '/api/config/validate', {'config': configured(tcp, 257)}) + assert status == 400 and not body['ok'] + assert request('GET', '/api/health')[1]['persistence_ready'] + assert service.store.all() == []