Merge #824 MTConnect empty string Good (Java parity); #837 host-name check stays opt-in, toggle in Security panel

Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
2026-11-11 11:58:17 -05:00
parent e857135e8b e6ec5a0948
commit 8c712c5ee0
7 changed files with 200 additions and 26 deletions
@@ -293,7 +293,8 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa
 - **OpcUaClient server host-name check (#816):** the SDK checks the server certificate's host only with
  `checkDomain: true`; its 8-arg `DefaultSessionFactory.CreateAsync` hard-codes `false`, so driver, AdminUI browser and
  Client.Shared never checked before #816. Now `certificateValidation.validateServerHostName` (tri-state, unset = off =
-  unchanged; AdminUI keeps it unset) / CLI `--validate-host-name`. It checks the **configured** host (the rewritten
+  unchanged; AdminUI keeps it unset; **stays opt-in by owner decision #837** — the toggle lives in the form's Security
+panel with the NAT/IPv6-zone/loose-match caveats) / CLI `--validate-host-name`. It checks the **configured** host (the rewritten
  `ConfiguredEndpoint.EndpointUrl`) and auto-accept accepts a mismatch. ⚠️ The SDK checks in TWO places —
  `DefaultSessionFactory.CreateChannelAsync` (GetEndpoints cert, BEFORE any security-mode test, so it fired on `None`)
  and `Session.OpenAsync` — so pass `checkDomain` only for a non-`None` endpoint (`OpcUaEndpointSelection.CheckDomainFor`).
@@ -312,6 +313,11 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa
  the adaptive raised count is clamped to the known `bufferSize`, #846). 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 empty value (#824, Java parity):** an empty/whitespace observation on a string-typed (or unauthored) item is
+  Good `""`; on a non-string item (numeric/bool/DateTime/array) it stays `BadNoCommunication` (no value — not
+  `BadTypeMismatch`). Only the literal `UNAVAILABLE` is the no-value sentinel on a string item; it, the comms-loss
+  fan-out and the condition path are unchanged. A ScadaBridge string tag that used to go Bad on a blank value now
+  receives Good `""`.
 - **MTConnect refused-connection fast path (#846):** a `/sample`/re-baseline failure whose chain holds
  `SocketError.ConnectionRefused`, unbroken for `CommsLossConfirmMs` (default 2000; 0 = first; negative = off), publishes
  every subscribed value `BadNoCommunication` once per outage; the first delivery afterwards republishes all of them from
@@ -271,6 +271,18 @@ 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.

+**An empty value is not `UNAVAILABLE` (#824, owner decision 2026-11-11 — Java parity).** A
+genuinely empty observation (`<Program …></Program>`, `<Program …/>`, or whitespace-only text,
+which the parser trims to `""`) on a **string-typed** item (`String`/`Reference`, or an unauthored
+item, which coerces as String) is served **Good `""`** — an Agent can legitimately report a blank
+`Program`, `Block` or `Message`. On any **non-string** item (numeric, `Boolean`, `DateTime`, a
+`TIME_SERIES` array) `""` cannot be a value, so it stays **`BadNoCommunication`** with a null value
+— deliberately not `BadTypeMismatch`, which would tell the operator the Agent sent text of the wrong
+type and point them at the tag's authored data type; the Agent sent no value at all. Only the literal
+`UNAVAILABLE` sentinel is the "no value" statement on a string item. Before #824 every empty value
+was `BadNoCommunication`. A `DATA_SET`/`TABLE`-shaped observation is unaffected (its entry-less
+form is still Good `{}`; non-`<Entry>` children are still `BadNotSupported`).
+
 **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
@@ -296,7 +308,7 @@ limitations 4; a probe that does declare one yields a fixed `ArrayDim`).
 Parsing: the observation text is split on any whitespace, each token parsed
 invariant-culture. The result is **all-or-nothing** — one non-numeric /
 non-finite token is `BadTypeMismatch`, a literal `UNAVAILABLE` token or an
-empty body is `BadNoCommunication`, and a count that disagrees with the
+empty body is `BadNoCommunication` (an array is non-string, so #824's Good `""` never applies), and a count that disagrees with the
 observation's own `sampleCount` attribute (or a fixed `ArrayDim`) is
 `BadDecodingError`. A partial vector is never served as Good. Only a
 **Float64** array tag is served; any other declared element type stays
@@ -677,7 +689,7 @@ deliberately different, kept.
 | 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) |
+| Blank text on a String node is a Good empty string | **covered** (was a divergence until #824, owner decision 2026-11-11) | `BuildSnapshot` serves Good `""` on a string-typed or unauthored item; a non-string item stays `BadNoCommunication` (Java has no typed node to refuse `""` on); `UNAVAILABLE` unchanged · `Empty_observation_value_on_a_string_item_is_a_Good_empty_string`, `A_parsed_empty_event_on_a_string_item_is_a_Good_empty_string`, controls `Empty_observation_value_on_a_non_string_item_stays_BadNoCommunication`, `Unavailable_on_a_string_item_is_still_BadNoCommunication` |
 | 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` |
@@ -932,7 +944,7 @@ for anything beyond values-and-conditions.
 - **Unit tests** — `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/`
  (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,
+  `UNAVAILABLE` → `BadNoCommunication` (an empty string-typed value is Good `""`, #824), CONDITION state-word mapping,
  multipart chunk framing, and ring-buffer re-baseline paging (both the
  sequence-gap and the `OUT_OF_RANGE` legs). The write path adds
  `MTConnectAgentClientWriteTests` (`9f17d7ef` — verb, URL, agent-error-in-200
@@ -189,8 +189,16 @@ Consequences:
 - Test Connect (`OpcUaClientDriverProbe`) only calls GetEndpoints on an unsecured discovery channel — no session, so no
  certificate or host-name check.

-**Policy: `certificateValidation.validateServerHostName`** (driver JSON; AdminUI driver form "Server host-name check";
-the browser reads the same JSON):
+**Decision (#837, owner, 2026-11-11): the check stays OPT-IN — unset means off, and it will not become a default.**
+Turning it on by default would refuse every NAT / port-published / connect-by-IP endpoint whose certificate does not
+list that exact host, and every link-local IPv6 endpoint, on upgrade; and because the match is loose (first label) it
+would not deliver the identity guarantee a default implies. Operators enable it per driver where trust is CA-issued
+and certificate names are fully-qualified and site-unique. The AdminUI shows the toggle in the driver form's
+**Security** panel (beside mode/policy) as "Server certificate host-name check", with help text covering the NAT/IP
+caveat, IPv6 zone ids and the loose first-label match.
+
+**Policy: `certificateValidation.validateServerHostName`** (driver JSON; AdminUI driver form, Security panel,
+"Server certificate host-name check"; the browser reads the same JSON):

 | Value | Meaning |
 |---|---|
@@ -76,7 +76,10 @@ internal sealed class MTConnectObservationIndex

    private const uint Good = 0x00000000u;

-    /// <summary>The Agent reported <c>UNAVAILABLE</c>, or supplied no text at all.</summary>
+    /// <summary>
+    ///     The Agent reported <c>UNAVAILABLE</c>, or supplied no text at all for a non-string item
+    ///     (an empty string-typed item is a Good <c>""</c> — lmxopcua#824).
+    /// </summary>
    private const uint BadNoCommunication = 0x80310000u;

    /// <summary>The tag is authored but the Agent has not yet reported it.</summary>
@@ -443,10 +446,8 @@ internal sealed class MTConnectObservationIndex
        var serverTimestamp = Now;
        var sourceTimestamp = DateTime.SpecifyKind(observation.TimestampUtc, DateTimeKind.Utc);

-        // No text at all is the degenerate spelling of "no value": MTConnect defines UNAVAILABLE as
-        // the only way to report absence, so an empty element is not an empty string value.
-        if (string.IsNullOrWhiteSpace(observation.Value) ||
-            string.Equals(observation.Value, UnavailableSentinel, StringComparison.Ordinal))
+        // UNAVAILABLE is MTConnect's one explicit "no value" sentinel — always BadNoCommunication.
+        if (string.Equals(observation.Value, UnavailableSentinel, StringComparison.Ordinal))
        {
            return new DataValueSnapshot(null, BadNoCommunication, sourceTimestamp, serverTimestamp);
        }
@@ -456,7 +457,7 @@ internal sealed class MTConnectObservationIndex
        // Publishing that as a Good string would be noise an operator would trust. Checked before
        // the tag lookup because it is a fact about the wire shape, true whether or not the data
        // item is authored — and after the sentinel, so an unavailable data set still reports the
-        // truthful no-comms.
+        // truthful no-comms, but BEFORE the empty-text test (#824).
        if (observation.IsStructured)
        {
            return new DataValueSnapshot(null, BadNotSupported, sourceTimestamp, serverTimestamp);
@@ -464,6 +465,24 @@ internal sealed class MTConnectObservationIndex

        _tags.TryGetValue(observation.DataItemId, out var tag);

+        // A genuinely EMPTY element (no text, or whitespace the parser trimmed away) is NOT the
+        // sentinel. Owner decision on lmxopcua#824 (Java parity): on a string-typed item it is a
+        // Good empty string — an Agent can legitimately report a blank Program/Message/Block.
+        // An unauthored item coerces as String (below), so it is served the same way.
+        // On any other type (numeric, Boolean, DateTime, any array) "" cannot be a value, so it
+        // stays Bad — and stays BadNoCommunication rather than BadTypeMismatch on purpose:
+        // the Agent supplied no value at all (the same meaning as UNAVAILABLE, just spelled
+        // degenerately), whereas BadTypeMismatch tells the operator the Agent sent text of the
+        // wrong type and points them at the tag's authored data type, which is not the fault.
+        // Checked AFTER the structured test above, so element-children with no text stay
+        // BadNotSupported rather than becoming a Good "".
+        if (string.IsNullOrWhiteSpace(observation.Value))
+        {
+            return tag is null or { IsArray: false, DriverDataType: DriverDataType.String or DriverDataType.Reference }
+                ? new DataValueSnapshot(string.Empty, Good, sourceTimestamp, serverTimestamp)
+                : new DataValueSnapshot(null, BadNoCommunication, sourceTimestamp, serverTimestamp);
+        }
+
        // TIME_SERIES vectors arrive as one whitespace-separated string. Checked AFTER the
        // sentinel so an unavailable array tag still reports the truthful no-comms.
        if (tag is { IsArray: true })
@@ -111,6 +111,30 @@
                    }
                </InputSelect>
            </div>
+            @* #816 / #837: opt-in by owner decision (unset = off). Kept in the Security panel so it is
+               found next to the mode/policy it depends on; "Default (off)" leaves the key absent. *@
+            <div class="col-md-4">
+                <label class="form-label" for="opcuaValidateHostName">Server certificate host-name check</label>
+                <InputSelect id="opcuaValidateHostName" @bind-Value="_form.OpcUa.ValidateServerHostName" @bind-Value:after="EmitAsync" class="form-select form-select-sm">
+                    <option value="">Default (off)</option>
+                    <option value="true">Validate configured host</option>
+                    <option value="false">Off (NAT / port publish)</option>
+                </InputSelect>
+            </div>
+            <div class="col-12">
+                <div class="form-text">
+                    <strong>Opt-in (off unless set).</strong> When validating, the server certificate must name the
+                    endpoint host <em>you configured</em>; a miss refuses the connection
+                    (<code>BadCertificateHostNameInvalid</code>). Applies to Sign / SignAndEncrypt only, never to
+                    <code>None</code>, and auto-accept accepts a mismatch. Caveats:
+                    <ul class="mb-0">
+                        <li><strong>NAT / port publish / connecting by IP:</strong> fails unless the certificate lists that exact host or IP — leave off.</li>
+                        <li><strong>IPv6 zone ids</strong> (<code>fe80::1%eth0</code>, link-local) never match — leave off.</li>
+                        <li><strong>Loose match:</strong> the first DNS label alone counts (<code>plc1.site-b.corp</code> passes a certificate for <code>PLC1</code>), so use fully-qualified, site-unique certificate names.</li>
+                    </ul>
+                    Most useful with CA-issued trust; adds little over a pinned self-signed certificate.
+                </div>
+            </div>
        </div>
    </div>
 </section>
@@ -195,15 +219,6 @@
                </div>
                <div class="form-text">Revoked certificates are always rejected.</div>
            </div>
-            <div class="col-md-3">
-                <label class="form-label">Server host-name check</label>
-                <InputSelect @bind-Value="_form.OpcUa.ValidateServerHostName" @bind-Value:after="EmitAsync" class="form-select form-select-sm">
-                    <option value="">Default (off)</option>
-                    <option value="true">Validate configured host</option>
-                    <option value="false">Off (NAT / port publish)</option>
-                </InputSelect>
-                <div class="form-text">Sign / SignAndEncrypt only (never on None): the server certificate must name the endpoint host you configured. Off for link-local IPv6.</div>
-            </div>
            <div class="col-md-6">
                <label class="form-label">Issuer store / CRL directory</label>
                <InputText @bind-Value="_form.OpcUa.IssuerStorePath" @bind-Value:after="EmitAsync" class="form-control form-control-sm mono" />
@@ -320,21 +320,107 @@ public sealed class MTConnectObservationIndexTests
            .ShouldBe(new DateTime(2026, 7, 24, 12, 0, 0, 500, DateTimeKind.Utc));
    }

-    /// <summary>An observation whose text is empty is "the Agent supplied no value" — never a Good empty string.</summary>
+    /// <summary>
+    ///     lmxopcua#824 (Java parity): an empty element on a string-typed item is a Good empty string, not an
+    ///     absence. Whitespace-only text is the same (the parser trims it).
+    /// </summary>
    [Theory]
-    [InlineData("")]
-    [InlineData("   ")]
-    public void Empty_observation_value_maps_to_BadNoCommunication(string value)
+    [InlineData("", DriverDataType.String)]
+    [InlineData("   ", DriverDataType.String)]
+    [InlineData("", DriverDataType.Reference)]
+    public void Empty_observation_value_on_a_string_item_is_a_Good_empty_string(string value, DriverDataType type)
    {
-        var idx = new MTConnectObservationIndex([Tag("x", DriverDataType.String)]);
+        var idx = new MTConnectObservationIndex([Tag("x", type)]);
        idx.Apply(Synthetic("x", value));

        var snap = idx.Get("x");

+        snap.StatusCode.ShouldBe(Good);
+        snap.Value.ShouldBe(string.Empty);
+    }
+
+    /// <summary>An unauthored item coerces as String, so its empty value is a Good empty string too (#824).</summary>
+    [Fact]
+    public void Empty_observation_value_on_an_unauthored_item_is_a_Good_empty_string()
+    {
+        var idx = new MTConnectObservationIndex();
+        idx.Apply(Synthetic("x", ""));
+
+        var snap = idx.Get("x");
+
+        snap.StatusCode.ShouldBe(Good);
+        snap.Value.ShouldBe(string.Empty);
+    }
+
+    /// <summary>
+    ///     #824: on a non-string item "" cannot be a value, so it stays BadNoCommunication (the Agent supplied
+    ///     no value) — not BadTypeMismatch, which would blame the tag's authored type.
+    /// </summary>
+    [Theory]
+    [InlineData(DriverDataType.Float64)]
+    [InlineData(DriverDataType.Float32)]
+    [InlineData(DriverDataType.Int32)]
+    [InlineData(DriverDataType.Int64)]
+    [InlineData(DriverDataType.Boolean)]
+    [InlineData(DriverDataType.DateTime)]
+    public void Empty_observation_value_on_a_non_string_item_stays_BadNoCommunication(DriverDataType type)
+    {
+        var idx = new MTConnectObservationIndex([Tag("x", type)]);
+        idx.Apply(Synthetic("x", "  "));
+
+        var snap = idx.Get("x");
+
        snap.StatusCode.ShouldBe(BadNoCommunication);
        snap.Value.ShouldBeNull();
    }

+    /// <summary>
+    ///     #824 control: the UNAVAILABLE sentinel is unaffected — still BadNoCommunication on a String item, where an
+    ///     empty value is now Good.
+    /// </summary>
+    [Fact]
+    public void Unavailable_on_a_string_item_is_still_BadNoCommunication()
+    {
+        var idx = new MTConnectObservationIndex([Tag("x", DriverDataType.String)]);
+        idx.Apply(Synthetic("x", "UNAVAILABLE"));
+
+        var snap = idx.Get("x");
+
+        snap.StatusCode.ShouldBe(BadNoCommunication);
+        snap.Value.ShouldBeNull();
+    }
+
+    /// <summary>#824 end to end through the parser: an empty, a self-closing and a blank element are all Good "".</summary>
+    [Theory]
+    [InlineData("""<Program dataItemId="p" timestamp="2026-07-24T12:00:00Z" sequence="1"></Program>""")]
+    [InlineData("""<Program dataItemId="p" timestamp="2026-07-24T12:00:00Z" sequence="1"/>""")]
+    [InlineData("""<Program dataItemId="p" timestamp="2026-07-24T12:00:00Z" sequence="1">  </Program>""")]
+    public void A_parsed_empty_event_on_a_string_item_is_a_Good_empty_string(string element)
+    {
+        var idx = new MTConnectObservationIndex([Tag("p", DriverDataType.String)]);
+        idx.Apply(MTConnectStreamsParser.Parse($"""
+            <MTConnectStreams>
+              <Header instanceId="1" nextSequence="2" firstSequence="1"/>
+              <Streams><DeviceStream><ComponentStream><Events>{element}</Events></ComponentStream></DeviceStream></Streams>
+            </MTConnectStreams>
+            """));
+
+        var snap = idx.Get("p");
+
+        snap.StatusCode.ShouldBe(Good);
+        snap.Value.ShouldBe(string.Empty);
+    }
+
+    /// <summary>A structured observation with no text stays BadNotSupported — it never becomes a Good "" (#824).</summary>
+    [Fact]
+    public void A_structured_observation_with_no_text_stays_BadNotSupported()
+    {
+        var idx = new MTConnectObservationIndex([Tag("x", DriverDataType.String)]);
+        idx.Apply(Synthetic("x", "", isStructured: true));
+
+        idx.Get("x").StatusCode.ShouldBe(BadNotSupported);
+    }
+
    // -------------------------------------------------------------------------- unknown lookups

    /// <summary>A dataItemId that is neither authored nor ever observed is unknown to this driver.</summary>
@@ -208,6 +208,34 @@ public sealed class OpcUaClientDriverPageFormSerializationTests
        reparsed.CertificateValidation.EffectiveValidateServerHostName.ShouldBe(value ?? false);
    }

+    /// <summary>
+    ///     #837: the host-name check stays opt-in (unset = off) and sits in the Security panel with help text naming
+    ///     the NAT/IP, IPv6-zone and loose-first-label caveats. Source scan (no bUnit in this repo).
+    /// </summary>
+    [Fact]
+    public void The_host_name_check_is_an_opt_in_in_the_security_panel_with_its_caveats()
+    {
+        var dir = new DirectoryInfo(AppContext.BaseDirectory);
+        while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx")))
+            dir = dir.Parent;
+        dir.ShouldNotBeNull("could not locate the repository root");
+        var razor = File.ReadAllText(Path.Combine(dir!.FullName, "src", "Server", "ZB.MOM.WW.OtOpcUa.AdminUI",
+            "Components", "Shared", "Drivers", "Forms", "OpcUaClientDriverForm.razor"));
+
+        razor.ShouldContain("<option value=\"\">Default (off)</option>");
+
+        var security = razor.IndexOf("<div class=\"panel-head\">Security</div>", StringComparison.Ordinal);
+        var next = razor.IndexOf("<div class=\"panel-head\">", security + 1, StringComparison.Ordinal);
+        security.ShouldBeGreaterThan(-1);
+        var panel = razor[security..next];
+        panel.ShouldContain("_form.OpcUa.ValidateServerHostName");
+        panel.ShouldContain("Opt-in");
+        panel.ShouldContain("NAT");
+        panel.ShouldContain("IPv6 zone ids");
+        panel.ShouldContain("Loose match");
+        razor.Split("_form.OpcUa.ValidateServerHostName").Length.ShouldBe(2, "the toggle must be rendered once");
+    }
+
    /// <summary>
    ///     #648: stored DriverConfig JSON from before <c>targetNamespaceKind</c>/<c>unsMappingTable</c> were removed
    ///     must still load into the form (the form's options use <c>UnmappedMemberHandling.Skip</c>) and the legacy