diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionManagerServices.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionManagerServices.cs index db6156a4..50dd65b0 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionManagerServices.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ConnectionManagerServices.cs @@ -29,6 +29,9 @@ public static class ConnectionManagerServices public static byte[] ForwardOpen(ForwardOpenParams p) { + if (p.ConnectionSize < 1 || p.ConnectionSize > (p.Large ? 0xFFFF : 0x1FF)) + throw new ArgumentOutOfRangeException(nameof(p), p.ConnectionSize, + p.Large ? "Large connection size must be 1..65535." : "Regular connection size must be 1..511."); var connPath = new CipWriter().Bytes(p.Route).Bytes(CipPath.MessageRouter).ToArray(); var w = new CipWriter().U8(p.Large ? (byte)0x5B : (byte)0x54).U8(2).Bytes(CipPath.ConnectionManager) .U8(PriorityTick).U8(TimeoutTicks) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/LogixServices.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/LogixServices.cs index 0637cf56..85dee883 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/LogixServices.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/LogixServices.cs @@ -29,6 +29,8 @@ public static class LogixServices public static (ushort TypeCode, ushort? StructHandle, ReadOnlyMemory Data) ParseReadReply(CipReply reply) { + if (reply.GeneralStatus is not (0x00 or 0x06)) + throw new AbTransportException(reply.Status, $"Read failed: general 0x{reply.GeneralStatus:X2}."); var r = new CipReader(reply.Data.Span); var type = r.U16(); ushort? handle = type == LogixTypeCode.Struct ? r.U16() : null; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs index dead6319..dd1f853e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs @@ -16,7 +16,10 @@ public static class SymbolObjectServices var a = Encoding.ASCII.GetBytes(programName.StartsWith("Program:", StringComparison.Ordinal) ? programName : "Program:" + programName); path.U8(0x91).U8((byte)a.Length).Bytes(a).PadToEven(); } - path.U8(0x20).U8(0x6B).U8(0x25).U8(0).U16((ushort)startInstance); + path.U8(0x20).U8(0x6B); + // 16-bit instance segment even for small values (libplctag's form, see capture); 32-bit above 0xFFFF. + if (startInstance <= 0xFFFF) path.U8(0x25).U8(0).U16((ushort)startInstance); + else path.U8(0x26).U8(0).U32(startInstance); var p = path.ToArray(); return new CipWriter().U8(0x55).U8((byte)(p.Length / 2)).Bytes(p) .U16(4).U16(2).U16(7).U16(8).U16(1).ToArray(); @@ -39,6 +42,9 @@ public static class SymbolObjectServices var nameLen = r.U16(); r.Bytes(nameLen); } - return (span.ToArray(), last, reply.GeneralStatus == 0x06); + var more = reply.GeneralStatus == 0x06; + if (more && span.IsEmpty) + throw new AbTransportException(AbStatus.BadReply, "Symbol listing reported more data but returned no entries."); + return (span.ToArray(), last, more); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/ConnectionManagerServicesTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/ConnectionManagerServicesTests.cs index ad8766dc..b8dcd1bf 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/ConnectionManagerServicesTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/ConnectionManagerServicesTests.cs @@ -1,3 +1,4 @@ +using System.Buffers.Binary; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip; @@ -26,9 +27,9 @@ public sealed class ConnectionManagerServicesTests public void Forward_close_matches_capture() { var cap = CapturedRequest("logix-forward-close.hex"); - var serial = BitConverter.ToUInt16(cap, 8); - var vendor = BitConverter.ToUInt16(cap, 10); - var orig = BitConverter.ToUInt32(cap, 12); + var serial = BinaryPrimitives.ReadUInt16LittleEndian(cap.AsSpan(8)); + var vendor = BinaryPrimitives.ReadUInt16LittleEndian(cap.AsSpan(10)); + var orig = BinaryPrimitives.ReadUInt32LittleEndian(cap.AsSpan(12)); vendor.ShouldBe((ushort)0xF33D); ConnectionManagerServices.ForwardClose(serial, vendor, orig, [0x01, 0x00]).ShouldBe(cap); } @@ -40,11 +41,11 @@ public sealed class ConnectionManagerServicesTests // Values read out of the capture: T->O id @12, serial @16, vendor @18, originator serial @20, // timeout multiplier @24, RPI @28, network params @32 (0x42000FA2 -> size 4002). var p = new ForwardOpenParams( - Large: true, TOConnectionId: BitConverter.ToUInt32(cap, 12), ConnectionSerial: BitConverter.ToUInt16(cap, 16), - VendorId: BitConverter.ToUInt16(cap, 18), OriginatorSerial: BitConverter.ToUInt32(cap, 20), - TimeoutMultiplier: cap[24], RpiMicros: BitConverter.ToUInt32(cap, 28), ConnectionSize: 4002, Route: [0x01, 0x00]); + Large: true, TOConnectionId: BinaryPrimitives.ReadUInt32LittleEndian(cap.AsSpan(12)), ConnectionSerial: BinaryPrimitives.ReadUInt16LittleEndian(cap.AsSpan(16)), + VendorId: BinaryPrimitives.ReadUInt16LittleEndian(cap.AsSpan(18)), OriginatorSerial: BinaryPrimitives.ReadUInt32LittleEndian(cap.AsSpan(20)), + TimeoutMultiplier: cap[24], RpiMicros: BinaryPrimitives.ReadUInt32LittleEndian(cap.AsSpan(28)), ConnectionSize: 4002, Route: [0x01, 0x00]); p.RpiMicros.ShouldBe(1_000_000u); - BitConverter.ToUInt32(cap, 32).ShouldBe(0x42000FA2u); + BinaryPrimitives.ReadUInt32LittleEndian(cap.AsSpan(32)).ShouldBe(0x42000FA2u); ConnectionManagerServices.ForwardOpen(p).ShouldBe(cap); } @@ -111,4 +112,15 @@ public sealed class ConnectionManagerServicesTests [InlineData("DB000000", false)] public void Fallback_rules(string hex, bool expected) => ConnectionManagerServices.ShouldFallBackToRegularOpen(CipReply.Parse(Convert.FromHexString(hex))).ShouldBe(expected); + + [Theory] + [InlineData(true, 0)] [InlineData(true, 0x10000)] [InlineData(false, 0)] [InlineData(false, 0x200)] + public void Connection_size_out_of_range_throws(bool large, int size) => + Should.Throw(() => ConnectionManagerServices.ForwardOpen( + new ForwardOpenParams(large, 1, 2, 3, 4, 1, 1_000_000, size, []))); + + [Theory] + [InlineData(true, 1)] [InlineData(true, 0xFFFF)] [InlineData(false, 1)] [InlineData(false, 0x1FF)] + public void Connection_size_bounds_accepted(bool large, int size) => + ConnectionManagerServices.ForwardOpen(new ForwardOpenParams(large, 1, 2, 3, 4, 1, 1_000_000, size, [])).ShouldNotBeEmpty(); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/LogixServicesTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/LogixServicesTests.cs index 7eb3c8ed..7bd51dd5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/LogixServicesTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/LogixServicesTests.cs @@ -1,4 +1,5 @@ using System.Text; +using System.Buffers.Binary; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip; @@ -11,6 +12,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Logix; // against ReadTagFragmented(path, 1, 0). Replies are service 0xD2 (0x52|0x80). public sealed class LogixServicesTests { + private static byte[] Int32Le(int v) { var a = new byte[4]; BinaryPrimitives.WriteInt32LittleEndian(a, v); return a; } + + [Fact] + public void Error_read_reply_surfaces_status_from_capture() => + Should.Throw(() => LogixServices.ParseReadReply(Rep("logix-read-unknown-tag.hex"))) + .Status.ShouldBe(AbStatus.NotFound); + private static byte[] Req(string file, int i = 0) => CaptureFile.CipPayload(CaptureFile.Load(file).Where(f => f.FromClient).ElementAt(i)); @@ -32,7 +40,7 @@ public sealed class LogixServicesTests type.ShouldBe(LogixTypeCode.DInt); handle.ShouldBeNull(); data.Length.ShouldBe(4); - BitConverter.ToInt32(data.Span).ShouldBe(123456); + BinaryPrimitives.ReadInt32LittleEndian(data.Span).ShouldBe(123456); } [Fact] @@ -42,7 +50,7 @@ public sealed class LogixServicesTests type.ShouldBe(LogixTypeCode.DInt); data.Length.ShouldBe(64); for (var i = 0; i < 16; i++) - BitConverter.ToUInt32(data.Span[(i * 4)..]).ShouldBe((uint)(((i + 1) << 24) | (i + 1))); + BinaryPrimitives.ReadUInt32LittleEndian(data.Span[(i * 4)..]).ShouldBe((uint)(((i + 1) << 24) | (i + 1))); } [Fact] @@ -62,7 +70,7 @@ public sealed class LogixServicesTests type.ShouldBe((ushort)0x00D0); handle.ShouldBeNull(); data.Length.ShouldBe(88); - BitConverter.ToInt32(data.Span).ShouldBe(7); + BinaryPrimitives.ReadInt32LittleEndian(data.Span).ShouldBe(7); Encoding.ASCII.GetString(data.Span.Slice(4, 7)).ShouldBe("HelloAB"); } @@ -76,7 +84,7 @@ public sealed class LogixServicesTests [Fact] public void Write_request_matches_capture() => - LogixServices.WriteTag(CipPath.EncodeSymbolic("TestDINT"), LogixTypeCode.DInt, null, 1, BitConverter.GetBytes(123456)) + LogixServices.WriteTag(CipPath.EncodeSymbolic("TestDINT"), LogixTypeCode.DInt, null, 1, Int32Le(123456)) .ShouldBe(Req("logix-write-dint.hex", 1)); [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs index afb01c31..d17ba8de 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs @@ -60,4 +60,15 @@ public sealed class SymbolObjectServicesTests public void Failure_status_throws() => Should.Throw(() => SymbolObjectServices.ParseListingReply(new CipReply(0x55, 0x04, [], ReadOnlyMemory.Empty))); + + [Fact] + public void Start_instance_above_16_bits_uses_32_bit_segment() => + Convert.ToHexString(SymbolObjectServices.GetInstanceAttributeList(null, 0x12345)).ShouldBe( + "5504" + "206B" + "2600" + "45230100" + "0400" + "0200" + "0700" + "0800" + "0100"); + + [Fact] + public void More_with_no_entries_throws() => + Should.Throw(() => + SymbolObjectServices.ParseListingReply(new CipReply(0x55, 0x06, [], ReadOnlyMemory.Empty))) + .Status.ShouldBe(AbStatus.BadReply); }