fix(tagconfig): a numerically-spelled enum names no member — reject it fleet-wide (Driver.Sql.Contracts-002)

TagConfigJson.ClassifyEnumText classified via Enum.TryParse(text, ignoreCase: true),
which accepts a numeric STRING for any enum: "1" returns the member at that ordinal
and an out-of-range "99" parses just as happily. Every driver contract's strict enum
read inherits that, so a Sql tag authored "type":"99" was ACCEPTED, carried
(DriverDataType)99, and SqlPollReader.TryCoerce fell through its switch to the String
arm — a stringified cell published Good under a type nothing can map.

Refuse a digit- or sign-leading spelling before the parse (judged on the TRIMMED text,
because Enum.TryParse trims), and require Enum.IsDefined — which covers what the
leading-character check cannot, since TryParse also ORs a comma-separated list for a
non-[Flags] enum. Mirrors RawBrowseCommitMapper.ParseNativeType and
MTConnectTagDefinitionFactory.TryParseDataType, which already guard their own parses.

Three AdminUI restatements of the same rule now ask ClassifyEnumText instead of
Enum.TryParse (TagConfigJson.GetEnum, SqlTagConfigModel.ReadEnumStrict,
MqttTagConfigModel.DescribeInvalidEnum/GetEnumNullable) — leaving them would have opened
the #549 direction the moment the runtime tightened: a blob the modal accepts and the
driver skips.

RED first: 10 failing in Core.Abstractions.Tests, 4 in Driver.Sql.Tests, and 10 of the
11 new fleet-wide parity cases (re-confirmed by reverting the Core fix).
This commit is contained in:
Joseph Doherty
2026-08-10 08:02:18 -04:00
parent 94b8633e68
commit 8d4ca5489a
7 changed files with 198 additions and 11 deletions
@@ -8,10 +8,13 @@ public enum JsonEnumRead
/// <summary>The property is absent, or present but not a JSON string (nothing to validate).</summary>
Absent,
/// <summary>The property is a JSON string that parses (case-insensitively) to the enum.</summary>
/// <summary>The property is a JSON string naming (case-insensitively) a declared enum member.</summary>
Valid,
/// <summary>The property is a JSON string that does NOT parse to the enum (a typo'd value).</summary>
/// <summary>
/// The property is a JSON string that names no declared enum member — a typo, or a numeric
/// spelling such as <c>"8"</c>, which is not a name at all.
/// </summary>
Invalid,
}
@@ -67,6 +70,19 @@ public static class TagConfigJson
/// treatment of an absent or present-but-non-string property — so a caller that extracts "the
/// string value, or null" gets the runtime's leniency for free.
/// </para>
/// <para>
/// <b>Only a member NAME is valid — a numeric spelling is refused before the parse.</b>
/// <see cref="Enum.TryParse{TEnum}(string, bool, out TEnum)"/> accepts <c>"1"</c> for any enum
/// and hands back whatever member sits at that ordinal, and an out-of-range <c>"99"</c> parses
/// just as readily — so a TagConfig carrying a numerically-serialised enum (the repo-wide
/// authoring trap) read <see cref="JsonEnumRead.Valid"/> at all six driver parsers and bound a
/// tag to a type nobody named, under <c>Good</c> quality. The same idiom guards
/// <c>RawBrowseCommitMapper.NativeType</c> and <c>MTConnectTagDefinitionFactory</c>'s own type
/// parse. <see cref="Enum.IsDefined{TEnum}(TEnum)"/> then covers what the leading-character
/// check cannot: <c>TryParse</c> also bitwise-ORs a comma-separated list for a
/// non-<c>[Flags]</c> enum, so <c>"Beta, Gamma"</c> would otherwise parse to a value no member
/// declares.
/// </para>
/// </summary>
/// <typeparam name="TEnum">The enum type to parse.</typeparam>
/// <param name="text">The authored value's text, or <see langword="null"/> when there is none.</param>
@@ -80,12 +96,26 @@ public static class TagConfigJson
value = default;
return JsonEnumRead.Absent;
}
if (Enum.TryParse(text, ignoreCase: true, out value))
if (IsMemberNameSpelling(text) && Enum.TryParse(text, ignoreCase: true, out value) && Enum.IsDefined(value))
return JsonEnumRead.Valid;
value = default;
return JsonEnumRead.Invalid;
}
/// <summary>
/// Whether <paramref name="text"/> is spelled as a member name rather than as a number. Judged on
/// the TRIMMED text because <see cref="Enum.TryParse{TEnum}(string, bool, out TEnum)"/> trims
/// before parsing, so <c>" 8 "</c> is the numeric spelling too; a C# member name can never begin
/// with a digit or a sign, so no valid name is lost.
/// </summary>
/// <param name="text">The authored value's text.</param>
/// <returns><see langword="true"/> when the text could be a member name.</returns>
private static bool IsMemberNameSpelling(string text)
{
var span = text.AsSpan().Trim();
return span.Length > 0 && !char.IsAsciiDigit(span[0]) && span[0] is not ('-' or '+');
}
/// <summary>
/// Lenient enum read with today's exact semantics: absent OR invalid ⇒ <paramref name="fallback"/>;
/// only a present, parseable string yields the parsed value. Bit-for-bit equivalent to the six
@@ -1,6 +1,8 @@
using System.Text.Json.Nodes;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Driver.Mqtt;
using RuntimeJsonEnumRead = ZB.MOM.WW.OtOpcUa.Core.Abstractions.JsonEnumRead;
using RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
@@ -415,13 +417,20 @@ public sealed class MqttTagConfigModel
/// <param name="name">The property name to read.</param>
/// <returns>The parsed enum value, or <c>null</c>.</returns>
private static TEnum? GetEnumNullable<TEnum>(JsonObject o, string name) where TEnum : struct, Enum
=> TagConfigJson.GetString(o, name) is { } s && Enum.TryParse<TEnum>(s, ignoreCase: true, out var v) ? v : null;
=> RuntimeTagConfigJson.ClassifyEnumText<TEnum>(TagConfigJson.GetString(o, name), out var v)
== RuntimeJsonEnumRead.Valid
? v
: null;
/// <summary>
/// Describes a present-but-invalid enum field, or <c>null</c> when it is absent or valid.
/// Mirrors <c>TagConfigJson.TryReadEnum</c>'s absent/valid/invalid split exactly — including its
/// treatment of a present-but-non-string value as ABSENT — so the editor never blocks a blob the
/// driver would happily default.
/// driver would happily default. The VERDICT is the runtime's own
/// <see cref="RuntimeTagConfigJson.ClassifyEnumText{TEnum}"/>, never a local
/// <see cref="Enum.TryParse{TEnum}(string, bool, out TEnum)"/>: a numeric spelling such as
/// <c>"1"</c> parses there and names no member (Driver.Sql.Contracts-002), so restating the rule
/// here would accept a blob the driver refuses — the Gitea #549 direction.
/// </summary>
/// <typeparam name="TEnum">The enum type expected.</typeparam>
/// <param name="o">The TagConfig key bag.</param>
@@ -433,7 +442,7 @@ public sealed class MqttTagConfigModel
{
return null;
}
return Enum.TryParse<TEnum>(raw, ignoreCase: true, out _)
return RuntimeTagConfigJson.ClassifyEnumText<TEnum>(raw, out _) == RuntimeJsonEnumRead.Valid
? null
: $"'{raw}' is not a valid {name}; valid: {string.Join(", ", Enum.GetNames<TEnum>())}.";
}
@@ -1,6 +1,7 @@
using System.Text.Json.Nodes;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
using RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
@@ -156,7 +157,9 @@ public sealed class SqlTagConfigModel
// Reads an enum-valued string field the way the parser's TryReadEnumStrict does: absent OR present-but-
// non-string ⇒ (null, null) — nothing to validate; a present string that parses ⇒ (value, null); a
// present string that does NOT parse ⇒ (null, offendingString) so Validate can reject it.
// present string that does NOT parse ⇒ (null, offendingString) so Validate can reject it. The verdict is
// the RUNTIME's own ClassifyEnumText, not a local Enum.TryParse — restating it is how the two doors drift
// (Gitea #549), and a numeric spelling like "1" is exactly where they would have (Driver.Sql.Contracts-002).
private static (TEnum? value, string? invalidRaw) ReadEnumStrict<TEnum>(JsonObject o, string name)
where TEnum : struct, Enum
{
@@ -164,7 +167,9 @@ public sealed class SqlTagConfigModel
{
return (null, null);
}
return Enum.TryParse<TEnum>(s, ignoreCase: true, out var parsed) ? (parsed, null) : (null, s);
return RuntimeTagConfigJson.ClassifyEnumText<TEnum>(s, out var parsed) == JsonEnumRead.Valid
? (parsed, null)
: (null, s);
}
}
@@ -1,5 +1,6 @@
using System.Text.Json;
using System.Text.Json.Nodes;
using JsonEnumRead = ZB.MOM.WW.OtOpcUa.Core.Abstractions.JsonEnumRead;
using RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
@@ -57,14 +58,23 @@ public static class TagConfigJson
public static bool? GetBoolNullable(JsonObject o, string name)
=> o.TryGetPropertyValue(name, out var n) && n is JsonValue v && v.TryGetValue<bool>(out var b) ? b : null;
/// <summary>Reads an enum by its serialised name, or <paramref name="fallback"/> if absent/unparseable.</summary>
/// <summary>
/// Reads an enum by its serialised NAME, or <paramref name="fallback"/> if absent/unreadable. The
/// name/number distinction comes from
/// <see cref="RuntimeTagConfigJson.ClassifyEnumText{TEnum}"/> rather than from
/// <see cref="Enum.TryParse{TEnum}(string, bool, out TEnum)"/> directly, so a numerically-spelled
/// value the runtime refuses is not DISPLAYED here as the member sitting at that ordinal — the
/// editor would show, and then save, a type nobody authored.
/// </summary>
/// <typeparam name="TEnum">The enum type to parse.</typeparam>
/// <param name="o">The JSON object to read from.</param>
/// <param name="name">The property name to read.</param>
/// <param name="fallback">The value to return if the property is absent or unparseable.</param>
/// <param name="fallback">The value to return if the property is absent or unreadable.</param>
/// <returns>The parsed enum value, or <paramref name="fallback"/>.</returns>
public static TEnum GetEnum<TEnum>(JsonObject o, string name, TEnum fallback) where TEnum : struct, Enum
=> GetString(o, name) is { } s && Enum.TryParse<TEnum>(s, ignoreCase: true, out var v) ? v : fallback;
=> RuntimeTagConfigJson.ClassifyEnumText<TEnum>(GetString(o, name), out var v) == JsonEnumRead.Valid
? v
: fallback;
/// <summary>
/// Describes a present-but-invalid enum field the way the RUNTIME parser judges it, or
@@ -35,6 +35,79 @@ public sealed class TagConfigJsonTests
v.ShouldBe(expectedValue);
}
// ---- A numeric spelling is not a member NAME (Driver.Sql.Contracts-002) ----
/// <summary>
/// <see cref="Enum.TryParse{TEnum}(string, bool, out TEnum)"/> accepts a numeric STRING for any
/// enum — <c>"1"</c> hands back whatever member sits at ordinal 1, and an out-of-range <c>"99"</c>
/// parses just as happily — so a TagConfig carrying a numerically-serialised enum (the repo-wide
/// authoring trap: a page that serialises enums as numbers into a string-typed field) used to read
/// <see cref="JsonEnumRead.Valid"/> at every one of the six driver parsers. The tag was then served
/// under a data type nobody named, publishing a confident <c>Good</c> — the exact failure R2-11's
/// strict read exists to prevent. A digit- or sign-leading value is refused BEFORE the parse.
/// </summary>
[Theory]
[InlineData("\"1\"")] // in range — parses today, and IsDefined would confirm it
[InlineData("\"99\"")] // out of range — parses today too
[InlineData("\"0\"")]
[InlineData("\"-1\"")] // sign-leading
[InlineData("\"+1\"")]
[InlineData("\" 2 \"")] // Enum.TryParse trims, so the guard must judge the TRIMMED text
public void TryReadEnum_aNumericSpelling_isInvalid_notAMemberName(string token)
{
TagConfigJson.TryReadEnum<Sample>(Root($"{{\"k\":{token}}}"), "k", out var v)
.ShouldBe(JsonEnumRead.Invalid);
v.ShouldBe(default(Sample));
}
/// <summary>
/// The <see cref="Enum.IsDefined{TEnum}(TEnum)"/> half of the guard, which the digit-leading check
/// cannot cover: <see cref="Enum.TryParse{TEnum}(string, bool, out TEnum)"/> also accepts a
/// COMMA-SEPARATED list for a non-<c>[Flags]</c> enum and bitwise-ORs the members, so
/// <c>"Beta, Gamma"</c> parses to <c>3</c> — a value no member declares. Deleting the
/// <c>IsDefined</c> check turns this case green again.
/// </summary>
[Fact]
public void TryReadEnum_aCombinationThatNoMemberDeclares_isInvalid()
{
TagConfigJson.TryReadEnum<Sample>(Root("{\"k\":\"Beta, Gamma\"}"), "k", out var v)
.ShouldBe(JsonEnumRead.Invalid);
v.ShouldBe(default(Sample));
}
/// <summary>
/// Falsifiability control for the two guards above: a guard that refused everything would satisfy
/// them all. Member NAMES — the only spelling a TagConfig may carry — keep parsing, whatever their
/// case and surrounding whitespace.
/// </summary>
[Theory]
[InlineData("Alpha", Sample.Alpha)]
[InlineData("beta", Sample.Beta)]
[InlineData("GAMMA", Sample.Gamma)]
[InlineData(" Gamma ", Sample.Gamma)]
public void ClassifyEnumText_stillAcceptsEveryMemberName(string text, Sample expected)
{
TagConfigJson.ClassifyEnumText<Sample>(text, out var v).ShouldBe(JsonEnumRead.Valid);
v.ShouldBe(expected);
}
/// <summary>
/// A numeric spelling is refused with the SAME wording every other rejection carries — naming the
/// field, the offending value and the members that are allowed — so the operator is told what to
/// write instead of being told nothing.
/// </summary>
[Fact]
public void DescribeInvalidEnum_aNumericSpelling_namesTheAllowedMembers()
{
var msg = TagConfigJson.DescribeInvalidEnum<Sample>(Root("{\"dataType\":\"8\"}"), "dataType");
msg.ShouldNotBeNull();
msg.ShouldContain("'8'");
msg.ShouldContain("dataType");
msg.ShouldContain("Alpha");
msg.ShouldContain("Beta");
msg.ShouldContain("Gamma");
}
[Theory]
[InlineData("{\"k\":\"Gamma\"}", Sample.Gamma)] // valid → parsed
[InlineData("{\"k\":\"typo\"}", Sample.Alpha)] // invalid → fallback
@@ -50,6 +123,8 @@ public sealed class TagConfigJsonTests
[InlineData("{}", true, Sample.Alpha)] // absent → fallback, ok
[InlineData("{\"k\":42}", true, Sample.Alpha)] // non-string → Absent → fallback, ok
[InlineData("{\"k\":\"typo\"}", false, default(Sample))] // invalid (typo) → REJECT (false)
[InlineData("{\"k\":\"1\"}", false, default(Sample))] // numeric spelling → REJECT (Driver.Sql.Contracts-002)
[InlineData("{\"k\":\"99\"}", false, default(Sample))] // …in range or out of it
public void TryReadEnumStrict_rejects_only_invalid(string json, bool expectedOk, Sample expectedValue)
{
var ok = TagConfigJson.TryReadEnumStrict(Root(json), "k", Sample.Alpha, out var v);
@@ -196,4 +196,31 @@ public class SqlEquipmentTagParserTests
=> SqlEquipmentTagParser.TryParse(
"""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"Nonsense"}""",
"raw", out _).ShouldBeFalse();
/// <summary>
/// Driver.Sql.Contracts-002. A numerically-spelled enum is not a member name, and the strict read
/// must refuse it exactly as it refuses a typo. It used to parse — <c>"99"</c> gave
/// <c>(DriverDataType)99</c>, a declared type nothing can map, and <c>SqlPollReader.TryCoerce</c>
/// then fell through its switch to the <c>String</c> arm and published a stringified cell under it.
/// </summary>
[Theory]
[InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"99"}""")]
[InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"8"}""")]
// "1" is SqlTagModel.WideRow's ordinal — an otherwise-complete WideRow blob, so the rejection can only
// come from the enum read and not from a missing field.
[InlineData("""{"driver":"Sql","model":"1","table":"T","columnName":"c","rowSelector":{"topByTimestamp":"ts"}}""")]
public void TryParse_aNumericallySpelledEnum_rejectsTag(string json)
=> SqlEquipmentTagParser.TryParse(json, "raw", out _).ShouldBeFalse();
/// <summary>
/// …and the deploy gate explains it in the same words every other invalid enum draws, rather than
/// falling through to the backstop warning that names no field at all.
/// </summary>
[Fact]
public void Inspect_aNumericallySpelledType_namesTheFieldAndTheValidMembers()
{
var warnings = SqlTagDefinitionFactory.Inspect(
"""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"99"}""");
warnings.ShouldContain(w => w.Contains("'99'") && w.Contains("type") && w.Contains(nameof(DriverDataType.Float64)));
}
}
@@ -220,6 +220,37 @@ public sealed class TagConfigAuthoringDeployParityTests
+ "import or API is unchecked — the exact G-7 asymmetry.");
}
/// <summary>
/// Driver.Sql.Contracts-002, fleet-wide. A numerically-SPELLED enum (<c>"1"</c>, not the JSON
/// number <c>1</c>) used to satisfy <c>Enum.TryParse</c> at every driver, so both doors accepted it
/// and the tag was served under whatever member sat at that ordinal — or, out of range, under a
/// value no member declares. The shared <c>TagConfigJson.ClassifyEnumText</c> now calls it what it
/// is, and because BOTH gates ask that one method, they moved together: the fix is only real if
/// each driver refuses it at the modal AND at the deploy gate.
/// </summary>
[Theory]
// Complete but for the enum spelling, so the refusal can only come from the enum read: "1" is
// SqlTagModel.WideRow's ordinal, and every WideRow-required field is present.
[InlineData(DriverTypeNames.Sql, """{"model":"1","table":"dbo.T","columnName":"c","rowSelector":{"topByTimestamp":"ts"}}""")]
[InlineData(DriverTypeNames.Sql, """{"model":"KeyValue","table":"dbo.T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"8"}""")]
[InlineData(DriverTypeNames.Modbus, """{"region":"1","address":1}""")]
[InlineData(DriverTypeNames.Modbus, """{"region":"HoldingRegisters","address":1,"dataType":"99"}""")]
[InlineData(DriverTypeNames.S7, """{"address":"DB1.DBW0","dataType":"1"}""")]
[InlineData(DriverTypeNames.AbCip, """{"tagPath":"Program:Main.Speed","dataType":"1"}""")]
[InlineData(DriverTypeNames.AbLegacy, """{"address":"N7:0","dataType":"1"}""")]
[InlineData(DriverTypeNames.TwinCAT, """{"symbolPath":"MAIN.bStart","dataType":"1"}""")]
[InlineData(DriverTypeNames.FOCAS, """{"address":"R100","dataType":"1"}""")]
[InlineData(DriverTypeNames.Mqtt, """{"topic":"otopcua/fixture/a","payloadFormat":"1"}""")]
[InlineData(DriverTypeNames.Mqtt, """{"topic":"otopcua/fixture/a","dataType":"8"}""")]
public void A_numerically_spelled_enum_is_refused_by_both_gates(string driverType, string tagConfig)
{
TagConfigValidator.Validate(driverType, tagConfig).ShouldNotBeNullOrEmpty(
$"{driverType}: the modal would let an operator save an enum spelling the driver refuses.");
EquipmentTagConfigInspector.Inspect(driverType, tagConfig).ShouldNotBeEmpty(
$"{driverType}: the deploy gate accepted a numeric enum spelling, so a CSV-imported or "
+ "API-authored tag binds to a type nobody named.");
}
// -------------------------------------------------------------------------------------------------
// Gitea #550 — MTConnect's second spelling of the coercion type. The assertions below drive the REAL
// runtime reader (MTConnectTagDefinitionFactory.TryFromTagConfig) rather than the deploy gate alone,