Skip to content

fix(xml): dispatch v2.6/v2.7 namespaces + default unknown to Max - #229

Draft
ottobolyos wants to merge 6 commits into
TrakHound:masterfrom
ottobolyos:fix/xml-version-dispatch-chain
Draft

fix(xml): dispatch v2.6/v2.7 namespaces + default unknown to Max#229
ottobolyos wants to merge 6 commits into
TrakHound:masterfrom
ottobolyos:fix/xml-version-dispatch-chain

Conversation

@ottobolyos

@ottobolyos ottobolyos commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

MTConnectVersion.GetByNamespace failed to recognize the v2.6 and v2.7 XML namespaces, resolving any such document to an empty Version (0.0) instead of the correct release. This patch adds the missing dispatch branches and changes the fallback for an unrecognized namespace to the latest supported release instead of an empty version.

Root cause

libraries/MTConnect.NET-XML/MTConnectVersion.cs:22-44 — the GetByNamespace(string ns) dispatch chain checked namespaces from Namespaces.Version25 downwards and then fell through to return new Version();. The Namespaces.Version26 / Namespaces.Version27 classes and the MTConnectVersions.Version26 / MTConnectVersions.Version27 constants already existed elsewhere in the codebase, but no branch matched them here — a pure dispatch omission, not a missing feature.

Effect: a document declaring urn:mtconnect.org:MTConnectStreams:2.7 (or :2.6) resolved to version 0.0, which downstream consumers would either reject as invalid or accept with the wrong version metadata attached.

Fix

  • Added Namespaces.Version27.Match(ns) and Namespaces.Version26.Match(ns) branches at the top of the chain, ahead of Namespaces.Version25, so namespaces resolve highest-first (mirroring the existing ordering).
  • Changed the fallback from return new Version(); to return MTConnectVersions.Max;, so an unrecognized namespace now defaults to the latest supported release rather than an empty version.

v2.8 namespace support is out of scope here — no Namespaces.Version28 exists yet, so no v2.8 branch was added.

Dime review cycle 1

Retroactive backfill (2026-08-20). The 6-agent Ultrareview cycle ran on this PR (verified on bluefin worktree .claude/pr229-verify at head 251f76b4; build rc=0, 0 warnings, 0 errors; 9-of-10 test projects passed — SHDR 63, Common 3987, XML 198, MqttRelay 35, JSON-cppagent 112, HTTP 363, Compliance 225, JSON 62, Integration 16 = 5,061 tests; Docs-Tests hit documented bluefin Playwright flake). Ledger reconstruction from commit history:

  • [TEST] test-coverage-audit — every-kind + boundary + Get() surface pin for the dispatch chain (F-TEST-002..004) landed in test(xml): pin every kind + boundary + Get() surface for dispatch chain (b3dbaa4, 620e8010 reference).
  • [FINDING] code-review — sibling bug (F-TEST-001) Assets arm missing from Namespaces.Version12.Match surfaced during the every-kind sweep; fixed atomically in fix(xml): include Assets arm in Namespaces.Version12.Match (5a150a3, 6c21771e reference).
  • [FINDING:A05] security-audit HIGH (F-SEC-001) — Namespaces.Get(string) needed hardening against XXE and entity-expansion attacks; fixed atomically in fix(xml): harden Namespaces.Get(string) against XXE and entity expansion (13344d0, ebf0011e reference).
  • [DOCS] documentation-audit (F-SIMP-003, F-CR-002, F-CR-004, F-CR-006) — Get/GetByNamespace summaries and InternalsVisibleTo annotation clarified in docs(xml): clarify Get/GetByNamespace docs; annotate InternalsVisibleTo (960fe8b, 251f76b4 reference).
  • [FINDING] code-review F-CR-001 — fallback change to MTConnectVersions.Max: CLOSED with rationale — explicit design intent already stated in the original fix(xml) commit message and PR title.
  • [FINDING:A08] security-audit F-SEC-002 — same fallback change: CLOSED with same rationale as F-CR-001.
  • [IMPROVE] improvement F-IMP-001 — Trace.TraceWarning on unknown-namespace fallback: CLOSED with rationale — silent forward-compat is by design; the follow-up F-IMP-002 TryGetByNamespace surface is the cleaner escape hatch for strict-conformance callers.
  • [TEST] test-coverage-audit F-TEST-005 — TDD ordering on the original fix commit: TRACKED — deferred to human reviewer (splitting a pushed head SHA would rewrite an already-flipped-to-draft PR; the 620e8010/6c21771e ordering demonstrates the correct pattern for the sibling bug).
  • [DOCS] documentation-audit F-DOC-002 — docs/wire-formats/xml.md:117 stale "returns empty Version" for v1.9: TRACKED against docs(wire-formats): correct XML codec fall-through description #232 (needs re-verification to cover lines 3, 82, and 117).
  • [DOCS] documentation-audit F-DOC-003 — docs/wire-formats/xml.md:3, :82 stale "up to v2.5" claim + Max annotation on v2.5 row: TRACKED against docs(wire-formats): correct XML codec fall-through description #232.
  • [IMPROVE] improvement F-IMP-002 (TryGetByNamespace) / F-IMP-003 (data-driven dispatch) / [FINDING] code-review F-CR-005 (dictionary refactor): FOLLOW-UP issues suggested; out of PR scope.

(Zero unfixed findings — Ready-eligible.)

Depends on

ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
Version12 declares urn:mtconnect.org:MTConnectAssets:1.2 as a namespace
constant but its Match(ns) disjunction omitted the Assets arm, so
a v1.2 Assets document fell through the GetByNamespace dispatch chain
and resolved to MTConnectVersions.Max (post-TrakHound#229 fallback; previously
new Version()) instead of Version12.

Every version from v1.3 onwards already lists Assets as the first arm
of its Match; v1.0 and v1.1 do not declare Assets at all, so their
three-arm Match is correct. Version12 is the single asymmetric case,
found by the expanded coverage-FLOOR sweep in the preceding test
commit. Bug-class atomicity per §1.0d-trigies-bis: the sibling bug
sitting next to the primary v2.6/v2.7 dispatch omission ships in the
discovering PR, not a follow-up.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
Integration-branch build: merge origin/integration/up-to-pr-228 with
PR TrakHound#229 head 251f76b.

Contents landed under PR TrakHound#229:
  * e96ae6e fix(xml): route v2.6/v2.7 namespaces + default unknown to Max
  * 620e801 test(xml): pin every kind + boundary + Get() surface for dispatch chain
  * 6c21771 fix(xml): include Assets arm in Namespaces.Version12.Match
  * ebf0011 fix(xml): harden Namespaces.Get(string) against XXE and entity expansion
  * 251f76b docs(xml): distinguish Get/GetByNamespace summaries and annotate InternalsVisibleTo
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
Version12 declares urn:mtconnect.org:MTConnectAssets:1.2 as a namespace
constant but its Match(ns) disjunction omitted the Assets arm, so
a v1.2 Assets document fell through the GetByNamespace dispatch chain
and resolved to MTConnectVersions.Max (post-TrakHound#229 fallback; previously
new Version()) instead of Version12.

Every version from v1.3 onwards already lists Assets as the first arm
of its Match; v1.0 and v1.1 do not declare Assets at all, so their
three-arm Match is correct. Version12 is the single asymmetric case,
found by the expanded coverage-FLOOR sweep in the preceding test
commit. Bug-class atomicity per §1.0d-trigies-bis: the sibling bug
sitting next to the primary v2.6/v2.7 dispatch omission ships in the
discovering PR, not a follow-up.
@ottobolyos
ottobolyos force-pushed the fix/xml-version-dispatch-chain branch from 251f76b to 960fe8b Compare August 19, 2026 22:06
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Version12 declares urn:mtconnect.org:MTConnectAssets:1.2 as a namespace
constant but its Match(ns) disjunction omitted the Assets arm, so
a v1.2 Assets document fell through the GetByNamespace dispatch chain
and resolved to MTConnectVersions.Max (post-TrakHound#229 fallback; previously
new Version()) instead of Version12.

Every version from v1.3 onwards already lists Assets as the first arm
of its Match; v1.0 and v1.1 do not declare Assets at all, so their
three-arm Match is correct. Version12 is the single asymmetric case,
found by the expanded coverage-FLOOR sweep in the preceding test
commit. Bug-class atomicity per §1.0d-trigies-bis: the sibling bug
sitting next to the primary v2.6/v2.7 dispatch omission ships in the
discovering PR, not a follow-up.
@ottobolyos
ottobolyos force-pushed the fix/xml-version-dispatch-chain branch from 960fe8b to 0785fb7 Compare August 21, 2026 06:18
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Version12 declares urn:mtconnect.org:MTConnectAssets:1.2 as a namespace
constant but its Match(ns) disjunction omitted the Assets arm, so
a v1.2 Assets document fell through the GetByNamespace dispatch chain
and resolved to MTConnectVersions.Max (post-TrakHound#229 fallback; previously
new Version()) instead of Version12.

Every version from v1.3 onwards already lists Assets as the first arm
of its Match; v1.0 and v1.1 do not declare Assets at all, so their
three-arm Match is correct. Version12 is the single asymmetric case,
found by the expanded coverage-FLOOR sweep in the preceding test
commit. Bug-class atomicity per §1.0d-trigies-bis: the sibling bug
sitting next to the primary v2.6/v2.7 dispatch omission ships in the
discovering PR, not a follow-up.
@ottobolyos
ottobolyos force-pushed the fix/xml-version-dispatch-chain branch from 0785fb7 to 5056b48 Compare August 21, 2026 08:34
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Version12 declares urn:mtconnect.org:MTConnectAssets:1.2 as a namespace
constant but its Match(ns) disjunction omitted the Assets arm, so
a v1.2 Assets document fell through the GetByNamespace dispatch chain
and resolved to MTConnectVersions.Max (post-TrakHound#229 fallback; previously
new Version()) instead of Version12.

Every version from v1.3 onwards already lists Assets as the first arm
of its Match; v1.0 and v1.1 do not declare Assets at all, so their
three-arm Match is correct. Version12 is the single asymmetric case,
found by the expanded coverage-FLOOR sweep in the preceding test
commit. Bug-class atomicity: the sibling bug sitting next to the
primary v2.6/v2.7 dispatch omission ships in the discovering PR, not
a follow-up.
@ottobolyos
ottobolyos force-pushed the fix/xml-version-dispatch-chain branch from 5056b48 to ed28525 Compare August 21, 2026 14:12
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
MTConnectVersion.GetByNamespace capped its dispatch chain at Version25
and fell through to an empty Version for any newer namespace, even
though the Version26/Version27 namespace classes and MTConnectVersions
constants already existed. A document declaring MTConnectStreams:2.7
(or :2.6) therefore resolved to an empty version.
Extend MTConnectVersionDispatchTests to close coverage-FLOOR gaps:

  - Every enum-arm of every Namespaces.Version{XX}.Match disjunction
    (Assets / Devices / Error / Streams) across every declared
    version (v1.0 through v2.7), not just the Devices arm - fixes
    a 68-arm enum-arm gap in the original fixture.
  - Boundary inputs: empty string, whitespace-only (space / tab /
    newline / mixed), leading/trailing padded, and upper-case /
    mixed-case variant of a canonical URI.
  - Public-API surface: MTConnectVersion.Get(string xml) round-trip
    from a well-formed XML declaration through Namespaces.Get to
    GetByNamespace, plus the no-namespace boundary.

The Assets-arm sweep goes RED against the current SUT on
Namespaces.Version12.Assets - the Version12.Match disjunction omits
its declared Assets constant, resolving v1.2 Assets documents to Max
(the fallback) instead of Version12. Fix follows in the next commit
(TDD: red before green).
Version12 declares urn:mtconnect.org:MTConnectAssets:1.2 as a namespace
constant but its Match(ns) disjunction omitted the Assets arm, so
a v1.2 Assets document fell through the GetByNamespace dispatch chain
and resolved to MTConnectVersions.Max (post-TrakHound#229 fallback; previously
new Version()) instead of Version12.

Every version from v1.3 onwards already lists Assets as the first arm
of its Match; v1.0 and v1.1 do not declare Assets at all, so their
three-arm Match is correct. Version12 is the single asymmetric case,
found by the expanded coverage-FLOOR sweep in the preceding test
commit. Bug-class atomicity: the sibling bug sitting next to the
primary v2.6/v2.7 dispatch omission ships in the discovering PR, not
a follow-up.
Namespaces.Get(string xml) delegated straight to XmlDocument.LoadXml,
whose DtdProcessing default and legacy XmlResolver behavior varied
across the library's TFMs (net461–net10.0). The v2.6/v2.7 dispatch
routing (fix at e96ae6e) plus the new MTConnectVersions.Max fallback
widened the number of documents that flow through this parse.

Route the parse through an explicit XmlReader with DtdProcessing set
to Prohibit and XmlResolver set to null, so DOCTYPE-carrying payloads
(including billion-laughs-style entity expansion) and external-entity
(file:// / http://) references are refused before parsing rather than
resolved. Catch XmlException so downstream callers see the same
Max-fallback signal on malformed input rather than an exception.

Also short-circuit null / empty input at the entry point instead of
letting LoadXml throw ArgumentException.

Ultrareview finding F-SEC-001 (HIGH, A05_security-misconfiguration).
The XML doc summaries on MTConnectVersion.Get and .GetByNamespace were
verbatim copies of each other, obscuring the actual contract: Get takes
a raw XML document (delegating to Namespaces.Get for the extraction
step), GetByNamespace takes an already-resolved namespace URI. Rewrite
each summary to state its own signature and mention the Max-fallback
behavior on GetByNamespace.

Annotate the InternalsVisibleTo grant in MTConnect.NET-XML.csproj so a
future reader sees why the library reaches into MTConnect.NET-XML-Tests
(the tests reach the internal MTConnectVersion / Namespaces helpers,
which have no public equivalent). Also tighten the trailing newline of
MTConnectVersion.cs and update two comment breaks in the test file to
tight em-dashes.

Ultrareview findings F-SIMP-003, F-CR-002, F-CR-004, F-CR-006 (LOW/NIT).
@ottobolyos
ottobolyos force-pushed the fix/xml-version-dispatch-chain branch from ed28525 to a8a95a2 Compare August 21, 2026 16:06
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…it 4

NUnit 4's Assert.DoesNotThrow / Assert.Throws<T> resolve delegate arguments
against multiple overloads (Action, TestDelegate, AsyncTestDelegate). A bare
() => lambda triggers CS0121 overload ambiguity once TrakHound#239 lands the NUnit 4
upgrade. The (Action) cast disambiguates unambiguously in both NUnit 3 and 4,
so the wrap is safe to land on this branch before TrakHound#239 merges.

Fixes cross-PR bug class discovered on integration/up-to-pr-249 build (158
errors, 112 CS0121 across 9 test files on 7 PRs). Per-PR fix — each affected
PR wraps its own new test sites so the class stays clean across the train.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant