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

Merge #564/#565: CIP codec hardening, EncapsulationClient gate/timeout bounds

Browse Source 
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 22:55:02 -04:00
parent bd2bebeda6 7892eb13a8
commit 13a6727033
15 changed files with 379 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/AbStatusCodesTests.cs
+4
View File
@@ -31,6 +31,10 @@ public sealed class AbStatusCodesTests
public void General_status_FF_maps_by_extended_status(ushort ext, AbStatus expected) =>
AbStatusCodes.FromCip(0xFF, [ext]).ShouldBe(expected);
[Fact]
public void General_status_FF_with_no_extended_status_is_BadReply() =>
AbStatusCodes.FromCip(0xFF, ReadOnlySpan<ushort>.Empty).ShouldBe(AbStatus.BadReply);
[Theory]
[InlineData((byte)0x00, null, AbStatus.Ok)]
[InlineData((byte)0x10, null, AbStatus.Unsupported)]
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Captures/CaptureFile.cs
+5 -4
View File
@@ -1,3 +1,4 @@
using System.Buffers.Binary;
namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Captures;
/// <summary>One EtherNet/IP encapsulation frame from a golden capture.</summary>
@@ -30,15 +31,15 @@ internal static class CaptureFile
public static byte[] CipPayload(CapturedFrame f)
{
var b = f.Bytes;
var cmd = BitConverter.ToUInt16(b, 0); // test-only; fixtures are LE on a LE host
var cmd = BinaryPrimitives.ReadUInt16LittleEndian(b.AsSpan(0));
if (cmd is not (0x6F or 0x70)) return []; // only SendRRData / SendUnitData carry CPF items
var pos = 24 + 4 + 2; // header + interface handle + timeout
var itemCount = BitConverter.ToUInt16(b, pos); pos += 2;
var itemCount = BinaryPrimitives.ReadUInt16LittleEndian(b.AsSpan(pos)); pos += 2;
byte[] last = [];
for (var i = 0; i < itemCount; i++)
{
var type = BitConverter.ToUInt16(b, pos);
var len = BitConverter.ToUInt16(b, pos + 2);
var type = BinaryPrimitives.ReadUInt16LittleEndian(b.AsSpan(pos));
var len = BinaryPrimitives.ReadUInt16LittleEndian(b.AsSpan(pos + 2));
last = b.AsSpan(pos + 4, len).ToArray();
if (type == 0x00B1) last = last[2..]; // strip connected sequence count
pos += 4 + len;
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Captures/CaptureFileTests.cs
+5 -4
View File
@@ -1,3 +1,4 @@
using System.Buffers.Binary;
using Shouldly;
using Xunit;
@@ -36,7 +37,7 @@ public sealed class CaptureFileTests
foreach (var f in frames)
{
f.Bytes.Length.ShouldBeGreaterThanOrEqualTo(24, name);
BitConverter.ToUInt16(f.Bytes, 2).ShouldBe((ushort)(f.Bytes.Length - 24), name);
BinaryPrimitives.ReadUInt16LittleEndian(f.Bytes.AsSpan(2)).ShouldBe((ushort)(f.Bytes.Length - 24), name);
}
}
@@ -54,12 +55,12 @@ public sealed class CaptureFileTests
[MemberData(nameof(AllCaptures))]
public void Every_CPF_item_walk_ends_exactly_at_the_frame_end(string name)
{
foreach (var f in CaptureFile.Load(name).Where(f => BitConverter.ToUInt16(f.Bytes, 0) is 0x6F or 0x70))
foreach (var f in CaptureFile.Load(name).Where(f => BinaryPrimitives.ReadUInt16LittleEndian(f.Bytes.AsSpan(0)) is 0x6F or 0x70))
{
var pos = 24 + 4 + 2;
var itemCount = BitConverter.ToUInt16(f.Bytes, pos);
var itemCount = BinaryPrimitives.ReadUInt16LittleEndian(f.Bytes.AsSpan(pos));
pos += 2;
for (var i = 0; i < itemCount; i++) pos += 4 + BitConverter.ToUInt16(f.Bytes, pos + 2);
for (var i = 0; i < itemCount; i++) pos += 4 + BinaryPrimitives.ReadUInt16LittleEndian(f.Bytes.AsSpan(pos + 2));
pos.ShouldBe(f.Bytes.Length, name);
}
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/CipPathTests.cs
+59
View File
@@ -49,6 +49,65 @@ public sealed class CipPathTests
public void Encodes_class_instance(ushort cls, uint inst, string hex) =>
Convert.ToHexString(CipPath.EncodeClassInstance(cls, inst)).ShouldBe(hex);
[Theory]
[InlineData("-1,0")] // negative port
[InlineData("65536,0")] // port does not fit a u16
[InlineData("x,0")] // non-numeric port
[InlineData("1,é.example")] // non-ASCII link
public void Malformed_route_throws_ArgumentException(string path) =>
Should.Throw<ArgumentException>(() => CipPath.EncodeRoute(path));
[Fact]
public void Route_link_over_255_characters_throws()
{
Should.Throw<ArgumentException>(() => CipPath.EncodeRoute("1," + new string('a', 256)));
CipPath.EncodeRoute("1," + new string('a', 255)).Length.ShouldBe(2 + 255 + 1);
}
[Fact]
public void Route_over_255_words_throws() =>
Should.Throw<ArgumentException>(() => CipPath.EncodeRoute(string.Join(",", Enumerable.Repeat("1,0", 256))));
[Theory]
[InlineData("")]
[InlineData("A..B")] // empty member
[InlineData(".A")]
[InlineData("A[1")] // unterminated subscript
[InlineData("A[x]")] // non-numeric subscript
[InlineData("A[-1]")]
[InlineData("A[]")]
[InlineData("A[1]B")] // junk after the subscript
[InlineData("Tä")] // non-ASCII
[InlineData("A[99999999999]")] // subscript overflows u32
[InlineData("Tag.99999999999")] // bit selector overflows int
public void Malformed_tag_names_throw_ArgumentException(string name) =>
Should.Throw<ArgumentException>(() => CipPath.EncodeSymbolic(name));
[Fact]
public void Tag_member_over_255_characters_throws()
{
Should.Throw<ArgumentException>(() => CipPath.EncodeSymbolic(new string('a', 256)));
CipPath.EncodeSymbolic(new string('a', 255)).Length.ShouldBe(2 + 255 + 1);
}
[Theory]
[InlineData(1)] // odd
[InlineData(3)]
[InlineData(512)] // 256 words: size byte would wrap
public void WithPathSize_rejects_odd_and_oversize_paths(int length) =>
Should.Throw<ArgumentException>(() => CipPath.WithPathSize(new byte[length]));
[Fact]
public void WithPathSize_accepts_the_255_word_maximum() =>
CipPath.WithPathSize(new byte[510])[0].ShouldBe((byte)255);
[Fact]
public void Well_known_paths_are_immutable_spans()
{
CipPath.MessageRouter.ToArray().ShouldBe(new byte[] { 0x20, 0x02, 0x24, 0x01 });
CipPath.ConnectionManager.ToArray().ShouldBe(new byte[] { 0x20, 0x06, 0x24, 0x01 });
}
[Fact]
public void Prefixes_path_size_in_words() =>
Convert.ToHexString(CipPath.WithPathSize([0x20, 0x02, 0x24, 0x01])).ShouldBe("0220022401");
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/ConnectionManagerServicesTests.cs
+19
View File
@@ -105,6 +105,25 @@ public sealed class ConnectionManagerServicesTests
ex.Message.ShouldContain("0x0109");
}
[Fact]
public void Refused_forward_open_carries_the_statuses_as_structured_data()
{
var reply = CipReply.Parse(Convert.FromHexString("DB000102" + "09010203"));
var ex = Should.Throw<ForwardOpenRefusedException>(() => ConnectionManagerServices.ParseForwardOpenReply(reply));
ex.GeneralStatus.ShouldBe((byte)0x01);
ex.ExtendedStatus.ShouldBe(new ushort[] { 0x0109, 0x0302 });
ex.Status.ShouldBe(AbStatus.BadConnection); // general 0x01 has no specific mapping
}
[Fact]
public void Refused_forward_open_exception_is_not_aliased_to_the_reply_array()
{
var reply = CipReply.Parse(Convert.FromHexString("DB000101" + "0901"));
var ex = Should.Throw<ForwardOpenRefusedException>(() => ConnectionManagerServices.ParseForwardOpenReply(reply));
reply.ExtendedStatus[0] = 0xFFFF;
ex.ExtendedStatus[0].ShouldBe((ushort)0x0109);
}
[Theory]
[InlineData("DB000800", true)] // Service Not Supported
[InlineData("DB0001010901", true)] // general 1 / extended 0x0109
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Eip/EncapsulationClientTests.cs
+104 -4
View File
@@ -151,13 +151,15 @@ public sealed class EncapsulationClientTests
using var callerCts = new CancellationTokenSource();
var first = client.SendRRDataAsync(new byte[] { 0x01 }, TimeSpan.FromMilliseconds(300), callerCts.Token);
await server.WaitForFrameAsync(IsRR, Guard);
// Both calls are issued back-to-back, before any await: `first` acquired the gate synchronously
// and `next` is therefore already queued behind it before the cancel — the ordering does not
// depend on how long the test thread takes to get around to it.
var next = client.SendRRDataAsync(new byte[] { 0x02 }, Generous, TestCt);
await callerCts.CancelAsync();
await Should.ThrowAsync<OperationCanceledException>(first.WaitAsync(Guard, TestCt));
client.IsConnected.ShouldBeTrue(); // cancel alone never faults
await server.WaitForFrameAsync(IsRR, Guard); // the abandoned request was on the wire
// Queued behind the draining exchange: must fail fast once that exchange times out, not hang.
var next = client.SendRRDataAsync(new byte[] { 0x02 }, Generous, TestCt);
var ex = await Should.ThrowAsync<AbTransportException>(next.WaitAsync(Guard, TestCt));
ex.Status.ShouldBe(AbStatus.BadConnection);
@@ -168,6 +170,104 @@ public sealed class EncapsulationClientTests
.Status.ShouldBe(AbStatus.BadConnection);
}
[Fact]
public async Task RegisterSession_reply_matches_only_through_the_zero_context_tolerance()
{
// Documents a dependency: RegisterSession goes out with sender context 0 (ab_server refuses
// non-zero) so its reply echoes 0, while the exchange's own context is a non-zero counter value.
// The only thing that lets the reply match is TryMatch's "reply context == 0" rule; tighten
// that rule and every Connect breaks, not just the zero-context-server case.
await using var server = new FakeEipServer();
await using var client = await Connect(server);
var request = EncapsulationCodec.ReadHeader(server.Received[0]);
request.Command.ShouldBe(EncapCommand.RegisterSession);
request.SenderContext.ShouldBe(0UL);
client.SessionHandle.ShouldBe(FakeEipServer.Session);
client.IsConnected.ShouldBeTrue();
}
[Theory]
[InlineData(0)]
[InlineData(-5)]
public async Task Non_positive_timeouts_are_rejected_everywhere(int milliseconds)
{
await using var server = new FakeEipServer();
var bad = TimeSpan.FromMilliseconds(milliseconds);
await Should.ThrowAsync<ArgumentOutOfRangeException>(
EncapsulationClient.ConnectAsync("127.0.0.1", server.Port, bad, TestCt));
await using var client = await Connect(server);
await Should.ThrowAsync<ArgumentOutOfRangeException>(client.SendRRDataAsync(new byte[] { 0x4C }, bad, TestCt));
await Should.ThrowAsync<ArgumentOutOfRangeException>(client.SendUnitDataAsync(1, new byte[] { 0x4C }, bad, TestCt));
client.IsConnected.ShouldBeTrue(); // a bad argument is not a comms fault
server.Received.Count(IsRR).ShouldBe(0);
}
[Fact]
public async Task Infinite_timeouts_are_rejected_everywhere()
{
await using var server = new FakeEipServer();
var inf = Timeout.InfiniteTimeSpan;
await Should.ThrowAsync<ArgumentOutOfRangeException>(
EncapsulationClient.ConnectAsync("127.0.0.1", server.Port, inf, TestCt));
await using var client = await Connect(server);
await Should.ThrowAsync<ArgumentOutOfRangeException>(client.SendRRDataAsync(new byte[] { 0x4C }, inf, TestCt));
await Should.ThrowAsync<ArgumentOutOfRangeException>(client.SendUnitDataAsync(1, new byte[] { 0x4C }, inf, TestCt));
client.IsConnected.ShouldBeTrue();
}
[Fact]
public async Task Waiting_for_the_gate_is_bounded_by_the_timeout_and_leaves_the_connection_intact()
{
await using var server = new FakeEipServer { Stall = true };
var client = await Connect(server);
var holder = client.SendRRDataAsync(new byte[] { 0x01 }, Generous, TestCt); // holds the gate: no reply comes
await server.WaitForFrameAsync(IsRR, Guard);
var ex = await Should.ThrowAsync<AbTransportException>(
client.SendRRDataAsync(new byte[] { 0x02 }, TimeSpan.FromMilliseconds(150), TestCt).WaitAsync(Guard, TestCt));
ex.Status.ShouldBe(AbStatus.Timeout);
client.IsConnected.ShouldBeTrue(); // the waiter wrote nothing, so nothing desynced
holder.IsCompleted.ShouldBeFalse(); // positive control: the holder is still waiting on its reply
server.Received.Count(IsRR).ShouldBe(1);
await client.DisposeAsync(); // closes the socket under the holder
(await Should.ThrowAsync<AbTransportException>(holder.WaitAsync(Guard, TestCt)))
.Status.ShouldBe(AbStatus.BadConnection);
}
[Theory]
[InlineData(false)] // drain outlives the unregister budget: the socket is closed under it, nothing is written
[InlineData(true)] // drain times out while Dispose waits for the gate: the session is faulted, nothing is written
public async Task Dispose_during_a_cancelled_reply_drain_closes_without_interleaving_an_UnRegisterSession(bool drainTimesOutFirst)
{
await using var server = new FakeEipServer { Stall = true };
var client = await Connect(server);
using var callerCts = new CancellationTokenSource();
var first = client.SendRRDataAsync(new byte[] { 0x01 },
drainTimesOutFirst ? TimeSpan.FromMilliseconds(100) : Generous, callerCts.Token);
await server.WaitForFrameAsync(IsRR, Guard);
await callerCts.CancelAsync();
await Should.ThrowAsync<OperationCanceledException>(first.WaitAsync(Guard, TestCt));
await client.DisposeAsync().AsTask().WaitAsync(Guard, TestCt); // must not hang on the drain
client.IsConnected.ShouldBeFalse();
(await Should.ThrowAsync<AbTransportException>(client.SendRRDataAsync(new byte[] { 0x03 }, Generous, TestCt)))
.Status.ShouldBe(AbStatus.BadConnection);
// Absence needs a positive control: a second client's clean dispose MUST put an UnRegisterSession
// on the wire, so by the time it arrives the first client's (had there been one) would have too.
var second = await Connect(server);
await second.DisposeAsync();
await server.WaitForFrameAsync(f => FakeEipServer.Command(f) == EncapCommand.UnRegisterSession, Guard);
server.Received.Count(f => FakeEipServer.Command(f) == EncapCommand.UnRegisterSession).ShouldBe(1);
server.Received.Count(IsRR).ShouldBe(1); // and the queued/abandoned work never doubled
}
[Fact]
public async Task An_abandoned_exchange_that_times_out_leaves_no_unobserved_task_exception()
{
@@ -228,7 +328,7 @@ public sealed class EncapsulationClientTests
var units = server.Received.Where(f => FakeEipServer.Command(f) == EncapCommand.SendUnitData).ToList();
var sequences = units.Select(FakeEipServer.RequestSequence).ToList();
sequences.ShouldBe(Enumerable.Range(1, 20).Select(i => (ushort)i).ToList()); // strictly increasing, no dupes
units.Select(f => BitConverter.ToUInt32(f, 36)).ShouldAllBe(id => id == 0xAABBCCDD); // A1 item connection id
units.Select(f => BinaryPrimitives.ReadUInt32LittleEndian(f.AsSpan(36))).ShouldAllBe(id => id == 0xAABBCCDD); // A1 item connection id
for (var i = 0; i < 20; i++) results[i].ToArray().ShouldBe(new[] { (byte)(i + 1) }); // each got its own reply
server.MaxOutstanding.ShouldBe(1);
}
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Eip/EncapsulationCodecTests.cs
+2 -1
View File
@@ -1,3 +1,4 @@
using System.Buffers.Binary;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Eip;
@@ -72,7 +73,7 @@ public sealed class EncapsulationCodecTests
var captured = CaptureFile.Load("logix-read-dint.hex").First(f => f.FromClient && f.Bytes[0] == 0x70).Bytes;
var h = EncapsulationCodec.ReadHeader(captured);
var parsed = EncapsulationCodec.ParseReply(captured, expectConnected: true);
var connId = BitConverter.ToUInt32(captured, 24 + 8 + 4); // header, iface+timeout+count, A1 type+len
var connId = BinaryPrimitives.ReadUInt32LittleEndian(captured.AsSpan(24 + 8 + 4)); // header, iface+timeout+count, A1 type+len
var ours = EncapsulationCodec.SendUnitData(h.SessionHandle, h.SenderContext, connId, parsed.Sequence, parsed.Cip.Span);
ours.Length.ShouldBe(captured.Length);
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Wire/CipReplyTests.cs
+19
View File
@@ -34,6 +34,25 @@ public sealed class CipReplyTests
r.Status.ShouldBe(AbStatus.OutOfBounds);
}
[Fact]
public void Replies_parsed_from_the_same_bytes_are_equal()
{
byte[] raw = [0xCC, 0x00, 0xFF, 0x02, 0x05, 0x21, 0x00, 0x00, 0xAA];
// Distinct arrays on both sides: reference equality (the record default) would say unequal.
var a = CipReply.Parse(raw.ToArray());
var b = CipReply.Parse(raw.ToArray());
a.ShouldBe(b);
a.GetHashCode().ShouldBe(b.GetHashCode());
}
[Fact]
public void Replies_differing_in_extended_status_or_data_are_not_equal()
{
var a = CipReply.Parse(new byte[] { 0xCC, 0x00, 0xFF, 0x01, 0x07, 0x21, 0x01 });
a.ShouldNotBe(CipReply.Parse(new byte[] { 0xCC, 0x00, 0xFF, 0x01, 0x05, 0x21, 0x01 }));
a.ShouldNotBe(CipReply.Parse(new byte[] { 0xCC, 0x00, 0xFF, 0x01, 0x07, 0x21, 0x02 }));
}
[Fact]
public void Truncated_reply_throws_BadReply()
{
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Wire/CipWriterTests.cs
+27
View File
@@ -0,0 +1,27 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Wire;
namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Wire;
public sealed class CipWriterTests
{
[Fact]
public void PatchU16_overwrites_an_already_written_word()
{
var w = new CipWriter().U16(0).U8(7);
w.PatchU16(0, 0xABCD);
w.ToArray().ShouldBe(new byte[] { 0xCD, 0xAB, 0x07 });
}
[Theory]
[InlineData(-1)]
[InlineData(2)] // would straddle the end: only bytes 0..2 are written
[InlineData(3)]
[InlineData(100)] // inside spare buffer capacity (256), but never written
public void PatchU16_outside_the_written_range_throws(int offset)
{
var w = new CipWriter().U16(0).U8(7);
Should.Throw<ArgumentOutOfRangeException>(() => w.PatchU16(offset, 1));
w.ToArray().ShouldBe(new byte[] { 0, 0, 7 }); // nothing was patched
}
}