diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
index 8c2883d0..3121f84f 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
@@ -2943,22 +2943,36 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
base.Browse(context, ref continuationPoint, references);
var returned = references.Count - before;
+ // Forward two-pointer compaction over the appended range, NOT a per-denial RemoveAt. Removing
+ // from the middle shifts every later element down one slot, so denying a large prefix of a wide
+ // browse costs O(kept × removed) element moves; copying each survivor down to a write cursor and
+ // trimming the tail afterwards is one pass. The tail trim removes from the END, where RemoveAt
+ // moves nothing.
var filtered = 0;
- for (var ii = references.Count - 1; ii >= before; ii--)
+ var write = before;
+ for (var ii = before; ii < references.Count; ii++)
{
// ExpandedNodeId → NodeId only makes sense for a reference into our own server; a reference
// to another server's node is left alone, as this manager has no grants that could speak to it.
var target = references[ii].NodeId;
- if (target is null || target.IsAbsolute) continue;
+ var keep = target is null
+ || target.IsAbsolute
+ || ExpandedNodeId.ToNodeId(target, Server.NamespaceUris) is not { } targetId
+ || !OwnsNamespace(targetId)
+ || Permits(identity, OpcUaOperation.Browse, targetId);
- var targetId = ExpandedNodeId.ToNodeId(target, Server.NamespaceUris);
- if (targetId is null || !OwnsNamespace(targetId)) continue;
- if (Permits(identity, OpcUaOperation.Browse, targetId)) continue;
+ if (!keep)
+ {
+ filtered++;
+ continue;
+ }
- references.RemoveAt(ii);
- filtered++;
+ if (write != ii) references[write] = references[ii];
+ write++;
}
+ for (var ii = references.Count - 1; ii >= write; ii--) references.RemoveAt(ii);
+
LogAclFiltering("Browse", identity, filtered, returned);
}
@@ -2995,14 +3009,24 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
var before = targetIds.Count;
base.TranslateBrowsePath(context, sourceHandle, relativePath, targetIds, unresolvedTargetIds);
- for (var ii = targetIds.Count - 1; ii >= before; ii--)
+ // Forward two-pointer compaction over the appended range — same reasoning as Browse above: a
+ // mid-list RemoveAt per denial is O(kept × removed) element moves, one compacting pass plus a
+ // tail trim is O(n).
+ var write = before;
+ for (var ii = before; ii < targetIds.Count; ii++)
{
var targetId = ExpandedNodeId.ToNodeId(targetIds[ii], Server.NamespaceUris);
- if (targetId is null || !OwnsNamespace(targetId)) continue;
- if (Permits(identity, OpcUaOperation.Browse, targetId)) continue;
+ var keep = targetId is null
+ || !OwnsNamespace(targetId)
+ || Permits(identity, OpcUaOperation.Browse, targetId);
- targetIds.RemoveAt(ii);
+ if (!keep) continue;
+
+ if (write != ii) targetIds[write] = targetIds[ii];
+ write++;
}
+
+ for (var ii = targetIds.Count - 1; ii >= write; ii--) targetIds.RemoveAt(ii);
}
///
diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs
index 595c677f..f60d7dd2 100644
--- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs
@@ -268,6 +268,85 @@ public sealed class NodeManagerStage6EnforcementTests : IDisposable
finally { await host.DisposeAsync(); }
}
+ ///
+ /// Browse filtering removes the denied references from the page it just appended in place,
+ /// so the survivors must come back exactly once each, in the order the base browser produced them,
+ /// and an earlier BrowseNext page already sitting in the list must be left untouched.
+ ///
+ ///
+ ///
+ /// The grant pattern is deliberately non-contiguous and denies the first child: the
+ /// filtering walk compacts survivors down over the gaps, so a denied prefix is what actually
+ /// exercises the copy-down path. A pattern granting a contiguous head would pass against a
+ /// compaction that never moves anything.
+ ///
+ ///
+ /// The expectation is taken from a baseline browse under a blanket grant rather than
+ /// hand-written, so the assertion is about what filtering did to the base browser's
+ /// page — not about the SDK's own reference ordering, which is not this manager's contract.
+ ///
+ ///
+ /// The pre-seeded sentinel reference is the range-boundary check: it belongs to a previous
+ /// page, sits below the filtering range, and names a node the session is not granted —
+ /// so an off-by-one that walks from index 0 rather than from the appended range removes it.
+ ///
+ ///
+ [Fact]
+ public async Task Browse_filtering_preserves_survivor_order()
+ {
+ var (host, server) = await BootAsync();
+ try
+ {
+ var nm = server.NodeManager!;
+ const string device = "Plant/Modbus/dev1";
+ nm.EnsureFolder(device, parentNodeId: null, displayName: "dev1", AddressSpaceRealm.Raw);
+
+ var children = new List();
+ for (var ii = 1; ii <= 6; ii++)
+ {
+ var rawPath = $"{device}/T{ii}";
+ nm.EnsureVariable(rawPath, parentFolderNodeId: device, displayName: rawPath, dataType: "Float",
+ writable: false, historianTagname: null, realm: AddressSpaceRealm.Raw);
+ children.Add(nm.TryGetVariable(rawPath, AddressSpaceRealm.Raw)!.NodeId);
+ }
+
+ var deviceId = new NodeId(device, children[0].NamespaceIndex);
+
+ // Baseline: everything the base browser returns for this folder, unfiltered.
+ nm.Authorizer = Authorizer(enabled: true, GrantOn(NodePermissions.Browse));
+ var baseline = Browse(nm, Identity(), deviceId);
+ baseline.Count.ShouldBeGreaterThanOrEqualTo(6);
+
+ // Now grant Browse on the 1st, 3rd and 5th child only.
+ var granted = new[] { children[0], children[2], children[4] };
+ nm.Authorizer = TagBrowseAuthorizer($"{device}/T1", $"{device}/T3", $"{device}/T5");
+
+ var sentinel = new ReferenceDescription
+ {
+ NodeId = children[1], // an explicitly DENIED node, from an earlier page
+ DisplayName = "previous-page",
+ };
+ var references = Browse(nm, Identity(), deviceId, sentinel);
+
+ var denied = new[] { children[1], children[3], children[5] };
+ var expected = new[] { (ExpandedNodeId)children[1] }
+ .Concat(baseline
+ .Select(r => r.NodeId)
+ .Where(id => !denied.Any(d => (ExpandedNodeId)d == id)))
+ .ToList();
+
+ // Non-vacuity: three references really were dropped (and the sentinel really was added).
+ expected.Count.ShouldBe(baseline.Count - 2);
+
+ references.Select(r => r.NodeId).ShouldBe(expected);
+
+ // Belt and braces: each survivor appears exactly once, and every granted child survived.
+ foreach (var id in granted)
+ references.Count(r => r.NodeId == (ExpandedNodeId)id).ShouldBe(1, $"{id} survives once");
+ }
+ finally { await host.DisposeAsync(); }
+ }
+
// ───────────────────────────── helpers ─────────────────────────────
private static NodeId Variable(OtOpcUaNodeManager nm, string rawPath)
@@ -297,6 +376,64 @@ public sealed class NodeManagerStage6EnforcementTests : IDisposable
return (values, errors);
}
+ ///
+ /// Drives the manager's Browse override directly, optionally with references already in the
+ /// list to stand in for an earlier BrowseNext page.
+ ///
+ private static List Browse(
+ OtOpcUaNodeManager nm,
+ IUserIdentity? identity,
+ NodeId source,
+ params ReferenceDescription[] previousPage)
+ {
+ var context = new OperationContext(
+ new RequestHeader(), secureChannelContext: null, RequestType.Browse, identity);
+
+ var continuationPoint = new ContinuationPoint
+ {
+ NodeToBrowse = nm.GetManagerHandle(source),
+ BrowseDirection = BrowseDirection.Forward,
+ ReferenceTypeId = null,
+ IncludeSubtypes = true,
+ NodeClassMask = 0,
+ ResultMask = BrowseResultMask.All,
+ MaxResultsToReturn = 0,
+ };
+
+ var references = new List(previousPage);
+ nm.Browse(context, ref continuationPoint, references);
+ continuationPoint?.Dispose();
+ return references;
+ }
+
+ /// A Browse-only grant per named raw tag path, each in its own Tag-kind scope.
+ private static DataPlaneAuthorizer TagBrowseAuthorizer(params string[] rawPaths)
+ {
+ var grants = rawPaths
+ .Select((_, ii) => new NodeAcl
+ {
+ NodeAclId = $"acl-tag-{ii}",
+ ClusterId = Cluster,
+ Principal = Principal,
+ ScopeKind = NodeAclScopeKind.Tag,
+ ScopeId = $"tag-{ii}",
+ PermissionFlags = NodePermissions.Browse,
+ })
+ .ToList();
+
+ var scopePaths = rawPaths
+ .Select((path, ii) => (Ref: new AclScopeRef(NodeAclScopeKind.Tag, $"tag-{ii}"), Path: path))
+ .ToDictionary(
+ e => e.Ref,
+ e => (IReadOnlyList)
+ [new NodeAclPath([NodeAclRealms.Raw, .. e.Path.Split('/')])]);
+
+ var state = new DataPlaneAuthorityState();
+ state.Apply(Cluster, "rev-1", [], grants, scopePaths);
+ return new DataPlaneAuthorizer(
+ state, new TriePermissionEvaluator(state.Tries), enabled: true, NullLogger.Instance);
+ }
+
private static DataPlaneAuthorizer Authorizer(
bool enabled,
NodeAcl grant,