diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs
index a9754706..d9508a81 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipConnection.cs
@@ -123,7 +123,14 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
}
pending = _pendingOpen;
}
- if (started is not null) _ = RunOpenAsync(started);
+ if (started is not null)
+ {
+ // Every waiter may have abandoned the open (its own token); observe a failure so it never
+ // surfaces as UnobservedTaskException.
+ _ = started.Task.ContinueWith(static t => _ = t.Exception, CancellationToken.None,
+ TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
+ _ = RunOpenAsync(started);
+ }
return pending.WaitAsync(ct);
}
@@ -170,6 +177,12 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
{
ThrowIfDisposed(); // disposal may have begun while this attempt waited out the throttle
var client = await EncapsulationClient.ConnectAsync(_key.Host, _key.Port, _o.Timeout, ct).ConfigureAwait(false);
+ if (Volatile.Read(ref _disposed) != 0)
+ {
+ // Disposal began during the TCP connect / RegisterSession: never put a Forward Open on the wire.
+ await client.DisposeAsync().ConfigureAwait(false);
+ throw new AbTransportException(AbStatus.Abort, "Connection disposed.");
+ }
if (_o.Routing != CipRouting.Connected) return new Open(client, null, 0, UnconnectedMessageLimit);
try
{
diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipPath.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipPath.cs
index e653a932..058176aa 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipPath.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/CipPath.cs
@@ -17,6 +17,10 @@ public static class CipPath
{
var port = int.Parse(parts[i], CultureInfo.InvariantCulture);
var link = parts[i + 1];
+ // A link is a slot byte or an IP address/hostname; neither contains whitespace. Without this a
+ // typo like "1,2 3" would be sent as an ASCII link "2 3" instead of failing here.
+ if (link.Length == 0 || link.Any(char.IsWhiteSpace))
+ throw new ArgumentException($"CIP path '{cipPath}' has an empty or malformed link '{link}'.", nameof(cipPath));
var isByte = byte.TryParse(link, NumberStyles.None, CultureInfo.InvariantCulture, out var linkByte);
if (port < 15)
{
diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/DeviceKey.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/DeviceKey.cs
index 9756d613..c8331d21 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/DeviceKey.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/DeviceKey.cs
@@ -3,14 +3,16 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
///
/// Identity of one physical CIP connection: exactly one (and one Forward Open
/// throttle) exists per key (press safety). and compare
-/// case-insensitively; compares exactly; is canonicalised (all
-/// whitespace removed, so "1,0" and " 1, 0 " are the same device) and then compares exactly.
+/// case-insensitively; compares exactly; is canonicalised (each
+/// comma-separated part trimmed, so "1,0" and " 1, 0 " are the same device) and then
+/// compares exactly. Whitespace INSIDE a part is kept, so a typo such as "1,2 3" still fails loudly in
+/// instead of silently becoming slot 23.
///
public sealed record DeviceKey(string Host, int Port, string CipPath, string PlcAttribute)
{
private readonly string _cipPath = Canonicalise(CipPath);
- /// The route, whitespace-free. Canonicalised on construction and on with.
+ /// The route with each part trimmed. Canonicalised on construction and on with.
public string CipPath
{
get => _cipPath;
@@ -18,7 +20,7 @@ public sealed record DeviceKey(string Host, int Port, string CipPath, string Plc
}
private static string Canonicalise(string? path) =>
- path is null ? "" : string.Concat(path.Where(c => !char.IsWhiteSpace(c)));
+ string.IsNullOrWhiteSpace(path) ? "" : string.Join(',', path.Split(',').Select(p => p.Trim()));
public bool Equals(DeviceKey? other) =>
other is not null
diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ForwardOpenThrottle.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ForwardOpenThrottle.cs
index 778f1995..b315aa06 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ForwardOpenThrottle.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Cip/ForwardOpenThrottle.cs
@@ -24,9 +24,12 @@ public sealed class ForwardOpenThrottle
_time = time ?? throw new ArgumentNullException(nameof(time));
}
- /// Run one open attempt once the throttle allows it. Throws
- /// () when
- /// fires before the attempt could start; the attempt itself sees too.
+ /// Run one open attempt once the throttle allows it. bounds only the wait
+ /// BEFORE the attempt (queued behind an in-flight attempt, or the interval): when it fires there, this
+ /// throws (, "Forward Open
+ /// throttled"). is also passed to , which may ignore it:
+ /// does, running the attempt under its own open deadline so that an accepted
+ /// Forward Open is never abandoned without a Forward Close.
public async Task RunAsync(Func> open, CancellationToken ct)
{
ArgumentNullException.ThrowIfNull(open);
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/DeviceSessionPoolTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/DeviceSessionPoolTests.cs
index d9cdb478..bf14ebf9 100644
--- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/DeviceSessionPoolTests.cs
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/DeviceSessionPoolTests.cs
@@ -162,6 +162,7 @@ public sealed class DeviceSessionPoolTests
b.ShouldBe(a);
b.GetHashCode().ShouldBe(a.GetHashCode());
(a with { CipPath = "1, 2" }).CipPath.ShouldBe("1,2");
+ new DeviceKey("plc1", 44818, " ", "ControlLogix").CipPath.ShouldBe("");
var pool = new DeviceSessionPool(new ManualTimeProvider());
await using var la = pool.Acquire(a, Options());
@@ -170,6 +171,16 @@ public sealed class DeviceSessionPoolTests
pool.EntryCount.ShouldBe(1);
}
+ [Fact]
+ public void Whitespace_inside_a_cip_path_part_is_kept_so_a_typo_fails_loudly()
+ {
+ var typo = new DeviceKey("plc1", 44818, "1,2 3", "ControlLogix");
+ typo.CipPath.ShouldNotBe("1,23"); // never silently slot 23 on another controller
+ typo.CipPath.ShouldBe("1,2 3");
+ Should.Throw(() => CipPath.EncodeRoute(typo.CipPath));
+ CipPath.EncodeRoute(new DeviceKey("plc1", 44818, " 1, 0 ", "ControlLogix").CipPath).ShouldBe(new byte[] { 0x01, 0x00 });
+ }
+
[Fact]
public async Task A_new_connection_waits_for_every_earlier_close_not_just_the_latest()
{