mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-20 00:31:32 +08:00
fix/encoder-stack-depth-json-flake
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 romanett
|
cce2004fdd |
CTT: keep GeneratesEvent off Object instances and Views off non-hierarchical references; classify Deadband Err-006 as CTT defect (#4667)
# Description Three findings from the 2026-10-03 CTT run with the patched 1.05.513 scripts. Two are server/model defects and are fixed here. The third is a CTT script defect, recorded as C56 in `plans/ctt-issues.md`. ## 1. Monitored Items Deadband Filter `Err-006.js`: CTT defect (C56, no server change) The script modifies a MonitoredItem on the first static numeric scalar (a plain BaseDataVariable with no EURange) to use a PercentDeadband. It accepts only `Good` or `Bad_MonitoredItemFilterUnsupported`. Since #4629 the server returns `Bad_DeadbandFilterInvalid`, and that is the code the spec defines for this case: - OPC 10000-8 §7.2: PercentDeadband "applies only to AnalogItems with an EURange Property". - OPC 10000-8 §7.3.2 Table 61: `Bad_DeadbandFilterInvalid` is returned when "a PercentDeadband is not supported, since an EURange is not configured". CreateMonitoredItems and ModifyMonitoredItems both go through `ValidateMonitoringFilterAsync` and return the same code. Monitor Basic `Err-022.js` already accepts `Bad_FilterNotAllowed` for non-analog items, so the scripts disagree with each other here. Draft Mantis note (not filed yet): > **Monitored Items Deadband Filter Err-006.js rejects Bad_DeadbandFilterInvalid for a PercentDeadband on a node without EURange** > Err-006.js picks the first node of `/Server Test/NodeIds/Static/All Profiles/Scalar/NumericSettings`, which is normally a BaseDataVariable without an EURange. It then calls ModifyMonitoredItems with a PercentDeadband (10 %) and accepts only Good or Bad_MonitoredItemFilterUnsupported. OPC 10000-8 §7.2 limits PercentDeadband to AnalogItems with an EURange. §7.3.2 Table 61 defines Bad_DeadbandFilterInvalid for "a PercentDeadband is not supported, since an EURange is not configured", so a compliant server fails this test. Monitor Basic Err-022.js already accepts Bad_FilterNotAllowed for the non-analog fallback. > Proposed fix: add StatusCode.BadDeadbandFilterInvalid (and, consistent with Err-022.js, StatusCode.BadFilterNotAllowed) to the ExpectedAndAcceptedResults of the ModifyMonitoredItems call. ## 2. GeneratesEvent on Objects (Base Info Core Structure 2 `001.js`): source generator defect The CTT reported `ns=9;i=234` (the `<ServiceName>` placeholder of AuthorizationServicesFolderType) and `ns=9;s=AuthorizationServices/Default` as Objects that are the source of GeneratesEvent references. OPC 10000-3 §7.15 says: "The SourceNode of References of this type shall be an ObjectType, a VariableType or a Method". `OpcUaGdsModel.xml` declares both references (AccessTokenRequested/Issued audit events) correctly, on `AuthorizationServiceType`. The source generator copied the type's own non-hierarchical references onto every Object or Variable built from the type: onto instance declarations (the placeholder) and into `CreateInstanceOf<Type>`, which `AuthorizationServiceState` initialises from, and that is how the `Default` service gets them. UA-ModelCompiler does the same thing, so the official `Opc.Ua.Gds.NodeSet2.xml` in UA-Nodeset has the same two references on `ns=1;i=234`. Fix: - `TranslateReferences` now marks references declared on a type (`HierarchyReference.DefinedOnType`, which already existed but was never set). - `NodeStateGenerator.GetReferences` drops forward GeneratesEvent references (including subtypes such as AlwaysGeneratesEvent) for Object and Variable nodes when the reference came from the type. - ObjectTypes, VariableTypes and Methods keep these references. For example, the RegisterApplication methods and the Acknowledge/AddComment methods of the alarm types are unchanged. - References that a model declares explicitly on an instance are emitted as before. ## 3. View targeted by HasCause/HasEffect (Base Info Core Structure 2 `001.js`): sample model defect #4485 added `Views_Operations HasCause/HasEffect Views_Engineering` so that View Minimum Continuation Point 01 `012.js` and View Basic 2 `018.js` have at least two references to View nodes on the configured View (`/Server Test/NodeIds/NodeClasses/View` = `Views_Operations`). OPC 10000-3 §5.4 says Views "can be the SourceNode of any hierarchical Reference. They shall not be the SourceNode of any NonHierarchical Reference." `Views_Operations` now Organizes `Views_Engineering` and a new View, `Views_Maintenance`, which organizes `DataAccess`. This keeps two View-class references and adds no loops (ContainsNoLoops stays true). ## Tests - `NodeStateGeneratorTests.TypeGeneratesEventReferencesAreNotCopiedToObjectInstances`: covers the type, the placeholder, the instance, `CreateInstanceOf`, Methods and an explicit instance reference. It fails without the fix. - `ReferenceServerTest.SampleViewsHaveNoNonHierarchicalReferencesAsync`. The existing `NodeClassSampleNodesReferenceTheirOwnNodeClassAsync` still passes. - Results with net10.0: - SourceGeneration.Core.Tests: 4267 passed, 0 failed - InformationModel.Tests: 1078 passed, 0 failed - Di.Tests, including RobotOpenUsdE2e: 438 passed, 0 failed - OpenUsd.Tests: 921 passed, 0 failed - Server.Tests `ReferenceServerTest`: 56 passed, 0 failed - `UA.slnx` builds with net10.0. The changed test projects build with net48. ## CTT (patched 1.05.513 scripts, compared with the 2026-10-03 patched baseline) | Part | Change | | --- | --- | | p02-base-info | Base Info Core Structure 2 `001.js`: Error → Skipped. The GeneratesEvent and View errors are gone; the remaining skips are the CTT's own "type not defined within the test-script" notes. No other change. | | p03-base-view-method | No change. View Basic 2 `018.js` and View Minimum Continuation Point 01 `012.js` still pass. | | p13-monitored-items | Deadband `Err-006.js` still Error (C56). With the script fix it passes. Monitor Basic `007.js` gave a response-timestamp delay warning (>200 ms) caused by machine load; it also appears in the 2026-09-29 base run. | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
2415503ab9 |
Close CTT skips in the reference server sample address space, diagnostics EnabledFlag and project settings (#4485)
Closes #4479 The CTT skipped a number of reference-server test cases because the sample address space, the diagnostics EnabledFlag or the CTT project settings did not provide what the scripts need. This PR fixes the sample data and the server behaviour behind those skips, updates `samples/UAReferenceServer.ctt.xml`, and records the CTT script defects that the newly running cases expose. ## Changes ### Sample address space (`ReferenceServer.NodeSet2.xml`, `ReferenceNodeManager`) | # | Skip | Fix | | --- | --- | --- | | 1 | Base Info Engineering Units `004.js`: ArrayItemType `EngineeringUnits` have an empty NamespaceUri | UNECE `mV` (12890) EUInformation, and the axis definitions use `s` (5457219) instead of UnitId 0. | | 2 | Data Access PercentDeadband `006/016/017`: `DataAccess_AnalogType_Float` EURange 0..0 | EURange 0..100 like the other analog items. | | 3 | Attribute Read `033.js` (19 nodes): second dimension of 3 | All `Scalar_Static_Arrays2D_*` are 5x5 (generated values; the Variant matrix holds scalar Int32 elements). The code-seeded Integer/Number/UInteger matrices are 5x5 too. | | 4 | View Minimum Continuation Point 01 `012.js`, View Basic 2 `018.js`: Method/View nodes need two references of their own NodeClass | `Methods_Void` HasCause/HasEffect to two methods, `Views_Operations` HasCause/HasEffect to `Views_Engineering`. | | 5 | Base Info Server Capabilities 2 `002.js`: no node without MinimumSamplingInterval | `DataAccess_DataItem_String` reports `Bad_AttributeIdInvalid` for the optional attribute (Part 3 §5.6.2). A DA item is used because Address Space WriteMask writes every attribute of the static scalars back. | | 6 | Historical Access Read Raw `Err-025.js`: no non-historizing Static Scalar | New `Scalar_Static_NonHistorizing_Boolean` (HistoryRead access, Historizing = false). See the historian change below. | | 10 | Address Space Atomicity `001.js`: no NonatomicRead node found | New `AccessRights_AccessAll_NonatomicReadWrite`. The script examines only the first 10000 variables sorted by NodeId string; the 5000 mass variables sort before `Scalar_Static_NonatomicReadWrite`. | ### Server library - **Diagnostics EnabledFlag (item 7, Base Info Diagnostics `018-1/2/3`).** `Server.ServerDiagnostics.EnabledFlag` is writable for SecurityAdmin or ConfigureAdmin over SignAndEncrypt (UserAccessLevel hook, asynchronous write handler); other users get `Bad_UserAccessDenied`. Transitions are serialized, including the node removal. Disabling returns `Bad_NotReadable` for ServerDiagnosticsSummary, its children and the diagnostics arrays, and removes the session and subscription diagnostics nodes (Part 5 §6.3.3). The server keeps the registrations: enabling restores the nodes of the live sessions and subscriptions with the same NodeIds (sessions created while disabled included) and resets the arrays to Good. Counters stay cumulative (required by `018-1.js`). Subscriptions created while diagnostics are disabled still get no diagnostics node. - `DiagnosticsNodeManager.IsDiagnosticsStructureNode` had an inverted NodeId check, so monitored diagnostics nodes never started the scan timer; `OnMonitoringModeChangedAsync` counted every node and replaced a running timer. Both fixed. - **Non-historized nodes (item 6).** `HistorianDispatcher` raw/modified, processed and at-time reads ask the resolved provider `IsHistorizingAsync` and return `Bad_HistoryOperationUnsupported` for a node it does not historize, instead of the empty result the in-memory historian gave for any node. - **Browse order (item 8).** `ReferenceDictionary.Find` returned references of a type from hash-based target maps, so a Browse filtered by ReferenceType returned references in a different order than an unfiltered Browse. It now returns them in insertion order (no extra allocation; a single match keeps the fast path). View Basic 2 `015.js` compares both lists by index. The characterization test in `NodeStateReferenceStorageTests` is updated accordingly. ### CTT project settings (`samples/UAReferenceServer.ctt.xml`) | Setting | Old | New | | --- | --- | --- | | `/Server Test/NodeIds/Static/All Profiles/Scalar/Bool` | `ns=2;s=Scalar_Static_Boolean` | `ns=2;s=Scalar_Static_NonHistorizing_Boolean` | | `/Server Test/NodeIds/References/Has References of a ReferenceType and SubType` | `ns=2;s=References_HasReferenceTypeAndSubType` | `i=2253` | | `/Server Test/NodeIds/NodeClasses/Object` | `ns=2;s=Scalar` | `i=2253` | `References_HasReferenceTypeAndSubType` loses its HasComponent/HasOrderedComponent references in the source generator; that is tracked in #4484 and not changed here. The Server object has HasComponent and HasAddIn references. ### Decisions - **Item 11, Attribute Write StatusCode & Timestamp `003/006/008` (writing ServerTimestamp): unchanged.** The stack rejects writes that contain a ServerTimestamp (`NodeState`/`BaseVariableState`), which Part 4 §5.11.4.2 allows (`Bad_WriteNotSupported`); the CTT reports it as "Not Supported", not as an error. Side finding, not changed: the StatusWrite/TimestampWrite AccessLevel bits are not enforced. - **Item 6:** Part 4 §5.11.3.4 (Table 52) specifies `Bad_HistoryOperationUnsupported`; `Err-025.js` accepts only `Bad_NotSupported`, so the case now runs and reports that as an error. Recorded as a CTT defect in `plans/ctt-issues.md`. - **Item 12:** removed the stale Historical Access `Err-012.js` entry (fixed by #4026). `plans/ctt-issues.md` adds C32 (`Err-025.js` status code), C33 (Atomicity `001.js`: 10000 cap, operator precedence, substring matching of `UaNodesToIgnore`) and C34 (View Basic 2 `015.js` index comparison), and replaces the Err-025/Err-012 configuration notes with the new settings and the EnabledFlag access requirement. Monitor Basic `039.js` is already C17. The run procedure in `plans/ctt-testing.md` is unchanged. ## Verification CTT 1.05.06, scripts 1.05.513, `ConsoleReferenceServer --ctt -a`, fresh server per conformance unit/group. "master" = |
||
|
romanett
|
162fc5ebb3 |
Fix AddNodes scaling and OpenSecureChannel revocation errors found by CTT Monitored Item, Node Management and security runs (#4486)
# Description Findings from running the CTT conformance groups **Monitored Item Services**, **Node Management Services**, **Security General** and **Security User Token** against `ConsoleReferenceServer --ctt` (CTT 1.05.06, scripts 1.05.513). The security groups ran **without `-a`**, against an isolated server PKI built from the CTT's `copyToServer` folders, so the negative certificate tests are meaningful. ## Server fixes ### AddNodes below one parent was quadratic (Node Management Delete Node `Err-002.js`) The CTT adds 15,000 variables below `ns=2;s=CTT` in batches of 5,000 and warned that AddNodes responses took 400–800 ms. For every added node the server searched all children of the parent twice: once in the BrowseName duplicate check in `AsyncCustomNodeManager.AddNodeCoreAsync`, which also copied the children list, and once in the NodeVersion lookup of `FilterChangesByNodeVersion`. - `NodeState.FindChild` now searches children lists with ≥ 64 entries through a BrowseName index kept in a `ConditionalWeakTable`, so nodes with few children get no extra field. - It keeps first-match semantics and is maintained by `AddChild`/`RemoveChild`. - It is dropped on replacement and invalidated when a child is renamed or moves to another parent. - Lists whose children point at another parent (clones) are still searched linearly. - The duplicate check uses `FindChild` instead of copying the list. | 5,000-node AddNodes batch (in-process, Debug) | 1st | 2nd | 3rd | | --- | --- | --- | --- | | before | 769 ms | 2,083 ms | 4,011 ms | | after | 141 ms | 166 ms | 172 ms | In the CTT, AddNodes of 5,000 items now answers in 51–72 ms and the warning is gone. ### OpenSecureChannel leaked revocation errors (Security Certificate Validation `002.js`) Part 4 §6.1.3, Table 106: - For the revocation check (`Bad_CertificateRevoked`, `Bad_CertificateIssuerRevoked`), `Bad_SecurityChecksFailed` **shall** be reported to the client. - For a missing revocation list (`Bad_CertificateRevocationUnknown`, `Bad_CertificateIssuerRevocationUnknown`), it **should** be. `TcpServerChannel` masked only `Bad_CertificateRevoked`, and `StandardServer.OnApplicationCertificateError` (CreateSession) had its own list that missed `Bad_CertificatePolicyCheckFailed`. Both now use one helper, `CertificateErrorReporting` in Opc.Ua.Core, which also inspects the nested errors of the certificate validator (both paths now throw the validation result with `ThrowIfInvalid`). Only validity period, host name, URI and usage errors keep their own code, and audit events still carry the specific result. The CTT warning is gone. ### Sample data - **Monitor Value Change V2 `020.js`** skipped because `Scalar_Static_Arrays_ByteString` had a 1-byte and a 3-byte element. The script checks every element, and the simulation only padded elements 0–2. The seed value now has no element shorter than four bytes, and the simulation pads every element. - **Monitor Value Change V2 `042.js`** got 18 of 19 notifications. The missing item is the Float matrix: its element [1,1] was `-1.03179664E+33`, where adding 1 is below Float precision, so the value really did not change. The server did not drop a notification. The element is now `-1031.79664`. ## CTT results after the fixes | Group | Result | | --- | --- | | Monitored Item Services (208) | Monitor Value Change V2 44/44. Remaining: Monitor Basic `039.js` (C17, missing include), `038.js` warning (C32), and 1–2 sporadic timestamp-tolerance warnings that move between test cases from run to run. | | Node Management Services (15) | Only Add Node `Err-008.js` (Mantis 11266). | | Security General (314) | Certificate Validation: 23 pass, 24 not implemented, 3 not-applicable skips. None CreateSession/ActivateSession: all pass. The other 50 CUs are *Not Implemented* in scripts 1.05.513. | | Security User Token (66) | Anonymous `002.js` (C35) and User Name Password 2 `015.js` (C36) fail because of CTT defects. X509: 18/18 automated cases pass. | ## CTT defects documented (`plans/ctt-issues.md`) - **C32** `038.js` compares a RevisedSamplingInterval of 0 with a project setting instead of the node's MinimumSamplingInterval or the server's MinSupportedSampleRate. - **C33** `020.js` requires every ByteString element to be ≥ 4 bytes, but its message says elements 0–2. - **C34** `RequestedNewNodeId()` always sends `ns=1`, regardless of `RequestedNodeId_Namespace`, so client NodeIds can't be enabled. - **C35** Security User Anonymous picks the `opc.wss` endpoint, which the CTT cannot open (`BadNotSupported`). - **C36** `UaEndpointDescription.FindTokenType` rewrites the cached endpoints' token policies, so `015.js` reports PolicyIds as reused. The server's GetEndpoints output was verified compliant (PolicyIds unique per configuration, Mantis 11258). - Evidence added to **C9** (042.js) and **issue 17** (015.js). `plans/ctt-testing.md` gains how to run the Monitored Item/Node Management groups and a recommended setup for the security groups: isolated PKI from `copyToServer`, no `-a`, PKI restore per part, and waiting for `Server started`. Other findings recorded there: - the Create/DeleteMonitoredItems timing warnings are jitter; - `LoginNameAccessDenied` is not a real user; - `samples\UAReferenceServer.ctt.xml` needs no change (enabling `RequestedNodeId` breaks six Add Node cases because of C34). **Note:** `plans/ctt-testing.md` and the refreshed `plans/ctt-issues.md` are introduced by #4482. The plans files here are based on #4482's versions, and new entries start at C32 because #4481 uses C19–C31. Please merge after #4482 (or rebase the plans files onto it). ## Tests - `NodeStateChildNameIndexTests` (Opc.Ua.Types.Tests): index semantics (duplicates, remove, rename, replace, re-parent, clone, shrink/grow, 2,000 random operations against a linear search). Also a scan guard: 5,000 misses against 50,000 children take 97 ms with the index and 7,083 ms with it disabled. - `AsyncCustomNodeManagerNodeManagementTests.AddNodeAsync_DuplicateBrowseNameUnderParentWithManyChildren…` - `TcpServerChannelCertificateErrorTests` (Opc.Ua.Core.Tests): masked vs reported certificate errors. - Opc.Ua.Server.Tests 5,553 passed, Opc.Ua.Core.Security.Tests 556 passed, Opc.Ua.Core.Tests (Security + Stack) 3,561 passed, InformationModel NodeManagement 26 passed (net10.0). - Opc.Ua.Types.Tests 10,404 passed and 1 failed: `WotSpecFixtureManifestTests.EveryEmbeddedExampleMatchesItsRecordedHash`. That test hashes WoT assets which are `eol=lf` in the index but CRLF in this worktree's checkout; no WoT file is touched by this PR. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING doc. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added all necessary documentation. - [ ] I ran all tests locally using UA.slnx against .NET Framework and .NET 10 (net10.0 suites listed above only). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> |
||
|
romanett
|
d547747e22 |
Generate ReferenceNodeManager address space from a NodeSet2 model (#4220)
# Description The CTT/compliance `ReferenceNodeManager` built its ~6,000 node address space entirely in imperative C#. This PR migrates it to a source-generated `[NodeManager]` partial driven by an exported NodeSet2 model (`ReferenceServer.NodeSet2.xml`), mirroring the `MinimalBoilerServer` sample and completing the goal of issue #4204. The address space is now materialized from the model by the source generator. The large orchestration body was kept but converted from create-based to **find-instead-of-create**: each `Create*` primitive now looks up the model-materialized node via `FindPredefinedNode<T>` and re-applies runtime values, DataAccess metadata, and write handlers, rather than constructing nodes from scratch. This keeps the deterministic value seeding and behavior wiring intact while the structure comes from the model. ## Notable details for reviewers - **Idempotent reference wiring.** The mesh references in the `References` folder are already present in the model, so the historic manual `AddReference` calls would throw on duplicates. Added a small `AddReferenceIfMissing` helper so those calls are safe. - **Non-string NodeIds.** The `NodeIds` folder assigns numeric / GUID / opaque NodeIds. The model exports those nodes with their final ids, so find-by-string-path no longer works; added `CreateVariableById(NodeId)` to locate them by their real id. - **Generator bug fix (worth a careful look).** `ModelDesignExtensions.GetValueRankAsCode` reconstructed `ValueRank` as `dimensions.Length + 1`, so a node with `ArrayDimensions="3,3"` was emitted as `ValueRank = 3`. This affected **any** NodeSet2 with a fixed multi-dimensional array (rank-3 matrices, `BadTypeMismatch` on whole-matrix writes, wrong decoded dimensions). Corrected to `dimensions.Length` and updated the unit tests that had codified the off-by-one. ## Validation - `ReferenceServerTest`: 44/44 on net10.0 and net48 - `Opc.Ua.SourceGeneration.Core.Tests`: pass on net8.0/net9.0/net10.0/net48 - Integration on net10.0: ComplexTypes, Client, History, Sessions, Subscriptions all pass - Clean build, no new analyzer warnings ## Related Issues - Fixes #4204 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |