dohertj2/lmxopcua
1
0
Fork 0
Code Issues 12 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(modbus-rtu): validate response unit + cover partial-read/truncation in ModbusRtuFraming

Browse Source 
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-24 13:42:25 -04:00
parent eed6617784
commit 20100c36ad
2 changed files with 98 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs
+26 -9
View File
@@ -35,6 +35,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus;
/// a CRC-valid exception PDU throws <see cref="ModbusException"/> carrying the original
/// function code (<c>fc &amp; 0x7F</c>) and exception code.
/// </para>
/// <para>
/// 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 <b>wrong</b> slave would otherwise be
/// silently accepted as the addressed unit's data; such a frame is a unit-mismatch
/// <see cref="ModbusTransportDesyncException"/>. 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.
/// </para>
/// </remarks>
public static class ModbusRtuFraming
{
@@ -59,13 +67,14 @@ public static class ModbusRtuFraming
/// </summary>
/// <param name="stream">The byte stream to read the response from.</param>
/// <param name="expectedUnit">
/// 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.
/// </param>
/// <param name="ct">A token to cancel the read.</param>
/// <returns>The bare response PDU (function code followed by its data).</returns>
/// <exception cref="ModbusTransportDesyncException">
/// 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 <paramref name="expectedUnit"/>.
/// </exception>
/// <exception cref="ModbusException">
/// 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<byte[]> 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<byte>.Empty, tail);
return ValidateAndStrip(expectedUnit, header, ReadOnlySpan<byte>.Empty, tail);
}
/// <summary>
/// Assembles the full frame from its pieces, validates the trailing CRC over everything
/// except the CRC bytes, then strips <c>addr</c> + <c>CRC</c> to return the bare PDU.
/// except the CRC bytes, then validates the response address against
/// <paramref name="expectedUnit"/> and strips <c>addr</c> + <c>CRC</c> to return the bare PDU.
/// A CRC-valid exception PDU throws <see cref="ModbusException"/>.
/// </summary>
private static byte[] ValidateAndStrip(
ReadOnlySpan<byte> header, ReadOnlySpan<byte> middle, ReadOnlySpan<byte> tail)
byte expectedUnit, ReadOnlySpan<byte> header, ReadOnlySpan<byte> middle, ReadOnlySpan<byte> 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();
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs
+72
View File
@@ -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<ModbusTransportDesyncException>(
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<ModbusTransportDesyncException>(
ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken));
}
/// <summary>
/// A read-only test double that dribbles its backing bytes out at most <c>chunk</c> at a time
/// per <see cref="ReadAsync(Memory{byte}, CancellationToken)"/> call, then returns 0 (EOF)
/// once exhausted. Exercises the partial-read accumulation loop and the truncation → desync
/// path that a single-shot <see cref="MemoryStream"/> never reaches.
/// </summary>
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<int> ReadAsync(Memory<byte> 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();
}
}