From d6392dae174a4f8ba3c3b171406604a766bb6840 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 10 Nov 2026 23:08:42 -0500 Subject: [PATCH 1/2] fix(mtconnect): close Java-driver parity gaps found by the #804 review - Lift an Agent's MTConnectError body under a non-2xx status (cppagent 2.x serves OUT_OF_RANGE as HTTP 400, NO_DEVICE as 404) into the refusal-marked InvalidDataException, reading at most 64 KiB. The 400 used to be a transport fault, so the OUT_OF_RANGE re-baseline never ran against a real 2.x Agent. - Run the instanceId restart test on every re-baseline snapshot: on 2.x a restart is first seen there (the old cursor gets a 400), and was adopted silently. - Report the stream Degraded after 3 consecutive re-baselines with no /sample chunk (it read Healthy for ever); LastSuccessfulRead keeps advancing. - Prohibit DTDs on the /probe and write-answer parses; bound the streams walk to 64 levels (unbounded recursion = uncatchable StackOverflowException). - Skip structural elements under / instead of failing the probe. - Never serve a non-finite float as Good; serve integer-valued decimals on integral tags losslessly (a real fraction stays BadOutOfRange). - Merge sibling browse containers that resolve to one folder name. Unit 678 -> 714, integration 18 -> 20 (live fixture); every gap test failed first. --- .../IMTConnectAgentClient.cs | 3 +- .../MTConnectAgentClient.cs | 87 +++- .../MTConnectDriver.cs | 186 ++++++- .../MTConnectObservationIndex.cs | 75 ++- .../MTConnectProbeParser.cs | 22 +- .../MTConnectStreamsParser.cs | 17 + .../MTConnectXml.cs | 90 ++++ .../MTConnectAgentIntegrationTests.cs | 110 ++++ .../MTConnectJavaParityTests.cs | 482 ++++++++++++++++++ .../MTConnectSubscribeTests.cs | 129 +++++ 10 files changed, 1168 insertions(+), 33 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectJavaParityTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs index 5ab068021..39f57f503 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs @@ -256,7 +256,8 @@ public interface IMTConnectAgentClient : IDisposable /// This check alone does NOT cover every ring-buffer overflow. It can only judge /// a chunk the Agent was willing to send — and cppagent answers a from that has /// already fallen out of its buffer with an MTConnectError / OUT_OF_RANGE - /// document served under HTTP 200, which this client surfaces as an + /// document — served under HTTP 200 by older Agents and under HTTP 400 by a 2.x + /// one (both lifted since #804) — which this client surfaces as an /// out of , never as a chunk. /// A pump that re-baselines only on IsSequenceGap will therefore fault instead of /// recovering exactly when it has fallen furthest behind. Treat an OUT_OF_RANGE diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs index 13e2bfab3..4d604a09c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs @@ -175,7 +175,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable // ResponseContentRead (the default) buffers the whole body *under* this awaited, token-bound // call, so the parse below reads from memory — no network read can outlive the deadline. - using var response = await SendAsync(_probeUri, ct).ConfigureAwait(false); + using var response = await SendAsync(_probeUri, "MTConnectDevices", "/probe response", ct).ConfigureAwait(false); await using var body = await response.Content.ReadAsStreamAsync(ct).ConfigureAwait(false); @@ -216,7 +216,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable { ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposed) != 0, this); - using var response = await SendAsync(_currentUri, ct).ConfigureAwait(false); + using var response = await SendAsync(_currentUri, "MTConnectStreams", "streams response", ct).ConfigureAwait(false); await using var body = await response.Content.ReadAsStreamAsync(ct).ConfigureAwait(false); @@ -432,7 +432,13 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable XElement? root; try { - root = XDocument.Parse(body).Root; + root = MTConnectXml.ParseHardened(body).Root; + } + catch (XmlException) when (body.Contains(" SendAsync(Uri uri, CancellationToken ct) + private async Task SendAsync( + Uri uri, string expectedRootName, string subject, CancellationToken ct) { using var lifetime = CancellationTokenSource.CreateLinkedTokenSource(ct, _disposeCts.Token); using var deadline = CancellationTokenSource.CreateLinkedTokenSource(lifetime.Token); @@ -510,7 +517,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable throw TimedOut(uri); } - response.EnsureSuccessStatusCode(); + await EnsureAgentAnsweredAsync(response, expectedRootName, subject, deadline.Token).ConfigureAwait(false); return response; } @@ -537,11 +544,79 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable throw TimedOut(uri); } - response.EnsureSuccessStatusCode(); + await EnsureAgentAnsweredAsync(response, "MTConnectStreams", "streams response", deadline.Token) + .ConfigureAwait(false); return response; } + /// + /// How much of a failure body is read looking for the Agent's MTConnectError document. An Agent's + /// error document is a few hundred bytes; a proxy's HTML error page never needs to be held in full. + /// + internal const int MaxErrorBodyBytes = 64 * 1024; + + /// + /// Passes a 2xx response through; otherwise throws — the Agent's own refusal (a refusal-marked + /// , see ) when the + /// failure body is an MTConnectError document, else the plain + /// raises. The response is disposed on every throw. + /// + /// + /// At most of the body are read, under the caller's deadline. A body that + /// cannot be read in time is not worth a different failure: the status alone is reported, exactly as before. + /// + private static async Task EnsureAgentAnsweredAsync( + HttpResponseMessage response, string expectedRootName, string subject, CancellationToken deadline) + { + if (response.IsSuccessStatusCode) + { + return; + } + + try + { + string? head = null; + try + { + head = await ReadHeadAsync(response.Content, deadline).ConfigureAwait(false); + } + catch (Exception ex) when (ex is IOException or HttpRequestException or OperationCanceledException) + { + // Unreadable body: fall through to the status-only failure below — what was reported before. + } + + var refusal = MTConnectXml.AgentRefusalFromErrorBody(head, (int)response.StatusCode, expectedRootName, subject); + if (refusal is not null) + { + throw refusal; + } + + response.EnsureSuccessStatusCode(); + } + catch + { + response.Dispose(); + throw; + } + } + + /// Reads at most of a failure body as UTF-8. + private static async Task ReadHeadAsync(HttpContent content, CancellationToken ct) + { + await using var body = await content.ReadAsStreamAsync(ct).ConfigureAwait(false); + var buffer = new byte[MaxErrorBodyBytes]; + var filled = 0; + int read; + while (filled < buffer.Length && + (read = await body.ReadAsync(buffer.AsMemory(filled, buffer.Length - filled), ct).ConfigureAwait(false)) > 0) + { + filled += read; + } + + return Encoding.UTF8.GetString(buffer, 0, filled); + } + /// Reads a non-multipart streaming body whole, under the unary deadline. private async Task ReadWholeBodyAsync(Stream body, Uri uri, CancellationTokenSource lifetime) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 74e84efd6..dd6b53940 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -161,6 +161,17 @@ public sealed partial class MTConnectDriver /// private static readonly TimeSpan TeardownWait = TimeSpan.FromSeconds(5); + /// + /// How many consecutive /current re-baselines with no /sample chunk delivered between them + /// report the stream Degraded (#804; the Java driver's MAX_REBASELINES_WITHOUT_PROGRESS). One + /// re-baseline is a genuine ring-buffer overrun resolving; three in a row is an Agent and a cursor/count + /// that cannot agree. + /// + internal const int MaxRebaselinesWithoutProgress = 3; + + /// The type-only LastError a stalled stream () reports. + internal const string StalledStreamReason = "repeated /sample re-baselines with no observations in between"; + /// /// Config-JSON reader options, mirroring the sibling driver factories. Note there is /// deliberately no JsonStringEnumConverter: enum-carrying DTO fields stay @@ -1433,6 +1444,9 @@ public sealed partial class MTConnectDriver var devices = 0; var variables = 0; + // Siblings that resolve to one folder name share ONE folder (#804) — see SiblingFolders. + var deviceFolders = new SiblingFolders(builder); + foreach (var device in model.Devices) { if (device is null || !InDeviceScope(device, deviceScope)) @@ -1454,7 +1468,7 @@ public sealed partial class MTConnectDriver } devices++; - variables += StreamContainer(device, builder.Folder(name, name), writeEnabled); + variables += StreamContainer(device, deviceFolders.Get(name), writeEnabled); } // A configured scope that matches NO device is an authoring error — almost always a typo or @@ -1555,6 +1569,8 @@ public sealed partial class MTConnectDriver streamed++; } + var componentFolders = new SiblingFolders(scope); + foreach (var component in container.Components) { if (component is null) @@ -1574,12 +1590,42 @@ public sealed partial class MTConnectDriver continue; } - streamed += StreamContainer(component, scope.Folder(name, name), writeEnabled); + streamed += StreamContainer(component, componentFolders.Get(name), writeEnabled); } return streamed; } + /// + /// One builder folder per distinct name under one parent scope: two sibling containers that resolve to the + /// same folder name (two components named alike, or one's name equal to another's id) are + /// streamed into a SINGLE shared folder, deterministically, rather than as two same-named folders. + /// + /// + /// Why (#804, Java-driver parity). A browse picker keys a folder by its path, so two same-named sibling + /// folders share one key: the universal browser's capture (CapturingAddressSpaceBuilder) indexes the + /// FIRST and every expand of that path returns the first one's children — the second container's data items + /// were listed in the tree but unreachable. Merging is what the Java driver does for the same collision, and + /// it loses nothing: every data item is still streamed, keyed by its own DataItem id. A data item whose name + /// equals a sibling folder's is NOT a collision here — a leaf is keyed by its DataItem id, never its path. + /// + private sealed class SiblingFolders(IAddressSpaceBuilder parent) + { + private readonly Dictionary _folders = new(StringComparer.Ordinal); + + /// The folder named under the parent, created on first use. + public IAddressSpaceBuilder Get(string name) + { + if (!_folders.TryGetValue(name, out var folder)) + { + folder = parent.Folder(name, name); + _folders[name] = folder; + } + + return folder; + } + } + /// /// Whether would accept a write to — the /// browse-time answer, asked of the same two facts the runtime asks of. @@ -2186,6 +2232,9 @@ public sealed partial class MTConnectDriver var counts = new SampleCountState(options.SampleCount); Volatile.Write(ref _effectiveSampleCount, counts.Count); + // #804 (Java-driver parity) — consecutive re-baselines with no /sample chunk delivered in between. + var rebaselinesWithoutProgress = 0; + try { if (republishOnStart) @@ -2215,7 +2264,15 @@ public sealed partial class MTConnectDriver } } - var outcome = await ConsumeStreamAsync(client, options, cursor, instanceId, generation, counts, ct).ConfigureAwait(false); + var outcome = await ConsumeStreamAsync( + client, options, cursor, instanceId, generation, counts, + holdStalled: rebaselinesWithoutProgress >= MaxRebaselinesWithoutProgress, ct) + .ConfigureAwait(false); + + if (outcome.DeliveredAny) + { + rebaselinesWithoutProgress = 0; + } cursor = outcome.Cursor; instanceId = outcome.InstanceId; @@ -2291,6 +2348,25 @@ public sealed partial class MTConnectDriver attempt = outcome.DeliveredAny ? 0 : attempt + 1; _reconnectAttempts = attempt; + // #804 — the backoff above PACES a stream that only ever re-baselines, but each /current + // also marked the stream delivered, so it read Healthy for ever while no /sample chunk + // arrived (a SampleCount at or above the Agent's bufferSize is refused OUT_OF_RANGE on + // EVERY open). After MaxRebaselinesWithoutProgress in a row it is reported Degraded, and + // stays so (holdStalled) until a chunk is delivered. + if (!outcome.DeliveredAny && + ++rebaselinesWithoutProgress == MaxRebaselinesWithoutProgress) + { + if (!TryHoldStreamStalled(generation, StalledStreamReason)) + { + return; + } + + _logger.LogWarning( + outcome.Error, + "MTConnect driver {DriverInstanceId}: {Count} consecutive /current re-baselines of {AgentUri} without a single /sample chunk in between; reporting the stream Degraded until one is delivered. Values are re-baselined, not streamed. An Agent answering every /sample with OUT_OF_RANGE usually means SampleCount is at or above its bufferSize.", + _driverInstanceId, rebaselinesWithoutProgress, AgentDisplay(options)); + } + break; default: @@ -2354,6 +2430,7 @@ public sealed partial class MTConnectDriver long instanceId, long generation, SampleCountState counts, + bool holdStalled, CancellationToken ct) { // The RUNNING cursor the gap check is made against: the previous chunk's nextSequence, equal @@ -2402,23 +2479,13 @@ public sealed partial class MTConnectDriver // Dropped BEFORE the re-baseline, and the subscribers are told: if the /current // then fails, holding stale values from a dead device model would be worse than // holding none. - if (!TryCaptureSessionState(generation, out var restartIndex, out var restartTracker)) + if (!TryDropForAgentRestart(generation, chunk.InstanceId, options)) { return new StreamOutcome(StreamVerdict.Cancelled, expected, instanceId, delivered, null); } - // Not live while the index is empty (review m2): reads go to /current until the re-baseline lands. - _pumpLive = false; - restartIndex.Clear(); - restartTracker.Clear(); - FanOut(SubscribedReferences(), restartIndex); - - // The driver's own state is consistent by this point, so it is safe to hand the - // restart to arbitrary caller code — which may call straight back into - // DiscoverAsync. - AnnounceAgentRestart(chunk.InstanceId, options); - - return await RebaselineAsync(client, options, expected, instanceId, delivered, generation, ct) + return await RebaselineAsync( + client, options, expected, chunk.InstanceId, delivered, generation, holdStalled, ct) .ConfigureAwait(false); } @@ -2428,7 +2495,7 @@ public sealed partial class MTConnectDriver "MTConnect driver {DriverInstanceId} fell out of {AgentUri}'s buffer (expected sequence {Expected}, oldest retained {FirstSequence}); re-baselining from /current.", _driverInstanceId, AgentDisplay(options), expected, chunk.FirstSequence); - return await RebaselineAsync(client, options, expected, instanceId, delivered, generation, ct) + return await RebaselineAsync(client, options, expected, instanceId, delivered, generation, holdStalled, ct) .ConfigureAwait(false); } @@ -2501,7 +2568,7 @@ public sealed partial class MTConnectDriver "MTConnect driver {DriverInstanceId} could not resume {AgentUri}'s /sample stream at sequence {Cursor} (the Agent rejected it as out of range); re-baselining from /current.", _driverInstanceId, AgentDisplay(options), cursor); - return await RebaselineAsync(client, options, expected, instanceId, delivered, generation, ct) + return await RebaselineAsync(client, options, expected, instanceId, delivered, generation, holdStalled, ct) .ConfigureAwait(false); } catch (Exception ex) @@ -2532,6 +2599,7 @@ public sealed partial class MTConnectDriver long fallbackInstanceId, bool deliveredAny, long generation, + bool holdStalled, CancellationToken ct) { try @@ -2545,8 +2613,31 @@ public sealed partial class MTConnectDriver StreamVerdict.Cancelled, fallbackCursor, fallbackInstanceId, deliveredAny, null); } + // #804 — the restart test runs on the SNAPSHOT too (the Java driver's rule). On a real 2.x Agent this + // is how a restart is usually first seen: the restarted process refuses the dead one's cursor with + // HTTP 400 OUT_OF_RANGE, so no chunk ever reaches the in-stream instanceId check above, and the + // re-baseline used to adopt the new instanceId silently — no rediscovery prompt, and the dead model's + // held values (data items the new Agent may not declare) kept being served. + if (current.InstanceId != fallbackInstanceId) + { + _logger.LogWarning( + "MTConnect driver {DriverInstanceId} saw {AgentUri} restart while re-baselining (instanceId {Previous} -> {Current}); dropping every held observation.", + _driverInstanceId, AgentDisplay(options), fallbackInstanceId, current.InstanceId); + + if (!TryDropForAgentRestart(generation, current.InstanceId, options)) + { + return new StreamOutcome( + StreamVerdict.Cancelled, fallbackCursor, fallbackInstanceId, deliveredAny, null); + } + } + PublishAgentCursor(generation, current.InstanceId, current.NextSequence); - if (!ApplyAndFanOut(current, generation, baseline: true) || !TryMarkStreamDelivered(generation)) + // A stalled stream (#804) keeps its Degraded verdict across the re-baseline: the /current answered, but + // it is not the /sample progress the verdict is about. Only a delivered chunk clears it. + if (!ApplyAndFanOut(current, generation, baseline: true) || + !(holdStalled && !deliveredAny + ? TryHoldStreamStalled(generation, StalledStreamReason) + : TryMarkStreamDelivered(generation))) { return new StreamOutcome( StreamVerdict.Cancelled, fallbackCursor, fallbackInstanceId, deliveredAny, null); @@ -2732,6 +2823,63 @@ public sealed partial class MTConnectDriver } } + /// + /// An Agent restart (a changed instanceId) seen by the pump started under : + /// drop every held observation and condition state — the device model they describe may no longer exist — + /// tell the subscribers, and raise rediscovery (once per distinct id). + /// + /// when the pump's session has been replaced (nothing dropped). + private bool TryDropForAgentRestart(long generation, long newInstanceId, MTConnectDriverOptions options) + { + if (!TryCaptureSessionState(generation, out var restartIndex, out var restartTracker)) + { + return false; + } + + // Not live while the index is empty (review m2): reads go to /current until the re-baseline lands. + _pumpLive = false; + restartIndex.Clear(); + restartTracker.Clear(); + FanOut(SubscribedReferences(), restartIndex); + + // The driver's own state is consistent by this point, so it is safe to hand the restart to arbitrary + // caller code — which may call straight back into DiscoverAsync. + AnnounceAgentRestart(newInstanceId, options); + + return true; + } + + /// + /// #804: the pump started under has re-baselined + /// times in a row without one /sample chunk: degrade the + /// stream — generation-checked, like — but stamp + /// LastSuccessfulRead now, because each /current it re-baselined from IS the Agent answering. + /// That is deliberate: the comms-loss watchdog reconnects on non-Healthy AND a frozen stamp, and a reconnect + /// cannot fix an Agent that refuses the count or cursor — it would only reconnect-loop the driver (the + /// "an Agent refusal is a reply" health contract). + /// + /// The pump's session generation. + /// The type-only reason. + /// when the pump's session has been replaced (nothing written). + private bool TryHoldStreamStalled(long generation, string message) + { + lock (_subscriptionLock) + { + if (_sessionGeneration != generation) + { + return false; + } + + _streamDegraded = true; + if (ReadHealth().State != DriverState.Faulted) + { + WriteHealth(new DriverHealth(DriverState.Degraded, DateTime.UtcNow, message)); + } + + return true; + } + } + /// /// Review I-1: the stream of the pump started under delivered — clear its /// degradation, mark it live and publish Healthy, all under the generation check and in one critical section, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs index 8c463697c..40f6d0ada 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs @@ -558,6 +558,13 @@ internal sealed class MTConnectObservationIndex case DriverDataType.Float32: if (float.TryParse(text, NumberStyles.Float, CultureInfo.InvariantCulture, out var single)) { + // .NET parses an overflowing literal ("1e39") to ±Infinity and accepts the NaN/Infinity + // spellings outright; publishing either as a Good reading is a fabricated value (#804). + if (!float.IsFinite(single)) + { + return NonFiniteStatus(text); + } + value = single; return Good; } @@ -567,6 +574,11 @@ internal sealed class MTConnectObservationIndex case DriverDataType.Float64: if (double.TryParse(text, NumberStyles.Float, CultureInfo.InvariantCulture, out var real)) { + if (!double.IsFinite(real)) + { + return NonFiniteStatus(text); + } + value = real; return Good; } @@ -636,8 +648,65 @@ internal sealed class MTConnectObservationIndex // Integer parse failed. Distinguishing the two causes is worth one extra parse: it tells an // operator whether to retype the tag or widen it. - return double.TryParse(text, NumberStyles.Float, CultureInfo.InvariantCulture, out _) - ? BadOutOfRange // a number the authored type cannot represent (overflow, or a fraction) - : BadTypeMismatch; // not a number at all + if (!decimal.TryParse(text, NumberStyles.Float, CultureInfo.InvariantCulture, out var exact)) + { + return double.TryParse(text, NumberStyles.Float, CultureInfo.InvariantCulture, out _) + ? BadOutOfRange // a number the authored type cannot represent (beyond even decimal's range) + : BadTypeMismatch; // not a number at all + } + + // #804 (Java-driver parity): Agents do format integer-valued items as "42.000000". A decimal whose value + // IS an integer converts losslessly, so it is served. A real fraction ("3.5") is still refused rather than + // truncated (stricter than the Java driver, which truncates toward zero): truncating is a fabricated value. + return decimal.Truncate(exact) == exact && TryIntegral(exact, type, out value) + ? Good + : BadOutOfRange; + } + + /// + /// The status for a float text that parsed to a non-finite value: an overflowing numeric literal is a + /// number the type cannot represent (BadOutOfRange); a NaN/Infinity spelling is not a measurement + /// at all (BadTypeMismatch). + /// + private static uint NonFiniteStatus(string text) + { + foreach (var c in text) + { + if (c is not ((>= '0' and <= '9') or '+' or '-' or '.' or 'e' or 'E')) + { + return BadTypeMismatch; + } + } + + return BadOutOfRange; + } + + /// Range-checked conversion of an integer-valued decimal onto an integral driver type. + private static bool TryIntegral(decimal exact, DriverDataType type, out object? value) + { + value = null; + switch (type) + { + case DriverDataType.Int16 when exact is >= short.MinValue and <= short.MaxValue: + value = (short)exact; + return true; + case DriverDataType.Int32 when exact is >= int.MinValue and <= int.MaxValue: + value = (int)exact; + return true; + case DriverDataType.Int64 when exact is >= long.MinValue and <= long.MaxValue: + value = (long)exact; + return true; + case DriverDataType.UInt16 when exact is >= ushort.MinValue and <= ushort.MaxValue: + value = (ushort)exact; + return true; + case DriverDataType.UInt32 when exact is >= uint.MinValue and <= uint.MaxValue: + value = (uint)exact; + return true; + case DriverDataType.UInt64 when exact is >= ulong.MinValue and <= ulong.MaxValue: + value = (ulong)exact; + return true; + default: + return false; + } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs index 0a184e49f..d50d66bfa 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs @@ -80,7 +80,7 @@ internal static class MTConnectProbeParser XDocument document; try { - document = XDocument.Parse(xml); + document = MTConnectXml.ParseHardened(xml); } catch (XmlException ex) { @@ -101,7 +101,7 @@ internal static class MTConnectProbeParser XDocument document; try { - document = XDocument.Load(stream); + document = MTConnectXml.LoadHardened(stream); } catch (XmlException ex) { @@ -125,7 +125,7 @@ internal static class MTConnectProbeParser ?? throw new InvalidDataException( "MTConnect /probe response has no element; it does not describe a device model."); - var devices = devicesContainer.Elements().Select(ReadDevice).ToList(); + var devices = ComponentLikeChildren(devicesContainer).Select(ReadDevice).ToList(); if (devices.Count == 0) { throw new InvalidDataException( @@ -162,11 +162,25 @@ internal static class MTConnectProbeParser } return MTConnectXml.ChildrenNamed(container, "Components") - .SelectMany(components => components.Elements()) + .SelectMany(ComponentLikeChildren) .Select(element => ReadComponent(element, depth)) .ToList(); } + /// + /// Element names that can sit beside components but are MTConnect's own structural children, never a + /// component or device. Defence in depth (#804, Java-driver parity): a schema-valid document never puts one + /// directly under <Components> or <Devices>, but the "every element child is a + /// component" rule is deliberately permissive, and without this list a stray <Description> there + /// would fail the WHOLE probe on its missing id — every data item lost over one misplaced element. + /// + private static readonly HashSet NotAComponent = + new(StringComparer.Ordinal) { "DataItems", "Description", "Configuration", "References", "Compositions" }; + + /// A container's element children minus . + private static IEnumerable ComponentLikeChildren(XElement container) => + container.Elements().Where(child => !NotAComponent.Contains(child.Name.LocalName)); + private static IReadOnlyList ReadDataItems(XElement container) => MTConnectXml.ChildrenNamed(container, "DataItems") .SelectMany(dataItems => MTConnectXml.ChildrenNamed(dataItems, "DataItem")) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs index 4e32e5f7e..4c0827416 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs @@ -109,6 +109,17 @@ internal static class MTConnectStreamsParser /// How those messages name the header element. private const string HeaderOwner = "the
"; + /// + /// Bound on element nesting (#804, Java-driver parity). and + /// recurse once per nesting level, and the document comes from an + /// unauthenticated peer: a part nested a few hundred thousand levels deep (well inside the + /// multipart reader's 32 MiB part cap) would otherwise recurse into a + /// , which .NET cannot catch — it would take the whole server + /// process down, not just this driver. A real streams document nests about eight levels + /// (MTConnectStreams/Streams/DeviceStream/ComponentStream/Samples/observation/Entry/Cell). + /// + internal const int MaxElementDepth = 64; + /// /// How the whole document is read. Two settings are load-bearing rather than incidental. /// @@ -381,6 +392,12 @@ internal static class MTConnectStreamsParser ValueSink? textSink, List? entryCollector = null) { + if (reader.Depth >= MaxElementDepth) + { + throw new InvalidDataException( + $"MTConnect {Subject} nests elements more than {MaxElementDepth} levels deep; refusing to recurse further."); + } + var isObservation = parent != ContainerKind.None; var isCondition = parent == ContainerKind.Condition; var kind = ContainerKindOf(reader); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs index 3497d27f8..c9237a7fd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs @@ -28,6 +28,96 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// internal static class MTConnectXml { + /// + /// The reader settings every tree-building parse of an Agent document uses (the + /// /probe model, an error body, a write answer). The Agent is an unauthenticated peer on + /// the plant network, so a DOCTYPE is refused outright and nothing external is ever resolved — + /// no legitimate MTConnect document carries one. on its own + /// parses DTDs (it expands internal entities up to ten million characters); the streams + /// parser already prohibited them, and this is what brings the other parses into line (#804). + /// reproduces XDocument.Parse's default. + /// + private static readonly XmlReaderSettings HardenedTreeSettings = new() + { + DtdProcessing = DtdProcessing.Prohibit, + XmlResolver = null, + IgnoreWhitespace = true, + CloseInput = false, + }; + + /// Parses into a tree with DTDs prohibited (see ). + /// The document text. + /// The parsed document. + /// The text is not well-formed, or carries a DOCTYPE. + public static XDocument ParseHardened(string xml) + { + using var text = new StringReader(xml); + using var reader = XmlReader.Create(text, HardenedTreeSettings); + + return XDocument.Load(reader); + } + + /// Loads into a tree with DTDs prohibited (see ). + /// The document bytes; not closed. + /// The parsed document. + /// The bytes are not well-formed, or carry a DOCTYPE. + public static XDocument LoadHardened(Stream stream) + { + using var reader = XmlReader.Create(stream, HardenedTreeSettings); + + return XDocument.Load(reader); + } + + /// + /// The Agent's MTConnectError document in , rendered as the + /// refusal-marked every caller already routes (see + /// ), or when the body is not one. + /// + /// + /// Why this exists (#804). A modern Agent (cppagent 2.x, the fixture's + /// mtconnect/agent:2.7) does not serve its error documents under HTTP 200 the way older + /// ones did: it answers an out-of-range /sample?from= — and a count at or above its + /// buffer — with HTTP 400, and an unknown device with 404, each carrying a + /// well-formed MTConnectError body. A status check alone turns those into an + /// , which the pump treats as a transient transport loss and + /// reconnects at the SAME evicted cursor, so the OUT_OF_RANGE re-baseline never ran + /// against a real 2.x Agent. Lifting the document is what routes it. + /// + /// The (already size-bounded) response body. + /// The HTTP status the body arrived under, for the message. + /// The root the request expected. + /// How to describe the document. + /// The refusal to throw, or . + public static InvalidDataException? AgentRefusalFromErrorBody( + string? body, int status, string expectedRootName, string subject) + { + if (string.IsNullOrWhiteSpace(body)) + { + return null; + } + + XElement? root; + try + { + root = ParseHardened(body.Trim('', ' ', '\t', '\r', '\n')).Root; + } + catch (XmlException) + { + return null; + } + + if (root is null || !IsNamed(root, "MTConnectError")) + { + return null; + } + + var ex = new InvalidDataException( + $"{DescribeUnexpectedRoot(root, expectedRootName, subject)} (served under HTTP {status})"); + ex.Data[AgentRefusalDataKey] = true; + + return ex; + } + /// Does have the given local name, whatever its namespace? /// The element to test. /// The unqualified name to match. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs index 4db8f1abe..47a57aa6d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs @@ -837,6 +837,116 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) } } + // ---- #804: the 2.x Agent's error documents under a non-2xx status ---- + + /// + /// #804 — a 2.x Agent answers a from outside its buffer with HTTP 400 + MTConnectError + /// OUT_OF_RANGE (live-verified against this fixture). The client must surface it as the + /// the pump re-baselines on; before #804 it was a bare + /// , which the pump read as transport loss and reconnected at the same + /// evicted cursor — the documented OUT_OF_RANGE re-baseline never ran against a real 2.x Agent. + /// + [Fact(Timeout = TestTimeoutMs)] + public async Task A_sample_cursor_outside_the_buffer_is_the_agents_out_of_range_refusal() + { + SkipIfDown(); + + // A cursor the Agent cannot hold: far past its nextSequence. (An OLD cursor is not reliable here — a + // freshly restarted Agent may still hold sequence 1, and then the stream simply opens.) + var next = (long?)(await CurrentAsync()).Root?.Elements().FirstOrDefault(e => e.Name.LocalName == "Header") + ?.Attribute("nextSequence") ?? 0L; + using var client = new MTConnectAgentClient(LiveOptions()); + + var ex = await Should.ThrowAsync(async () => + { + await foreach (var _ in client.SampleAsync(next + 1_000_000, Ct)) + { + } + }); + + ex.Message.ShouldContain("OUT_OF_RANGE"); + } + + /// + /// #804 — a SampleCount at or above the Agent's bufferSize is refused OUT_OF_RANGE on + /// EVERY /sample open (live: 'count' must be less than N). The driver keeps serving values off + /// /current re-baselines, and — after three in a row with no /sample chunk — reports the stream + /// Degraded with LastSuccessfulRead still advancing, rather than Healthy for ever (before #804 the + /// 400 was a transport fault: Degraded, but every value stuck at the priming /current). + /// + [Fact(Timeout = TestTimeoutMs)] + public async Task A_sample_count_the_agent_refuses_is_reported_degraded_while_values_keep_flowing() + { + SkipIfDown(); + + var bufferSize = (int?)agent.ProbeDocument?.Root?.Elements().FirstOrDefault(e => e.Name.LocalName == "Header") + ?.Attribute("bufferSize"); + if (bufferSize is not > 0) + { + Assert.Skip($"The Agent at {agent.AgentUri} declares no bufferSize in its /probe header."); + } + + var ids = await NumericSampleIdsAsync(); + if (ids.Count == 0) + { + Assert.Skip($"The Agent at {agent.AgentUri} is currently reporting no numeric SAMPLE value."); + } + + var options = new MTConnectDriverOptions + { + AgentUri = agent.AgentUri, + RequestTimeoutMs = 15_000, + SampleIntervalMs = 200, + SampleCount = bufferSize.Value + 1, + HeartbeatMs = 5_000, + Probe = new MTConnectProbeOptions { Enabled = false }, + Reconnect = new MTConnectReconnectOptions { MinBackoffMs = 0, MaxBackoffMs = 200 }, + }; + var driver = new MTConnectDriver(options, "mtconnect-live-oor"); + await using var live = new LiveDriver(driver); + await driver.InitializeAsync( + JsonSerializer.Serialize(new + { + agentUri = options.AgentUri, + requestTimeoutMs = options.RequestTimeoutMs, + sampleIntervalMs = options.SampleIntervalMs, + sampleCount = options.SampleCount, + heartbeatMs = options.HeartbeatMs, + probe = new { enabled = false }, + reconnect = new { minBackoffMs = 0, maxBackoffMs = 200, backoffMultiplier = 2.0 }, + tags = ids.Select(id => new { fullName = id, driverDataType = "Float64" }), + }), + Ct); + + var seen = new ConcurrentQueue(); + driver.OnDataChange += (_, e) => seen.Enqueue(e); + await driver.SubscribeAsync([.. ids], TimeSpan.FromMilliseconds(200), Ct); + + var deadline = DateTime.UtcNow + StreamWait; + while (driver.GetHealth().LastError != "repeated /sample re-baselines with no observations in between" && + DateTime.UtcNow < deadline) + { + await Task.Delay(100, Ct); + } + + var health = driver.GetHealth(); + health.State.ShouldBe(DriverState.Degraded); + health.LastError.ShouldBe("repeated /sample re-baselines with no observations in between"); + var stamp = health.LastSuccessfulRead.ShouldNotBeNull(); + + // Values keep flowing off the re-baselines: the stamp advances and Good values keep arriving. + var before = seen.Count; + deadline = DateTime.UtcNow + StreamWait; + while ((driver.GetHealth().LastSuccessfulRead <= stamp || seen.Count <= before) && DateTime.UtcNow < deadline) + { + await Task.Delay(100, Ct); + } + + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + driver.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThan(stamp); + seen.Skip(before).ShouldContain(e => e.Snapshot.StatusCode == Good); + } + private static bool Is(string? value, string expected) => string.Equals(value?.Trim(), expected, StringComparison.OrdinalIgnoreCase); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectJavaParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectJavaParityTests.cs new file mode 100644 index 000000000..d40d5a2e1 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectJavaParityTests.cs @@ -0,0 +1,482 @@ +using System.Net; +using System.Text; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests; + +/// +/// Gitea #804 — the gaps a parity review against the ignitionoee Java MTConnect driver verified in this +/// driver, one region per gap. Each test failed against the pre-#804 driver; the capability matrix that +/// says which Java behaviours were already covered (and by which existing tests) is +/// docs/drivers/MTConnect.md §"Java reference parity (#804)". +/// +public sealed class MTConnectJavaParityTests +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private const string OutOfRangeError = + """ + + +
+ + 'from' must be greater than 59962705 + + + """; + + private const string NoDeviceError = + """ + + Could not find the device 'nosuch' + + """; + + private static MTConnectAgentClient NewClient(HttpMessageHandler handler) => + new(new MTConnectDriverOptions { AgentUri = "http://agent:5000", RequestTimeoutMs = 5000 }, handler); + + private static async Task DrainAsync(IAsyncEnumerable stream) + { + await foreach (var _ in stream) + { + } + } + + // ───────────── gap 1: an Agent error document under a non-2xx status is the Agent's refusal ───────────── + // cppagent 2.x (the fixture's mtconnect/agent:2.7, verified live) answers an evicted `from` and a count at or + // above its buffer with HTTP 400 + MTConnectError OUT_OF_RANGE. Pre-#804 the client threw a bare + // HttpRequestException, which the pump treats as a transient transport loss: it reconnected at the SAME + // evicted cursor for ever and the OUT_OF_RANGE re-baseline never ran against a real 2.x Agent. + + [Fact] + public async Task Sample_answered_400_with_an_out_of_range_document_surfaces_as_the_agents_refusal() + { + using var client = NewClient(new StubHandler(_ => Respond(HttpStatusCode.BadRequest, OutOfRangeError))); + + var ex = await Should.ThrowAsync(async () => await DrainAsync(client.SampleAsync(1, Ct))); + + ex.Message.ShouldContain("OUT_OF_RANGE: 'from' must be greater than 59962705"); + ex.Message.ShouldContain("HTTP 400"); + MTConnectXml.IsAgentRefusal(ex).ShouldBeTrue(); + } + + [Fact] + public async Task Current_answered_404_with_a_no_device_document_is_an_agent_refusal_not_a_comms_fault() + { + using var client = NewClient(new StubHandler(_ => Respond(HttpStatusCode.NotFound, NoDeviceError))); + + var ex = await Should.ThrowAsync(() => client.CurrentAsync(Ct)); + + ex.Message.ShouldContain("NO_DEVICE"); + MTConnectXml.IsAgentRefusal(ex).ShouldBeTrue(); + } + + [Fact] + public async Task Probe_answered_404_with_an_error_document_carries_the_agents_own_words() + { + using var client = NewClient(new StubHandler(_ => Respond(HttpStatusCode.NotFound, NoDeviceError))); + + var ex = await Should.ThrowAsync(() => client.ProbeAsync(Ct)); + + ex.Message.ShouldContain("Could not find the device 'nosuch'"); + ex.Message.ShouldContain("expected "); + } + + /// Positive control: a non-2xx body that is NOT an MTConnect error document keeps the status-only failure. + [Fact] + public async Task A_non_2xx_proxy_page_is_still_a_plain_http_failure() + { + using var client = NewClient(new StubHandler(_ => Respond(HttpStatusCode.BadGateway, "502"))); + + await Should.ThrowAsync(() => client.CurrentAsync(Ct)); + } + + /// The failure body is read only up to the cap: a huge error page never becomes a huge string. + [Fact] + public async Task An_error_document_beyond_the_body_cap_is_not_read_in_full() + { + var padding = new string(' ', 70 * 1024); // past the 64 KiB failure-body cap + using var client = NewClient(new StubHandler(_ => Respond(HttpStatusCode.BadRequest, padding + OutOfRangeError))); + + // The document starts past the cap, so it is never seen: status-only failure, not a refusal. + await Should.ThrowAsync(async () => await DrainAsync(client.SampleAsync(1, Ct))); + } + + // ───────────── gap 2 (driver-level, MTConnectSubscribeTests): repeated re-baselines without progress ───────────── + + // ───────────── gap 3: streams parser nesting bound ───────────── + // ReadElement/ReadChildren recurse per nesting level; an unbounded depth from an unauthenticated peer is a + // StackOverflowException, which .NET cannot catch — the whole server process dies. + + private static string Streams(string body) => + $""" + +
+ {body} + + """; + + [Fact] + public void Streams_parse_refuses_absurd_nesting_instead_of_recursing_without_bound() + { + const int depth = 80; // beyond the 64-level bound + var xml = Streams(string.Concat(Enumerable.Repeat("", depth)) + string.Concat(Enumerable.Repeat("", depth))); + + var ex = Should.Throw(() => MTConnectStreamsParser.Parse(xml)); + + ex.Message.ShouldContain("levels deep"); + } + + /// Positive control: a real document's nesting (with a TABLE's Entry/Cell) parses under the bound. + [Fact] + public void Streams_parse_accepts_ordinary_device_nesting() + { + var xml = Streams( + """ + + + + 1.0 + + ACTIVE + + + """); + + var result = MTConnectStreamsParser.Parse(xml); + + result.Observations.Select(o => o.DataItemId).ShouldBe(["wo", "exec"]); + } + + // ───────────── gap 4: DOCTYPE refused on every tree-building parse ───────────── + // The streams parser already prohibited DTDs; the /probe parse (and the write-answer parse) went through + // XDocument.Parse, which PARSES a DTD and expands its internal entities. + + private const string ProbeWithDoctype = + """ + + ]> + +
+ + + """; + + [Fact] + public void Probe_parse_refuses_a_doctype_from_a_string() + { + var ex = Should.Throw(() => MTConnectProbeParser.Parse(ProbeWithDoctype)); + + ex.Message.ShouldContain("not well-formed XML"); + } + + [Fact] + public void Probe_parse_refuses_a_doctype_from_a_stream() + { + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(ProbeWithDoctype)); + + Should.Throw(() => MTConnectProbeParser.Parse(stream)); + } + + /// A write answered with a DOCTYPE'd document cannot be read, so it is NOT reported as accepted. + [Fact] + public async Task A_write_answered_with_a_doctype_document_is_not_confirmed() + { + const string body = + """ + ]> + &e; + """; + using var client = NewClient(new StubHandler(_ => Respond(HttpStatusCode.OK, body))); + + var outcome = await client.WriteObservationAsync("d", "item", "v", Ct); + + outcome.Accepted.ShouldBeFalse(); + outcome.AgentError.ShouldNotBeNull().ShouldContain("DOCTYPE"); + } + + // ───────────── gap 5: no fabricated non-finite floats ───────────── + // .NET parses an overflowing literal to ±Infinity and accepts NaN/Infinity spellings; all were Good. + + [Theory] + [InlineData(DriverDataType.Float64, "1e400", 0x803C0000u)] // BadOutOfRange — a number the type cannot hold + [InlineData(DriverDataType.Float64, "-1e400", 0x803C0000u)] + [InlineData(DriverDataType.Float32, "1e39", 0x803C0000u)] // fits a double, overflows a float + [InlineData(DriverDataType.Float64, "NaN", 0x80740000u)] // BadTypeMismatch — not a measurement + [InlineData(DriverDataType.Float64, "Infinity", 0x80740000u)] + [InlineData(DriverDataType.Float32, "-Infinity", 0x80740000u)] + public void A_non_finite_float_is_never_served_as_good(DriverDataType type, string text, uint expected) + { + var snap = Coerce(type, text); + + snap.StatusCode.ShouldBe(expected); + snap.Value.ShouldBeNull(); + } + + // ───────────── gap 6: an integer-valued decimal on an integral tag ───────────── + // Agents format integer-valued items as "42.000000"; that was BadOutOfRange on every integral tag. + + [Theory] + [InlineData(DriverDataType.Int32, "42.000000", 42)] + [InlineData(DriverDataType.Int64, "1.5e3", 1500L)] + [InlineData(DriverDataType.Int16, "-7.0", (short)-7)] + [InlineData(DriverDataType.UInt32, "4000000000.0", 4000000000u)] + public void An_integer_valued_decimal_converts_losslessly(DriverDataType type, string text, object expected) + { + var snap = Coerce(type, text); + + snap.StatusCode.ShouldBe(0u); + snap.Value.ShouldBe(expected); + } + + /// …but a REAL fraction is still refused, never truncated (deliberately stricter than the Java driver). + [Theory] + [InlineData(DriverDataType.Int32, "42.5")] + [InlineData(DriverDataType.Int16, "40000.0")] + [InlineData(DriverDataType.UInt16, "-1.0")] + [InlineData(DriverDataType.Int64, "1e30")] + public void A_fraction_or_an_out_of_range_decimal_stays_out_of_range(DriverDataType type, string text) + { + var snap = Coerce(type, text); + + snap.StatusCode.ShouldBe(0x803C0000u); + snap.Value.ShouldBeNull(); + } + + private static DataValueSnapshot Coerce(DriverDataType type, string text) + { + var index = new MTConnectObservationIndex([new MTConnectTagDefinition("x", type)]); + index.Apply(new MTConnectStreamsResult( + 1L, 2L, 1L, [new MTConnectObservation("x", text, new DateTime(2026, 11, 10, 12, 0, 0, DateTimeKind.Utc))])); + + return index.Get("x"); + } + + // ───────────── gap 7: sibling containers resolving to one folder name ───────────── + // Two same-named sibling folders share one path key in the browse picker; the second one's data items were + // listed but unreachable on expand. Merged into ONE folder (what the Java driver does). + + private const string CollidingProbe = + """ + +
+ + + + + + + + + + + + + + + + """; + + [Fact] + public async Task Sibling_components_resolving_to_one_name_share_one_folder_and_keep_every_data_item() + { + var client = CannedAgentClient.FromFixtures(); + client.Probe = MTConnectProbeParser.Parse(CollidingProbe); + var driver = new MTConnectDriver(new MTConnectDriverOptions { AgentUri = "http://agent:5000" }, "mt1", _ => client); + await driver.InitializeAsync("{}", Ct); + + var cap = new CapturingBuilder(); + await driver.DiscoverAsync(cap, Ct); + + cap.Folders.Select(f => f.Path).ShouldBe(["Mill", "Mill/Axes", "Mill/Axes/C", "Mill/Axes/X"]); + cap.Variables.Single(v => v.Attr.FullName == "c_angle").ParentPath.ShouldBe("Mill/Axes/C"); + cap.Variables.Single(v => v.Attr.FullName == "c_pos").ParentPath.ShouldBe("Mill/Axes/C"); + cap.Variables.Single(v => v.Attr.FullName == "x_pos").ParentPath.ShouldBe("Mill/Axes/X"); // id fallback + cap.Variables.Single(v => v.Attr.FullName == "x2_pos").ParentPath.ShouldBe("Mill/Axes/X"); // name + } + + // ───────────── gap 8: structural elements are not components ───────────── + + [Fact] + public void A_structural_element_under_components_or_devices_is_skipped_not_fatal() + { + const string xml = + """ + +
+ + agent-wide note + + + stray, schema-invalid + + + + + + """; + + var model = MTConnectProbeParser.Parse(xml); + + model.Devices.ShouldHaveSingleItem().Components.ShouldHaveSingleItem().Id.ShouldBe("ctl"); + } + + // ───────────── plumbing ───────────── + + private static HttpResponseMessage Respond(HttpStatusCode status, string body) => + new(status) { Content = new StringContent(body, Encoding.UTF8, "application/xml") }; + + private sealed class StubHandler(Func respond) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken ct) => + Task.FromResult(respond(request)); + } + + // ───────────── Java regression scenarios this driver already met (coverage, not gaps) ───────────── + // Ported from the Java MTConnectMultipartReaderTest / MTConnectStreamsParserTest cases that had no + // .NET counterpart. Each passed before #804; they pin behaviour the matrix records as "covered". + + private static string Doc(long first, long next, string observations = "") => Streams( + $"""{observations}""") + .Replace("firstSequence=\"1\"", $"firstSequence=\"{first}\"").Replace("nextSequence=\"6\"", $"nextSequence=\"{next}\""); + + private static HttpResponseMessage MultipartResponse(byte[] body) => MultipartResponse(new ByteArrayContent(body)); + + private static HttpResponseMessage MultipartResponse(HttpContent content) + { + content.Headers.ContentType = + System.Net.Http.Headers.MediaTypeHeaderValue.Parse("multipart/x-mixed-replace; boundary=b0"); + + return new HttpResponseMessage(HttpStatusCode.OK) { Content = content }; + } + + private static async Task> CollectAsync(MTConnectAgentClient client, int count) + { + var results = new List(); + await foreach (var chunk in client.SampleAsync(1, Ct)) + { + results.Add(chunk); + if (results.Count == count) + { + break; + } + } + + return results; + } + + /// Java contentLengthIsHonoredOverBoundaryScanning: a document whose text contains the boundary token is returned whole. + [Fact] + public async Task A_declared_content_length_is_authoritative_over_a_boundary_token_inside_the_document() + { + var doc = Doc(1, 2, "see\r\n--b0 inside the text"); + var payload = Encoding.UTF8.GetBytes(doc); + var body = Encoding.ASCII.GetBytes($"--b0\r\nContent-type: text/xml\r\nContent-length: {payload.Length}\r\n\r\n") + .Concat(payload).Concat("\r\n--b0--\r\n"u8.ToArray()).ToArray(); + using var client = NewClient(new StubHandler(_ => MultipartResponse(body))); + + var chunk = (await CollectAsync(client, 1)).ShouldHaveSingleItem(); + + chunk.Observations.ShouldHaveSingleItem().Value.ShouldContain("--b0 inside the text"); + } + + /// Java preambleBeforeTheFirstBoundaryIsSkipped + lineFeedOnlyFramingIsAccepted. + [Fact] + public async Task A_preamble_is_skipped_and_line_feed_only_framing_is_accepted() + { + var first = Encoding.UTF8.GetBytes(Doc(1, 2)); + var second = Encoding.UTF8.GetBytes(Doc(2, 3)); + var body = Encoding.ASCII.GetBytes("this is preamble text\n--b0\nContent-length: " + first.Length + "\n\n") + .Concat(first) + .Concat(Encoding.ASCII.GetBytes("\n--b0\nContent-length: " + second.Length + "\n\n")) + .Concat(second) + .Concat("\n--b0--\n"u8.ToArray()).ToArray(); + using var client = NewClient(new StubHandler(_ => MultipartResponse(body))); + + var chunks = await CollectAsync(client, 2); + + chunks.Select(c => c.NextSequence).ShouldBe([2L, 3L]); + } + + /// Java aBodyShorterThanItsDeclaredContentLengthThrows: truncation is a lost stream, never an orderly end. + [Fact] + public async Task A_part_shorter_than_its_declared_content_length_is_a_lost_stream() + { + var payload = Encoding.UTF8.GetBytes(Doc(1, 2)); + var body = Encoding.ASCII.GetBytes($"--b0\r\nContent-length: {payload.Length + 500}\r\n\r\n").Concat(payload).ToArray(); + using var client = NewClient(new StubHandler(_ => MultipartResponse(body))); + + await Should.ThrowAsync(async () => await DrainAsync(client.SampleAsync(1, Ct))); + } + + /// Java aPartLargerThanTheBufferCapIsRefused: a peer that never frames a part cannot grow the buffer unbounded. + [Fact] + public async Task A_part_larger_than_the_buffer_cap_is_refused() + { + var head = "--b0\r\nContent-type: text/xml\r\n\r\n"u8.ToArray(); + using var client = NewClient(new StubHandler(_ => MultipartResponse(new StreamContent(new EndlessPart(head))))); + + var ex = await Should.ThrowAsync(async () => await DrainAsync(client.SampleAsync(1, Ct))); + + ex.Message.ShouldContain("refusing to buffer further"); + } + + /// Java timestampWithANonZuluOffsetIsNormalizedToTheSameInstant: the SourceTimestamp is the Agent's instant, in UTC. + [Fact] + public void A_timestamp_with_a_non_zulu_offset_is_normalized_to_the_same_utc_instant() + { + var result = MTConnectStreamsParser.Parse( + Doc(1, 2, """ACTIVE""")); + + var at = result.Observations.ShouldHaveSingleItem().TimestampUtc; + at.ShouldBe(new DateTime(2026, 11, 10, 12, 0, 0, 500, DateTimeKind.Utc)); + at.Kind.ShouldBe(DateTimeKind.Utc); + } + + /// + /// Java anAbsurdlyDeepErrorDocumentStillYieldsTheAgentsWords: the new nesting bound applies to the + /// observation walk only, so a deeply nested error document still reports the Agent's own text. + /// + [Fact] + public void An_absurdly_deep_error_document_still_yields_the_agents_words() + { + var nest = string.Concat(Enumerable.Repeat("", 100)); + var unnest = string.Concat(Enumerable.Repeat("", 100)); + var xml = $"""{nest}deep{unnest}"""; + + var ex = Should.Throw(() => MTConnectStreamsParser.Parse(xml)); + + ex.Message.ShouldContain("OUT_OF_RANGE: deep"); + } + + /// A read-only stream that serves head then an endless run of document bytes with no boundary. + private sealed class EndlessPart(byte[] head) : Stream + { + private long _position; + + public override int Read(byte[] buffer, int offset, int count) + { + var written = 0; + while (written < count) + { + buffer[offset + written] = _position < head.Length ? head[_position] : (byte)'a'; + _position++; + written++; + } + + return written; + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position { get => _position; 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(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs index d2bd82563..ccaccf26a 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs @@ -452,6 +452,56 @@ public sealed class MTConnectSubscribeTests For(seen, "dev1_pos").Last().Snapshot.Value.ShouldBe(9.5d); } + /// + /// #804 (Java-driver parity: the restart test runs on the re-baseline snapshot too). On a real 2.x Agent a + /// restart is usually seen FIRST by the re-baseline, not by a chunk: the restarted process refuses the dead + /// one's cursor with HTTP 400 OUT_OF_RANGE (live-observed on the fixture), so no chunk ever reaches the + /// in-stream instanceId check. The re-baseline used to adopt the new id silently — no rediscovery + /// prompt, and the dead model's held values kept being served. + /// + [Fact] + public async Task An_agent_restart_seen_only_by_the_rebaseline_drops_held_values_and_raises_rediscovery() + { + var (driver, client) = await InitializedDriverAsync(); + var raised = new ConcurrentQueue(); + driver.OnRediscoveryNeeded += (_, e) => raised.Enqueue(e); + var seen = Record(driver); + + // The restarted Agent refuses the old cursor, and its /current knows nothing about dev1_execution. + client.SampleFailures.Enqueue(new InvalidDataException("OUT_OF_RANGE: 'from' must be less than 42 (served under HTTP 400)")); + client.CurrentAnswers.Enqueue(ChunkFrom(RestartedInstanceId, 40, 42, ("dev1_pos", "9.5"))); + + await driver.SubscribeAsync(["dev1_pos", "dev1_execution"], TimeSpan.FromMilliseconds(50), Ct); + await client.WaitForSampleCallsAsync(2).WaitAsync(Watchdog, Ct); + + driver.AgentInstanceId.ShouldBe(RestartedInstanceId); + raised.ShouldHaveSingleItem().Reason.ShouldContain("instanceId"); + client.LastSampleFrom.ShouldBe(42); + + driver.ObservationIndex.Get("dev1_execution").StatusCode.ShouldBe(BadWaitingForInitialData); + driver.ObservationIndex.Get("dev1_pos").Value.ShouldBe(9.5d); + For(seen, "dev1_execution").Last().Snapshot.StatusCode.ShouldBe(BadWaitingForInitialData); + } + + /// …control: an OUT_OF_RANGE re-baseline against the SAME Agent raises nothing and keeps its values. + [Fact] + public async Task An_out_of_range_rebaseline_against_the_same_agent_raises_no_rediscovery() + { + var (driver, client) = await InitializedDriverAsync(); + var raised = new ConcurrentQueue(); + driver.OnRediscoveryNeeded += (_, e) => raised.Enqueue(e); + + client.SampleFailures.Enqueue(new InvalidDataException("OUT_OF_RANGE (served under HTTP 400)")); + client.CurrentAnswers.Enqueue(Chunk(5000, 5005, ("dev1_pos", "2.5"))); + + await driver.SubscribeAsync(["dev1_pos", "dev1_execution"], TimeSpan.FromMilliseconds(50), Ct); + await client.WaitForSampleCallsAsync(2).WaitAsync(Watchdog, Ct); + + raised.ShouldBeEmpty(); + driver.AgentInstanceId.ShouldBe(FixtureInstanceId); + driver.ObservationIndex.Get("dev1_execution").StatusCode.ShouldBe(Good); + } + // ---- stream ends ---- /// @@ -1116,6 +1166,85 @@ public sealed class MTConnectSubscribeTests driver.ReconnectAttempts.ShouldBe(0); } + /// + /// #804 (Java-driver parity: MAX_REBASELINES_WITHOUT_PROGRESS). An Agent that answers EVERY + /// /sample open with OUT_OF_RANGE (live: a count at or above its bufferSize) is + /// re-baselined from /current each time, and the backoff paces that — but every re-baseline also marked + /// the stream delivered, so the driver read Healthy for ever while not one /sample chunk arrived. + /// After in a row it is Degraded and stays so + /// across further re-baselines, with LastSuccessfulRead still advancing (each /current is the + /// Agent answering — the comms-loss watchdog must not reconnect-loop a configuration problem). + /// + [Fact] + public async Task Repeated_rebaselines_without_a_delivered_chunk_report_the_stream_degraded() + { + var options = Opts(new MTConnectReconnectOptions { MinBackoffMs = 0, MaxBackoffMs = 1 }); + var (driver, client) = await InitializedDriverAsync(options: options); + client.SampleFailure = new InvalidDataException("MTConnectError OUT_OF_RANGE: 'count' must be less than 65 (served under HTTP 400)"); + + var handle = await driver.SubscribeAsync(["dev1_pos"], TimeSpan.FromMilliseconds(50), Ct); + try + { + // Well past the cap, so the hold across LATER re-baselines is what is asserted, not a lucky instant. + await client.WaitForSampleCallsAsync(MTConnectDriver.MaxRebaselinesWithoutProgress + 3).WaitAsync(Watchdog, Ct); + var stalledAt = driver.GetHealth(); + + stalledAt.State.ShouldBe(DriverState.Degraded); + stalledAt.LastError.ShouldBe(MTConnectDriver.StalledStreamReason); + stalledAt.LastSuccessfulRead.ShouldNotBeNull(); + + // LastSuccessfulRead keeps advancing while it is stalled: the /current answers are real. + await client.WaitForSampleCallsAsync(client.SampleCallCount + 2).WaitAsync(Watchdog, Ct); + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + driver.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThanOrEqualTo(stalledAt.LastSuccessfulRead!.Value); + + // Positive control: the Agent starts streaming again — one delivered chunk clears the verdict. + client.SampleFailure = null; + await client.PumpAsync(Chunk(PrimedNextSequence, PrimedNextSequence + 1, ("dev1_pos", "9.5"))) + .WaitAsync(Watchdog, Ct); + await WaitForHealthAsync(driver, DriverState.Healthy, Ct); + } + finally + { + await driver.UnsubscribeAsync(handle, Ct).WaitAsync(Watchdog, Ct); + } + } + + /// + /// …the falsification control: FEWER than the cap is an ordinary recovery and must stay Healthy (a genuine + /// ring-buffer overrun is resolved by one re-baseline). + /// + [Fact] + public async Task A_rebaseline_short_of_the_cap_stays_healthy() + { + var (driver, client) = await InitializedDriverAsync(); + client.SampleFailures.Enqueue(new InvalidDataException("OUT_OF_RANGE (served under HTTP 400)")); + + var handle = await driver.SubscribeAsync(["dev1_pos"], TimeSpan.FromMilliseconds(50), Ct); + try + { + await client.WaitForSampleCallsAsync(2).WaitAsync(Watchdog, Ct); + await client.PumpAsync(Chunk(PrimedNextSequence, PrimedNextSequence + 1, ("dev1_pos", "1.5"))) + .WaitAsync(Watchdog, Ct); + + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + finally + { + await driver.UnsubscribeAsync(handle, Ct).WaitAsync(Watchdog, Ct); + } + } + + private static async Task WaitForHealthAsync(MTConnectDriver driver, DriverState state, CancellationToken ct) + { + using var watchdog = CancellationTokenSource.CreateLinkedTokenSource(ct); + watchdog.CancelAfter(Watchdog); + while (driver.GetHealth().State != state) + { + await Task.Delay(5, watchdog.Token).ConfigureAwait(false); + } + } + /// /// I3 — a non-positive cap is an operator-authorable hot loop. Clamping to it (the /// obvious Math.Max(0, …)) makes EVERY delay zero, so the pump reconnect-spins as fast From f17ff1e64395ec66246375ee60a622a7aa1b1e61 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 10 Nov 2026 23:08:42 -0500 Subject: [PATCH 2/2] docs(mtconnect): Java reference parity matrix + live validation record (#804) --- .claude/rules/drivers.md | 8 ++ docs/drivers/MTConnect.md | 167 +++++++++++++++++++++++++++++++++++--- 2 files changed, 164 insertions(+), 11 deletions(-) diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index a90a8e34c..1e7dc9118 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -291,6 +291,14 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa - **MTConnect (#627/#628):** TIME_SERIES reads as a variable-length `double[]` (Float64 only); a count disagreeing with `sampleCount` is `BadDecodingError`. One MTConnect driver = one agent client = at most one `/raw` Device (`MTConnectMultipleDevices` deploy error; `RawTreeService.CreateDeviceAsync` refuses a second). +- **MTConnect Java-parity review (#804):** a 2.x Agent serves its error documents under **non-2xx** (`OUT_OF_RANGE` + = HTTP 400, `NO_DEVICE` = 404); the client lifts them (≤ 64 KiB read) into the refusal-marked + `InvalidDataException` — before #804 the 400 was a transport fault and the `OUT_OF_RANGE` re-baseline never ran + live. The restart (`instanceId`) test also runs on every re-baseline snapshot (on 2.x that is where a restart is + first seen). 3 re-baselines with no `/sample` chunk ⇒ Degraded with `LastSuccessfulRead` still advancing (no + watchdog reconnect loop over a `SampleCount` ≥ `bufferSize`). Every parse prohibits DTDs; the streams walk is + depth-bounded (64 — an unbounded recursion is a process-killing `StackOverflowException`). Non-finite floats are + never Good; integer-valued decimals serve integral tags losslessly. Matrix: `docs/drivers/MTConnect.md`. - **MTConnect CONDITION alarms (#626):** a CONDITION raw tag is a native Part 9 condition only when its `TagConfig` carries the standard `alarm` object (opt-in — alarm tags leave the value subscription, so auto-promotion would break existing String tags). Normal→inactive, Warning→500, Fault→800; one condition per DataItem showing the most severe diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index 42cf80ca5..3731bbb23 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -60,6 +60,16 @@ the whole vendor component (and every DataItem beneath it) rather than fail loudly. Attributes are read **unqualified** for the same reason — so an `xsi:type` attribute is never mistaken for a DataItem's own `type`. +**Hardened against the peer (#804).** The Agent is unauthenticated, so every parse +refuses a `DOCTYPE` outright (`DtdProcessing.Prohibit` — the `/probe` and write-answer +parses used to go through `XDocument.Parse`, which *parses* DTDs and expands internal +entities), the streams walk refuses nesting deeper than 64 levels (it recurses per +level, and an unbounded depth is a `StackOverflowException` .NET cannot catch — the +whole server process, not the driver, would die), and a structural element +(`Description`/`Configuration`/`References`/`Compositions`/`DataItems`) sitting +directly under `` or `` is skipped rather than failing the whole +probe on its missing `id`. + ## Project split | Project | Target | Role | @@ -127,7 +137,8 @@ checks the answer's `instanceId`: a changed one (an Agent restart seen by a brow cached `/probe` model and raises `OnRediscoveryNeeded` once per distinct id — it used to be detected only by the pump. **An Agent refusal is a reply (health contract).** An `MTConnectError` document answering `/current` (e.g. -`NO_DEVICE` for a wrong `DeviceName` scope) still fails the batch (`BadCommunicationError` per ref), but it is the +`NO_DEVICE` for a wrong `DeviceName` scope — a 2.x Agent serves it under **HTTP 404**, an older one under 200; +both are lifted since #804) still fails the batch (`BadCommunicationError` per ref), but it is the Agent answering, not a comms fault: the read path is not degraded by it (that would let the comms-loss watchdog reconnect-loop the driver over a configuration problem), `LastSuccessfulRead` advances, a prior read degradation clears, and the Agent's text is logged once per distinct refusal. An answer that is not an MTConnect document at all @@ -258,12 +269,17 @@ item. A `CONDITION` observation's value is taken from its **element name** condition element normalises onto the same `UNAVAILABLE` sentinel as every other category so one comparison covers all three. +**Integer-valued decimals are served on integral tags (#804).** Agents format integer-valued items as +`42.000000`; an `Int*`/`UInt*` tag accepts any decimal whose value *is* an integer in range (`42.000000` → `42`, +`1.5e3` → `1500`) losslessly. A real fraction stays `BadOutOfRange` — deliberately stricter than the Java +reference driver, which truncates toward zero. + Other status codes an observation can surface: | Code | Meaning | |---|---| -| `BadTypeMismatch` (`0x80740000`) | The Agent's text isn't a value of the tag's coerced type at all | -| `BadOutOfRange` (`0x803C0000`) | The Agent reported a number the coerced type can't represent | +| `BadTypeMismatch` (`0x80740000`) | The Agent's text isn't a value of the tag's coerced type at all — including a `NaN`/`Infinity` spelling on a float tag (#804; it used to be served Good) | +| `BadOutOfRange` (`0x803C0000`) | The Agent reported a number the coerced type can't represent — an integer overflow, a real fraction on an integral tag (`3.5` is refused, never truncated), or a float literal that overflows to ±Infinity (`1e400`; #804, it used to be served as a Good Infinity) | | `BadDecodingError` (`0x80070000`) | A `TIME_SERIES` vector whose element count disagrees with the observation's own `sampleCount` (or the tag's fixed `ArrayDim`) — see [TIME_SERIES](#time_series-arrays-627) | | `BadNotSupported` (`0x803D0000`) | A shape this build doesn't materialize — a `DATA_SET`/`TABLE` observation whose children are not `` elements (a well-formed one is served as JSON since #650), or an array tag declared as anything other than Float64 | | `BadWaitingForInitialData` | Tag is authored but the Agent hasn't reported it yet | @@ -332,15 +348,41 @@ what the driver's cursor expects) is detected **two ways**, both triggering a 1. `IMTConnectAgentClient.IsSequenceGap` — the next chunk's `firstSequence` is newer than the driver's expected cursor. -2. An `MTConnectError` document reporting `OUT_OF_RANGE` — real Agents return - this both under HTTP 200 (a normal MTConnect error document) and as a bare - HTTP 400, and the client handles both. +2. An `MTConnectError` document reporting `OUT_OF_RANGE` — older Agents serve it + under HTTP 200; a 2.x Agent (the fixture's `mtconnect/agent:2.7`) serves it + under **HTTP 400**. ⚠️ **Until #804 only the 200 shape was handled**: the client + threw a bare `HttpRequestException` for the 400, the pump read it as transport + loss and reconnected at the same evicted cursor for ever, so this leg never ran + against a real 2.x Agent (the comms-loss watchdog's re-initialize was the only + escape). The client now reads up to 64 KiB of any non-2xx body and lifts an + `MTConnectError` into the same refusal-marked `InvalidDataException` the 200 + shape produces; a non-MTConnect body (a proxy page) stays a status-only + `HttpRequestException`. + +**Re-baselines without progress are bounded *and visible* (#804).** The reconnect +backoff already paced a stream that only ever re-baselines (`A_rebaseline_that_delivers_nothing…`), +but every `/current` also marked the stream delivered, so the driver read **Healthy** +for ever while no `/sample` chunk arrived — the live case is a `SampleCount` at or +above the Agent's `bufferSize`, refused `OUT_OF_RANGE` (`'count' must be less than N`) +on every open. After **3** consecutive re-baselines with no chunk in between +(`MaxRebaselinesWithoutProgress`, the Java driver's `MAX_REBASELINES_WITHOUT_PROGRESS`) +the stream is **Degraded** with `LastError = "repeated /sample re-baselines with no +observations in between"` and one Warning naming the `bufferSize` cause; it stays +Degraded across further re-baselines until a chunk is delivered. Values keep flowing +off the re-baselines and `LastSuccessfulRead` keeps advancing — each `/current` is the +Agent answering — so the comms-loss watchdog does not reconnect-loop a configuration +problem. Size `SampleCount` below the Agent's `bufferSize` (in the `/probe` header). **An Agent `instanceId` change means the Agent restarted**, and is checked **before** the sequence-gap check (a restart also usually trips the gap, so the two need disambiguating, not just OR-ing together). On a changed `instanceId` the driver clears its cached probe model and raises -`OnRediscoveryNeeded`, which surfaces a **re-browse prompt** on the AdminUI +`OnRediscoveryNeeded`. **The test also runs on every re-baseline's `/current` +(#804)** — on a 2.x Agent that is usually where a restart is first seen, because the +restarted process refuses the dead one's cursor with HTTP 400 `OUT_OF_RANGE` and no +chunk ever reaches the in-stream check; the re-baseline used to adopt the new id +silently (live-confirmed on the fixture). Either way the driver drops every held +observation (subscribers are told) and raises the rediscovery signal, which surfaces a **re-browse prompt** on the AdminUI `/hosts` page — see [Known limitations](#known-limitations) for what that does and does not do. @@ -472,7 +514,11 @@ and whose instance reports online discovery, renders the AdminUI **Browse** button, constructs the driver, runs `InitializeAsync` (the `/probe` connect) + `DiscoverAsync` into a `CapturingAddressSpaceBuilder`, then tears the throwaway instance down. Each captured leaf's NodeId is the DataItem `id`, -committed directly as `TagConfig.FullName` on pick. See +committed directly as `TagConfig.FullName` on pick. Sibling containers that resolve to +the **same folder name** (two components named alike, or one's `name` equal to another's +`id`) share **one** folder (#804): the picker keys folders by path, so two same-named +siblings used to list both but expand only the first one's children. A DataItem named +like a sibling folder is not a collision — leaves are keyed by DataItem id. See [`docs/plans/2026-07-15-universal-discovery-browser-design.md`](../plans/2026-07-15-universal-discovery-browser-design.md). ## CONDITION alarms (#626) @@ -571,6 +617,106 @@ table plus an unknown-value fallback, a per-type enum node shape through discove and it would break every consumer that reads these items as String (ScadaBridge) for little gain. Reopen only with an opt-in, tag-level `enumValues` design. +## Java reference parity (#804) + +Gitea #804 reviewed this driver against the read-only Java MTConnect driver in the `ignitionoee` repo +(`modules/drivers/gateway/src/{main,test}/java/com/zb/mom/driver/mtconnect/`, live-validated on Ignition +2026-08-10). That Java driver is largely a port of this one, so most rows are **covered**; the review found +nine concrete gaps, all closed on `feat/804-mtconnect-parity`. Code paths below are relative to +`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/`, tests to `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/` +(unit) and `…MTConnect.IntegrationTests/` (live); Java classes are `MTConnect*.java` in the package above. The +Java driver's live evidence does not validate this one — this driver's own live results are at the end. + +**Status key:** **covered** — .NET already did it (test named); **gap → closed** — verified missing, fixed in +#804 with a test that failed first; **.NET exceeds** — beyond the Java baseline, kept; **divergence** — +deliberately different, kept. + +### Transport, protocol and configuration + +| Java capability (class) | .NET status | .NET code · tests | +|---|---|---| +| HTTP/HTTPS Agent base URI, http/https only (`MTConnectConfigValidator`) | covered | `MTConnectAgentClient.ResolveAgentRoot` · `MTConnectFactoryTests.Malformed_or_non_http_agentUri_returns_not_ok` | +| Optional device scope `{agentUri}/{deviceName}/…` (`MTConnectAgentClient`) | covered (+ a belt-and-braces scope match in discovery) | `BuildRequestUri`, `MTConnectDriver.InDeviceScope` · `MTConnectDiscoverTests.Discover_with_a_device_scope_*` | +| `/probe` at connect, fault-fast (`MTConnectDevice.connect`) | covered | `StartCoreAsync` (initialize fails → Faulted) · `MTConnectDriverLifecycleTests` | +| `/current` snapshot; `/sample` multipart long poll with `from`/`interval`/`count`/`heartbeat` | covered | `CurrentAsync`/`SampleAsync` · `MTConnectStreamsParseTests.SampleAsync_composes_the_from_interval_count_and_heartbeat_query` | +| Strictly-positive timing knobs (`requirePositive`) | covered | `MTConnectDriver.RequirePositive` · `MTConnectDriverOptionsTests` | +| One HTTP client per device, not per reconnect (`SharedHttp`) | covered — two `HttpClient`s per agent client, rebuilt only on a request-shaping config change | `RequiresClientRebuild` · `MTConnectDriverLifecycleTests.Reinitialize_with_a_changed_device_scope_rebuilds_the_client` | +| Agent error document under a **non-2xx** status lifted (`probeAgentError`/`streamsAgentError`) — a 2.x Agent serves `OUT_OF_RANGE` as **400** and `NO_DEVICE` as **404** | **gap → closed** | `EnsureAgentAnsweredAsync`, `MTConnectXml.AgentRefusalFromErrorBody` · `MTConnectJavaParityTests.Sample_answered_400_…`, `Current_answered_404_…`, `Probe_answered_404_…`; live `A_sample_cursor_outside_the_buffer_is_the_agents_out_of_range_refusal` | +| Failure body read only to a cap (`MAX_ERROR_BODY_BYTES`, 64 KiB) | **gap → closed** (with the row above) | `MaxErrorBodyBytes` · `An_error_document_beyond_the_body_cap_is_not_read_in_full`, `A_non_2xx_proxy_page_is_still_a_plain_http_failure` | +| Demand-read TTL cache for un-monitored reads (`MTConnectDemandSource`, `demandReadTtlMs`) | covered, different model | reads answer from the live pump index (no round trip) or one `/current` per **batch**; the server never calls `ReadAsync` on its hot path · `MTConnectReadPathTests` | +| No credentials / no `SecretConfig` | divergence | a `user:pass@` URI is accepted and **redacted** everywhere (`MTConnectUriRedaction`) · `MTConnectUriRedactionTests` | + +### Parsing, browse and values + +| Java capability (class) | .NET status | .NET code · tests | +|---|---|---| +| Match on LocalName, read attributes unqualified (`MTConnectXml` rules 1–2) | covered | `MTConnectXml` · `Probe_parse_is_agnostic_to_the_document_namespace_version`, `Probe_parse_keeps_a_vendor_extension_component_in_its_own_namespace`, `Parse_reads_only_unqualified_attributes` | +| DOCTYPE refused on **every** parse (`disallow-doctype-decl`) | **gap → closed** for `/probe` and write answers (the streams parser already prohibited it) | `MTConnectXml.ParseHardened`/`LoadHardened` · `Probe_parse_refuses_a_doctype_from_a_string`/`…_stream`, `A_write_answered_with_a_doctype_document_is_not_confirmed` | +| Component nesting bound 64 (`MAX_COMPONENT_DEPTH`) | covered | `MTConnectProbeParser.MaxComponentDepth` · `MTConnectProbeParseTests` | +| Streams element nesting bound 64 (`MAX_ELEMENT_DEPTH`) | **gap → closed** — the walk recursed without bound: an uncatchable `StackOverflowException`, i.e. a server-process crash | `MTConnectStreamsParser.MaxElementDepth` · `Streams_parse_refuses_absurd_nesting_…`, controls `Streams_parse_accepts_ordinary_device_nesting`, `An_absurdly_deep_error_document_still_yields_the_agents_words` | +| Structural elements under ``/`` skipped (`NOT_A_COMPONENT`) | **gap → closed** (one stray `` failed the whole probe) | `MTConnectProbeParser.ComponentLikeChildren` · `A_structural_element_under_components_or_devices_is_skipped_not_fatal` | +| Malformed XML, missing header, missing `dataItemId` throw | covered | `Probe_parse_throws_on_malformed_xml`, `Parse_throws_on_malformed_xml`, `Parse_throws_on_a_malformed_observation` | +| BOM / framing whitespace tolerated on a chunk | covered | `Parse_from_bytes_tolerates_the_multipart_frames_trailing_line_break` | +| Multipart framing: Content-length authoritative, boundary-scan fallback, preamble, LF-only, split reads, truncation throws, 32 MiB part cap (`MTConnectMultipartReader`) | covered | `MultipartStreamReader` · `SampleAsync_frames_*`, `A_part_without_content_length_warns_once…`; Java cases ported as `A_declared_content_length_is_authoritative…`, `A_preamble_is_skipped_and_line_feed_only_framing_is_accepted`, `A_part_shorter_than_its_declared_content_length_is_a_lost_stream`, `A_part_larger_than_the_buffer_cap_is_refused` | +| Typed Device → Component → DataItem tree; `name` else `id`; leaf ref = DataItem id (`MTConnectBrowseTree`) | covered | `DiscoverAsync`/`StreamContainer` · `MTConnectDiscoverTests` | +| Sibling containers resolving to one name share **one** folder | **gap → closed** (the picker listed both, expanded only the first) | `MTConnectDriver.SiblingFolders` · `Sibling_components_resolving_to_one_name_share_one_folder_and_keep_every_data_item` | +| DataItem named like a sibling component (leaf-before-folder ordering) | covered — not a collision here: leaves are keyed by DataItem id, folders by path | `Commons/Browsing/CapturingAddressSpaceBuilder` | +| Bounded tree (`maxBrowseNodes` per-folder cap + truncation flag) | covered, different mechanism | the browse capture caps the whole tree at 50 000 nodes and flags `Truncated`; the *served* address space is authored raw tags, not the browse | +| Display-name trimming (`segmentsAreTrimmed`) | divergence (not closed) | names are used verbatim — cosmetic, never seen on a real Agent | +| Type inference SAMPLE→Double, numeric EVENT→Int64, else String (`MTConnectTypes`) | covered, **.NET exceeds** (TIME_SERIES → `double[]`, separator-insensitive types) | `MTConnectDataTypeInference` · `MTConnectDataTypeInferenceTests` | +| UNAVAILABLE → `Bad_NoCommunication`, null value, source timestamp kept (`MTConnectValues`) | covered | `MTConnectObservationIndex.BuildSnapshot` · `Unavailable_observation_maps_to_BadNoCommunication_with_null_value`, `Unavailable_observation_keeps_the_agent_source_timestamp` | +| UNAVAILABLE matched case-insensitively | divergence (kept) | ordinal match on the parser-normalised token, so a free-text EVENT reading "Unavailable" stays a value · `Unavailable_sentinel_is_matched_on_the_exact_literal` | +| Blank text on a String node is a Good empty string | divergence (kept) | empty text is `BadNoCommunication` (an empty element is not an empty-string value) | +| No fabricated numbers: narrow-integer range checks, float overflow ≠ Infinity, NaN/Infinity refused (`MTConnectValues.coerce`) | covered for integers; **gap → closed** for floats (`1e400`, `NaN`, `Infinity`, `1e39` on Float32 were served Good) | `Coerce`, `NonFiniteStatus` · `A_non_finite_float_is_never_served_as_good` | +| Integer-valued decimal (`42.000000`) on an integral node | **gap → closed** (was `BadOutOfRange`) — lossless only; a real fraction is still refused, never truncated (Java truncates) | `TryIntegral` · `An_integer_valued_decimal_converts_losslessly`, control `A_fraction_or_an_out_of_range_decimal_stays_out_of_range` | +| Source timestamps preserved; a non-Zulu offset normalised | covered | `ReadTimestampUtc` · `A_timestamp_with_a_non_zulu_offset_is_normalized_to_the_same_utc_instant` | +| An unparseable timestamp falls back to *now* | divergence (kept) | the chunk is refused (`InvalidDataException` → re-baseline): a fabricated SourceTimestamp is never served | +| Transport failure distinct from UNAVAILABLE | covered | `BadCommunicationError` per ref on a failed read; the host fans `BadNoCommunication` on comms loss (#481) | + +### Stream lifecycle + +| Java capability (class) | .NET status | .NET code · tests | +|---|---|---| +| Running cursor; gap = `firstSequence > expected`, strictly (`MTConnectPump.onDocument`) | covered | `IsSequenceGap` · `Sequence_gap_is_strictly_firstSequence_greater_than_expected_from`, `Sequence_gap_triggers_exactly_one_current_rebaseline_then_resumes` | +| `instanceId` checked before the gap; a restart drops held values | covered | `ConsumeStreamAsync` · `Agent_restart_clears_the_index_and_is_detected_before_the_gap_path` | +| The restart test also runs on the re-baseline snapshot | **gap → closed** — on a 2.x Agent a restart is usually seen ONLY here (the old cursor gets a 400) | `RebaselineAsync`, `TryDropForAgentRestart` · `An_agent_restart_seen_only_by_the_rebaseline_…`, control `An_out_of_range_rebaseline_against_the_same_agent_raises_no_rediscovery` | +| Restart → reconnect + re-probe + rebuild the tree | divergence (v3 design) | re-baseline in place + an `IRediscoverable` re-browse prompt; the served tree is authored, never grafted (Known limitations 1) | +| `OUT_OF_RANGE` (200 **or** 400) → re-baseline (`onError`) | covered for 200; **gap → closed** for 400 (Transport table) | the `InvalidDataException` arm of `ConsumeStreamAsync` · `Out_of_range_stream_error_also_triggers_a_rebaseline` | +| Any other Agent error → reconnect, not re-baseline | divergence | every unusable answer re-baselines; the backoff ladder and the no-progress cap below bound it | +| Re-baselines without progress bounded (`MAX_REBASELINES_WITHOUT_PROGRESS` = 3) | paced already (backoff); **gap → closed** for visibility — it read Healthy for ever | `MaxRebaselinesWithoutProgress`, `TryHoldStreamStalled` · `Repeated_rebaselines_without_a_delivered_chunk_report_the_stream_degraded`, control `A_rebaseline_short_of_the_cap_stays_healthy`; live `A_sample_count_the_agent_refuses_is_reported_degraded_while_values_keep_flowing` | +| Heartbeat watchdog; any part resets it (`watchdogExpired`, 2 × heartbeat) | covered (2 × `HeartbeatMs` + `RequestTimeoutMs`) | `MultipartStreamReader` idle timeout · `SampleAsync_faults_on_the_heartbeat_watchdog_rather_than_hanging_on_a_silent_agent` | +| Bounded reconnect backoff (fixed 2 s → 30 s, jitter) | covered, configurable (`Reconnect.*`; ladder reset after a delivering stream) | `BackoffFor` · `MTConnectSubscribeTests.Reconnect_backoff_*` | +| A superseded generation speaks for nothing (`PumpState`, `Host.running`) | covered | session generation · `MTConnectPumpGenerationTests` | +| Deterministic cancellation and disposal | covered | `ShutdownAsync`, bounded teardown waits · `Shutdown_stops_the_pump_before_disposing_the_client_and_leaks_nothing`, `Shutdown_while_the_pump_is_mid_rebaseline_does_not_deadlock` | + +### Representations and writes + +| Java capability | .NET status | Notes | +|---|---|---| +| TIME_SERIES / DATA_SET / TABLE **excluded** from the tree (one WARN) | **.NET exceeds** — kept | TIME_SERIES → `double[]` (#627); DATA_SET/TABLE → compact JSON String (#650) · `MTConnectDataSetTests`; live `Discovery_types_samples_as_Float64_and_time_series_as_a_float_array`, `Read_serves_data_set_and_table_as_json_strings` | +| CONDITION → String state word | covered, **.NET exceeds** (opt-in native Part 9 alarms, #626) | [CONDITION alarms](#condition-alarms-626) | +| **No write path** (read-only, no `writeTag()`) | **divergence — flagged, not removed** | `IWritable` (`b29623cf`, 2026-08-05) is the Agent's `AllowPut` **observation injection**: it sets what the Agent *reports* for a non-CONDITION, non-array DataItem, and nothing reaches the controller. It is off at two independent gates (`Write.Enabled`, default false; the Agent's `AllowPut`, default off) and a node is only writable once its raw tag is authored `ReadWrite`. So there is no *controller* write path — but there is an Agent write path the Java driver does not have. See [Write-back](#write-back--observation-injection-off-by-default); rejection is live-tested by `Write_is_refused_without_touching_the_agent_when_write_is_not_enabled` | + +### Live validation (2026-11-10, `mtconnect/agent:2.7.0.12` at `http://10.100.0.35:5000`) + +- **Integration suite: 20/20** (`MTCONNECT_AGENT_ENDPOINT=http://10.100.0.35:5000`): browse, typed discovery, + read, UNAVAILABLE, DATA_SET/TABLE, subscribe, RawPath binding, the write round trip, the Agent's write refusal and + the `write.enabled` rejection — plus the two #804 legs, both of which failed before the fix. +- **Lifecycle harness** — the real `MTConnectDriver` subscribed to `fixture_c_speed`/`fixture_mode`/`fixture_execution` + with `HeartbeatMs` 5000, while the fixture containers were stopped/restarted/paused (all restored and healthy after): + - *Adapter stop:* the Agent marked its items UNAVAILABLE ~12 s later (its own adapter timeout); all three went + `BadNoCommunication` while the driver stayed **Healthy** (the Agent kept answering). *Adapter start:* Good ~3 s later. + - *Agent restart:* one `Connection refused` Degraded tick, the restart seen on the re-baseline, **rediscovery + raised**, Healthy and Good ~2 s after the Agent was back. Before the re-baseline restart fix the same leg recovered + its values but raised **no** rediscovery — that run is what found the gap. + - *Frozen Agent* (`docker pause`): the watchdog fired at 15 s (2 × 5000 + 5000); the driver went Degraded with + `LastSuccessfulRead` frozen (the comms-loss watchdog's evidence) and held its last values (the host stamps them + `BadNoCommunication` on the comms-loss edge, #481); reconnects timed out until *unpause*, then Healthy and Good within ~2 s. + - *Teardown:* unsubscribe + shutdown returned in 1 ms. +- **Deferred live checks:** these legs ran through the driver, **not** through a deployed OtOpcUa server and an + OPC UA client (the docker-dev rig was not used); the no-progress cap was observed via a `SampleCount` above the + Agent's `bufferSize`, not a genuinely lapped ring buffer; no real machine tool or non-cppagent Agent was exercised. + ## Known limitations These are real, not placeholders — read them before relying on the driver @@ -679,7 +825,7 @@ for anything beyond values-and-conditions. ## Testing - **Unit tests** — `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/` - (canned-XML fixtures, no network) — 491/491 at time of writing. Covers + (canned-XML fixtures, no network) — 714/714 at #804 (2026-11-10). `MTConnectJavaParityTests` holds the #804 gap tests and the ported Java regression cases (see [Java reference parity](#java-reference-parity-804)). Covers discovery tree shape, `MTConnectDataTypeInference`, observation indexing, `UNAVAILABLE` → `BadNoCommunication`, CONDITION state-word mapping, multipart chunk framing, and ring-buffer re-baseline paging (both the @@ -690,8 +836,7 @@ for anything beyond values-and-conditions. - **Integration tests** — `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/` — env-gated on `MTCONNECT_AGENT_ENDPOINT` (default `http://10.100.0.35:5000`), skips - cleanly (12 Skipped) when the fixture is unreachable, 12/12 against a real - Agent. Read the fixture's own + cleanly when the fixture is unreachable, 20/20 against the real fixture Agent at #804. Read the fixture's own [`Docker/README.md`](../../tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/README.md) for the full recipe — summary only, here: - Image is **`mtconnect/agent:2.7.0.12`** — not `mtconnect/cppagent`, which