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

fix(mqtt): runtime mappers no longer refuse leading-whitespace TagConfig — JsonDocument.Parse + ValueKind is the single framing authority (#559)

Browse Source 
FromTagConfig and FromSparkplugTagConfig each carried a framing pre-check
(`tagConfig[0] != '{'`) evaluated on the UNTRIMMED string, so a TagConfig blob
with leading whitespace was refused by MQTT alone and the tag read
BadNodeIdUnknown forever — while the other six structural-parse drivers
(Modbus, S7, AbCip, AbLegacy, TwinCAT, FOCAS) hand the identical blob to
JsonDocument.Parse, which skips leading whitespace per the JSON spec.

The clause is dropped (the blank guard stays). JsonDocument.Parse plus the
root-ValueKind check now decide framing, exactly as for the six siblings: an
array/number/string root fails ValueKind, non-JSON is a JsonException. The gate
was a validator, not a discriminator — no caller (MqttSubscriptionManager,
SparkplugIngestor, MqttTagConfigModel, Inspect) used the first byte to pick a
format — so verdicts are identical for every input except whitespace-padded
JSON objects, which now get their content's verdict.

Inspect needs no logic change (it has trimmed since #556 and hands the RAW
reference to the mappers), but its comments claimed the padded-blob refusal was
production-reachable; rewritten. That shape was also MQTT's only vehicle for the
catch-all backstop, which is now unreachable-today like FOCAS's — so its test is
replaced by the same count-relation pin, and the backstop code stays as
defence-in-depth.

Tests: both mappers pinned by padded-vs-unpadded DEFINITION equality (a record,
so field-wise), padded non-object/malformed negative controls, an Inspect
verdict+inspection parity theory over the accepted AND rejected corpus with an
anti-vacuity control, and a padded MQTT corpus entry in the AdminUI
authoring/runtime parity suite.
This commit is contained in:
Joseph Doherty
2026-08-10 03:36:46 -04:00
parent 15e65c86d0
commit 621a9e5dc2
5 changed files with 227 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs
+33 -16
View File
@@ -49,7 +49,10 @@ public static class MqttTagDefinitionFactory
/// <summary>
/// Maps an authored <c>TagConfig</c> object to a typed definition keyed by <paramref name="rawPath"/>.
/// The input is always an authored TagConfig JSON object (there is no name-vs-blob heuristic in v3).
/// The input is always an authored TagConfig JSON object (there is no name-vs-blob heuristic in v3),
/// and framing is decided solely by <c>JsonDocument.Parse</c> plus the
/// root-<see cref="JsonValueKind.Object"/> check — so leading whitespace is skipped exactly as the JSON
/// spec says, and a padded blob reaches the same verdict its unpadded form does (Gitea #559).
/// Enum fields (<c>payloadFormat</c> / <c>dataType</c>) and <c>qos</c> are read STRICTLY — a
/// present-but-invalid (typo'd) value rejects the whole tag (returns <see langword="false"/> ⇒ the
/// driver surfaces <c>BadNodeIdUnknown</c>) rather than silently defaulting to a wrong-typed Good or
@@ -76,7 +79,14 @@ public static class MqttTagDefinitionFactory
public static bool FromTagConfig(string tagConfig, string rawPath, out MqttTagDefinition def)
{
def = null!;
if (string.IsNullOrWhiteSpace(tagConfig) || tagConfig[0] != '{') return false;
// Gitea #559: the blank guard only — NOT a `tagConfig[0] != '{'` framing check. That check ran on
// the UNTRIMMED string, so a blob carrying leading whitespace (which a CSV or API authoring path can
// introduce invisibly, and which no author ever means as semantic) was refused here while the six
// sibling factories' mappers handed the identical blob to JsonDocument.Parse, which skips leading
// whitespace per the JSON spec. JsonDocument.Parse + the ValueKind check below are the single framing
// authority now, exactly as they are for every other driver: an array/number/string root fails the
// ValueKind check, and anything unparseable is caught as a JsonException.
if (string.IsNullOrWhiteSpace(tagConfig)) return false;
try
{
using var doc = JsonDocument.Parse(tagConfig);
@@ -129,7 +139,9 @@ public static class MqttTagDefinitionFactory
/// <c>(groupId, edgeNodeId, deviceId?, metricName)</c> tuple; <c>deviceId</c> is optional (absent
/// means the metric is published by the edge node itself), and there is <b>no</b> <c>topic</c> —
/// a Sparkplug driver subscribes one group-wide filter and routes by the decoded topic + birth
/// certificate, never by a per-tag topic. Never throws.
/// certificate, never by a per-tag topic. Never throws. Framing is
/// <c>JsonDocument.Parse</c> plus the root-object check, so leading
/// whitespace does not change the verdict (Gitea #559) — see <see cref="FromTagConfig"/>.
/// </summary>
/// <remarks>
/// <para>
@@ -175,7 +187,10 @@ public static class MqttTagDefinitionFactory
public static bool FromSparkplugTagConfig(string tagConfig, string rawPath, out MqttTagDefinition def)
{
def = null!;
if (string.IsNullOrWhiteSpace(tagConfig) || tagConfig[0] != '{') return false;
// Gitea #559: the blank guard only — see the identical note on FromTagConfig. JsonDocument.Parse +
// the ValueKind check are the single framing authority; a leading-whitespace blob reaches the same
// verdict its unpadded form does, which is what every sibling factory has always done.
if (string.IsNullOrWhiteSpace(tagConfig)) return false;
try
{
using var doc = JsonDocument.Parse(tagConfig);
@@ -261,9 +276,12 @@ public static class MqttTagDefinitionFactory
/// The two classes are kept in <b>separate lists</b> (Gitea #556 item 2), and the catch-all
/// backstop fires on "no REJECTION was produced", never on "no warning at all" — otherwise an
/// advisory would silently swallow the only message that reports a refusal nothing can name.
/// For MQTT that shape is reachable in production: a leading-whitespace blob is refused by both
/// mappers' own framing gates while every rejection branch finds the parsed document perfectly
/// well-formed.
/// ⚠️ Since Gitea #559 removed the mappers' own framing gate, that shape has <b>no</b> known
/// vehicle: the leading-whitespace blob that used to reach it is now accepted or refused on its
/// content alone, and every remaining <c>return false</c> in either mapper has a matching
/// <see cref="DescribeRejection"/> branch. The backstop stays as defence-in-depth for the next
/// refusal cause someone adds without one — the same posture as the FOCAS inspector's, and pinned
/// the same way (a count relation, not a constructed vehicle).
/// </para>
/// </summary>
/// <remarks>
@@ -280,18 +298,17 @@ public static class MqttTagDefinitionFactory
// Gitea #556 item 2: the two classes live in SEPARATE lists, and the backstop below gates on the
// REJECTION list alone. Sharing one list meant an advisory (which fires on an accepted blob too)
// could satisfy a `warnings.Count == 0` guard and silently swallow the catch-all rejection message
// — the only thing that reports a refusal neither DescribeRejection branch can name, which for
// MQTT is a REACHABLE shape, not a theoretical one: see the leading-whitespace note below.
// — the only thing that reports a refusal neither DescribeRejection branch can name. That shape has
// no vehicle today (see the summary), so this is the structural guarantee, not a live fix.
var rejections = new List<string>();
var advisories = new List<string>();
// Gitea #556 item 1: the framing check TRIMS FIRST, so a blob whose only sin is leading whitespace
// is no longer waved through in silence. MQTT is the one factory of the seven whose RUNTIME
// mappers carry the same `[0] != '{'` gate, so — unlike its siblings, whose mappers hand the blob
// to a whitespace-tolerant JsonDocument.Parse — " {…}" is genuinely REFUSED by this driver. Which
// is exactly why the untrimmed check was a defect: the tag read BadNodeIdUnknown forever and the
// deploy gate said nothing. A reference that still does not start with '{' after trimming is not
// an equipment-tag TagConfig at all and keeps drawing nothing.
// The framing pre-filter is purely an is-this-even-a-TagConfig filter, exactly as it is for the six
// sibling factories: a reference that is not object-framed is a bare name or some other artefact,
// not a TagConfig this inspector has anything to say about, so it draws nothing. It TRIMS FIRST
// (Gitea #556 item 1) and the mappers no longer carry a framing gate of their own at all (Gitea
// #559 — JsonDocument.Parse skips leading whitespace, so " {…}" is served), which means this filter
// and the mappers agree on padded blobs instead of the filter being the laxer of the two.
if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{')
return rejections;