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

fix(adminui): a Sparkplug metric whose name contains / cannot be browse-committed #509

New Issue
Closed
opened 2026-07-27 13:50:29 -04:00 by dohertj2 · 1 comment
dohertj2 commented 2026-07-27 13:50:29 -04:00
Owner
Copy Link
Copy Source

Browse-commit derives the raw tag Name from the Sparkplug metric name. A tag Name is a RawPath segment
and may not contain /; a Sparkplug metricName legitimately may. So committing a browsed metric such as
Node Control/Rebirth is refused whole, with Row N: Name must not contain '/'.

This blocks a spec-mandatory metric. Node Control/Rebirth is required by the Sparkplug B spec on every
edge node, so the one metric an operator is most likely to browse for is the one that cannot be committed.

Current behaviour is safe, not silent — the commit is refused in words and nothing is mis-bound. The
workaround works: author the tag by manual entry with a /-free Name and type the slashed metricName
into the Sparkplug tag editor, which accepts it. So this is a usability defect, not a correctness one.

Fix. The commit mapper should derive a /-free Name (e.g. replace / with _, or take the last segment)
while carrying the original slashed string through as metricName — the two fields already exist
separately and only the derivation conflates them. Sanitising must not collide two distinct metrics onto one
Name; if it would, refuse that pair specifically rather than the whole batch.

Relevant: src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs.

Acceptance. Browsing the simulator and committing Node Control/Rebirth produces a tag whose Name is
/-free and whose metricName is the exact spec string, and reading it live returns the metric's value.

Browse-commit derives the raw **tag Name** from the Sparkplug metric name. A tag Name is a **RawPath segment** and may not contain `/`; a Sparkplug `metricName` legitimately may. So committing a browsed metric such as `Node Control/Rebirth` is refused **whole**, with `Row N: Name must not contain '/'`. **This blocks a spec-mandatory metric.** `Node Control/Rebirth` is required by the Sparkplug B spec on every edge node, so the one metric an operator is most likely to browse for is the one that cannot be committed. **Current behaviour is safe, not silent** — the commit is refused in words and nothing is mis-bound. The **workaround works**: author the tag by manual entry with a `/`-free Name and type the slashed `metricName` into the Sparkplug tag editor, which accepts it. So this is a usability defect, not a correctness one. **Fix.** The commit mapper should derive a `/`-free Name (e.g. replace `/` with `_`, or take the last segment) while carrying the **original** slashed string through as `metricName` — the two fields already exist separately and only the derivation conflates them. Sanitising must not collide two distinct metrics onto one Name; if it would, refuse that pair specifically rather than the whole batch. Relevant: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs`. **Acceptance.** Browsing the simulator and committing `Node Control/Rebirth` produces a tag whose Name is `/`-free and whose `metricName` is the exact spec string, and reading it live returns the metric's value.
dohertj2 added the driver/mqtt label 2026-07-27 13:50:29 -04:00
dohertj2 commented 2026-07-31 06:42:57 -04:00
Author
Owner
Copy Link
Copy Source

Fixed in master 2a7d1167. Node Control/Rebirth now commits.

The mapper was already correct. It carried metricName verbatim; only the derived tag Name —
a RawPath segment — tripped RawPaths.ValidateSegment. New RawBrowseCommitMapper.ToRawSegment
coerces the Name (Node Control/Rebirth → Node Control_Rebirth) and the address is untouched.

Two deliberate choices beyond the issue:

  • It applies to every driver, not just MQTT. The argument is about what a Name is, not about who
    produced it: any browser whose leaf name carries a / hit the same wall, and for all of them the
    binding address travels separately and survives.
  • The sanitiser is pinned against RawPaths.ValidateSegment, not a charset copied into the test —
    so a new rule there fails in RawBrowseCommitMapperTests rather than letting an illegal Name through.

The collision guard the issue asked for is DescribeNameCollisions, wired into the commit path: A/B
and A_B both land on A_B, and silently merging two metrics into one tag bound to whichever address
survived would be worse than the refusal this replaced. It is scoped per target group (the same
metric name under two devices is not a collision) and names the specific pair.

Fixed in master `2a7d1167`. `Node Control/Rebirth` now commits. **The mapper was already correct.** It carried `metricName` verbatim; only the derived tag **Name** — a RawPath segment — tripped `RawPaths.ValidateSegment`. New `RawBrowseCommitMapper.ToRawSegment` coerces the Name (`Node Control/Rebirth` → `Node Control_Rebirth`) and the address is untouched. Two deliberate choices beyond the issue: - **It applies to every driver, not just MQTT.** The argument is about what a Name *is*, not about who produced it: any browser whose leaf name carries a `/` hit the same wall, and for all of them the binding address travels separately and survives. - **The sanitiser is pinned against `RawPaths.ValidateSegment`,** not a charset copied into the test — so a new rule there fails in `RawBrowseCommitMapperTests` rather than letting an illegal Name through. The collision guard the issue asked for is `DescribeNameCollisions`, wired into the commit path: `A/B` and `A_B` both land on `A_B`, and silently merging two metrics into one tag bound to whichever address survived would be worse than the refusal this replaced. It is scoped **per target group** (the same metric name under two devices is not a collision) and names the specific pair.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto-managed
queue management
 cross-driver
queue management
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/mqtt
MQTT / Sparkplug B broker-subscribe driver
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
No Label driver/mqtt
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#509
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?