diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs index e3ab9b00..5f8b2b35 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs @@ -35,6 +35,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; /// a CRC-valid exception PDU throws carrying the original /// function code (fc & 0x7F) and exception code. /// +/// +/// Once the CRC passes, the response's address byte is validated against the addressed unit. +/// On an RS-485 multi-drop bus a CRC-valid reply from the wrong slave would otherwise be +/// silently accepted as the addressed unit's data; such a frame is a unit-mismatch +/// . This ordering is deliberate — a corrupt frame +/// stays a CRC/desync failure, and only a coherent-but-mis-addressed frame becomes a +/// unit-mismatch desync. +/// /// public static class ModbusRtuFraming { @@ -59,13 +67,14 @@ public static class ModbusRtuFraming /// /// The byte stream to read the response from. /// - /// The unit id the request was addressed to. Read from the wire and consumed when stripping - /// to the bare PDU; no mismatch validation is performed here. + /// The unit id the request was addressed to. The response's address byte is validated against + /// it after the CRC passes; a mismatch is a unit-mismatch desync. /// /// A token to cancel the read. /// The bare response PDU (function code followed by its data). /// - /// The frame was truncated (stream closed mid-frame) or its trailing CRC did not validate. + /// The frame was truncated (stream closed mid-frame), its trailing CRC did not validate, or its + /// address byte did not match . /// /// /// The frame was a CRC-valid Modbus exception PDU (high bit set on the function code). @@ -73,8 +82,6 @@ public static class ModbusRtuFraming public static async Task ReadResponsePduAsync( Stream stream, byte expectedUnit, CancellationToken ct) { - _ = expectedUnit; // read off the wire below; no mismatch validation is specified here. - // Header: addr(1) + fc(1). The function code selects how many more bytes to read. var header = new byte[2]; await ReadExactlyAsync(stream, header, ct).ConfigureAwait(false); @@ -94,7 +101,7 @@ public static class ModbusRtuFraming var byteCount = bc[0]; var rest = new byte[byteCount + 2]; await ReadExactlyAsync(stream, rest, ct).ConfigureAwait(false); - return ValidateAndStrip(header, bc, rest); + return ValidateAndStrip(expectedUnit, header, bc, rest); } else { @@ -105,16 +112,17 @@ public static class ModbusRtuFraming var tail = new byte[trailing]; await ReadExactlyAsync(stream, tail, ct).ConfigureAwait(false); - return ValidateAndStrip(header, ReadOnlySpan.Empty, tail); + return ValidateAndStrip(expectedUnit, header, ReadOnlySpan.Empty, tail); } /// /// Assembles the full frame from its pieces, validates the trailing CRC over everything - /// except the CRC bytes, then strips addr + CRC to return the bare PDU. + /// except the CRC bytes, then validates the response address against + /// and strips addr + CRC to return the bare PDU. /// A CRC-valid exception PDU throws . /// private static byte[] ValidateAndStrip( - ReadOnlySpan header, ReadOnlySpan middle, ReadOnlySpan tail) + byte expectedUnit, ReadOnlySpan header, ReadOnlySpan middle, ReadOnlySpan tail) { // Reassemble the whole frame: header(addr,fc) + middle(optional byteCount) + tail. var frame = new byte[header.Length + middle.Length + tail.Length]; @@ -131,6 +139,15 @@ public static class ModbusRtuFraming ModbusTransportDesyncException.DesyncReason.TruncatedFrame, $"Modbus RTU CRC mismatch: computed {expected:X4} got {actual:X4}"); + // Unit-address validation runs only AFTER the CRC passes: a corrupt frame is a CRC/desync + // failure, and only a coherent-but-mis-addressed frame (a CRC-valid reply from the wrong + // RS-485 slave) is a unit-mismatch desync. Never silently accept another unit's data. + var addr = frame[0]; + if (addr != expectedUnit) + throw new ModbusTransportDesyncException( + ModbusTransportDesyncException.DesyncReason.TruncatedFrame, + $"Modbus RTU unit mismatch: expected {expectedUnit} got {addr}"); + // Bare PDU = frame minus leading addr and trailing CRC. var pdu = body[1..].ToArray(); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs index 6e00614d..44bbb43f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs @@ -56,4 +56,76 @@ public sealed class ModbusRtuFramingTests var pdu = await ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken); pdu.ShouldBe(new byte[] { 0x06, 0x00, 0x07, 0x00, 0x2A }); } + + [Fact] + public async Task ReadResponse_wrong_unit_address_throws_desync() + { + // CRC-valid FC03 frame addressed to unit 0x02, but we asked for 0x01 -> unit-mismatch desync. + var frame = ModbusCrc.Append(new byte[] { 0x02, 0x03, 0x02, 0x00, 0x0A }); + await using var s = Canned(frame); + await Should.ThrowAsync( + ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task ReadResponse_FC03_deframes_correctly_when_delivered_in_dribbles() + { + // Same valid FC03 frame, but the stream hands back only 1-2 bytes per ReadAsync — proves + // the ReadExactlyAsync accumulation loop reassembles a length-less frame across many reads. + var frame = ModbusCrc.Append(new byte[] { 0x01, 0x03, 0x02, 0x00, 0x0A }); + await using var s = new DribbleStream(frame, chunk: 2); + var pdu = await ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken); + pdu.ShouldBe(new byte[] { 0x03, 0x02, 0x00, 0x0A }); + } + + [Fact] + public async Task ReadResponse_stream_ends_early_throws_desync() + { + // Only the first 4 bytes of a 7-byte FC03 frame are available, then EOF (ReadAsync returns 0) + // -> the truncation path in ReadExactlyAsync must surface a desync, not hang or mis-parse. + var frame = ModbusCrc.Append(new byte[] { 0x01, 0x03, 0x02, 0x00, 0x0A }); + var truncated = frame[..4]; + await using var s = new DribbleStream(truncated, chunk: 2); + await Should.ThrowAsync( + ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken)); + } + + /// + /// A read-only test double that dribbles its backing bytes out at most chunk at a time + /// per call, then returns 0 (EOF) + /// once exhausted. Exercises the partial-read accumulation loop and the truncation → desync + /// path that a single-shot never reaches. + /// + private sealed class DribbleStream(byte[] data, int chunk) : Stream + { + private int _pos; + + public override int Read(byte[] buffer, int offset, int count) + { + var n = Math.Min(Math.Min(chunk, count), data.Length - _pos); + if (n <= 0) return 0; + Array.Copy(data, _pos, buffer, offset, n); + _pos += n; + return n; + } + + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + var n = Math.Min(Math.Min(chunk, buffer.Length), data.Length - _pos); + if (n <= 0) return ValueTask.FromResult(0); + data.AsSpan(_pos, n).CopyTo(buffer.Span); + _pos += n; + return ValueTask.FromResult(n); + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => data.Length; + public override long Position { get => _pos; set => throw new NotSupportedException(); } + public override void Flush() { } + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } }