mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-21 09:06:37 +08:00
2.0.0
318 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
1fa492660c |
Test version.targets preview mapping through MSBuild
ApplyPreviewPackageVersion is what stamps package versions, but only its PowerShell mirror (ConvertTo-PreviewPackageVersion) was tested. Evaluate the real target through dotnet msbuild for the same inputs and assert both implementations produce the expected version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
866662e912 |
Number commit-id-only prerelease previews 0 so they never outrank a release
A non-public build of a stable version.json (master briefly at "2.0.0") gets the root version "2.0.0-g<commit>". version.targets Case 2 turned it into "2.0.0-preview.g<commit>", which SemVer 2 ranks above every numbered "-preview.N" - the cause of the release/2.0 ordering gate failure. Map a commit-id-only prerelease to "-preview.0.g<commit>" instead, in both version.targets and ConvertTo-PreviewPackageVersion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
6812621abd |
Exempt unremovable preview.g<commit> versions from the preview ordering gate
Two master NuGet Publish runs on 2026-10-06 (37461428995, 37462705289) pushed 2.0.0-preview.gfd360271d9 and 2.0.0-preview.ge78c648295 to GitHub Packages while master briefly carried the stable root version 2.0.0. SemVer 2 ranks the alphanumeric "g<commit>" identifier above every numeric one, so no PreviewPackageBuildNumber can outrank them and the stable 2.0.0 candidate on release/2.0 fails validate-preview-package-ordering.ps1. Add an explicit PreviewPackageOrderingExemptions list to preview-version.props that the gate skips by exact (case-insensitive) match; everything else still fails closed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
fa06c66dbb |
Stop versioning DI and ISA95 packages as preview (#4686)
## Summary Removes the **DI** (`Opc.Ua.Di*`) and **ISA95** (`Opc.Ua.ISA95*`) package families from the preview-only set, so they ship with the stable root version (e.g. `2.0.1` instead of `2.0.1-preview.N`). - `version.targets`: drop the two `_IsPreviewPackage` conditions. - `.azurepipelines/package-version-policy.ps1`: drop `ISA95|Di` from `Test-PreviewPackageId`. It must match `version.targets`, because the package-set validator fails if the two disagree. - `PackageVersionPolicyTests`: ISA95/Di IDs are now expected to be non-preview, they are removed from the classifier-agreement token list, and the "already-preview base version" case now uses Robotics. - Docs (`DeveloperGuide.md`, `ReleaseProcess.md`, `docs/README.md`): the preview-family list no longer includes DI or ISA95, and the "preview package" label is removed from the DI, Software Update (part of the DI packages), and ISA-95 entries. ## Test `dotnet test tests/Opc.Ua.Tools.Tests --filter PackageVersionPolicy` (net10.0): 87/87 passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
5572107ab8 |
Push Model compliance: UpdateCertificate issuer check, TrustList audit events, NoConfiguration state, DefaultHttpsGroup (#4684)
# Description
Fixes found by a manual run of the OPC 10000-12 *Push Model for Global
Certificate and TrustList Management* test plan (183 test cases,
TC0–TC182) against the Reference Server. After these changes no test
case fails; the remaining non-pass verdicts are test-plan expectations
that contradict Part 12/Part 4/Part 20 (documented in the run report).
### Server behaviour changes
- **UpdateCertificate requires the issuer in the TrustList**
(`ConfigurationNodeManager.PushValidation.cs`). The validator classifies
an incomplete chain as suppressible; OPC 10000-4 Table 100 does not ("An
error during the chain creation may not be suppressed") and §7.10.5
requires the issuers to already be in the group's TrustList. A
certificate of an unknown CA is now rejected with
`Bad_CertificateChainIncomplete`. Expiry / host-name errors stay ignored
as §7.10.5 requires.
- **TrustList audit events reach the Server Object.**
`TrustListUpdateRequested`/`TrustListUpdated` were reported on the
TrustList node (`EventNotifier=0`, not linked to the Server), so no
client could receive them. `TrustList.SetAuditEventServer` routes them
through `IAuditEventServer`; wired for the Push TrustLists and the GDS
CertificateGroup TrustLists. New `AuditEvents` overloads keep the
node-based ones.
- **Duplicate AddCertificate** no longer makes the following
ApplyChanges fail with a bare `Bad` (store refused the second copy); the
commit is idempotent and its rollback keeps the pre-existing
certificate.
- **CloseAndUpdate validates CRLs** (incl. CRL-only updates): every CRL
must be signed by a CA of the new TrustList (§7.8.2.5). Previously an
unknown/forged CRL passed CloseAndUpdate and only failed ApplyChanges.
- **X.509 v1 certificates are rejected** by
CloseAndUpdate/AddCertificate/UpdateCertificate (OPC 10000-6 §6.2.2; new
`X509Utils.IsX509Version3`).
- **Application setup state (Annex G.2):** new
`StandardServer.StartupServerState` (virtual); the DI host returns
`NoConfiguration` when `ServerConfigurationOptions.InApplicationSetup ==
true`. `IsRunning` and `NodeManagerLifecycle` treat `NoConfiguration` as
operational. The Reference Server `--provision` mode starts in
`NoConfiguration` and exposes `InApplicationSetup=TRUE`.
- **DefaultHttpsGroup:** the group is only created when Https stores are
configured — the Reference Server config had none, so its
`opc.https`/`opc.wss` endpoints used the RSA application certificate
with an empty TLS trust list and the Https certificate could not be
pushed. The sample config now declares
`HttpsIssuerCertificates`/`TrustedHttpsCertificates` and an
`HttpsCertificateType` certificate (new `CertificateTypeString` value
`Https`). An Https certificate is assigned to DefaultHttpsGroup only (no
longer also listed in DefaultApplicationGroup). Fixed
`AddSecurityConfigurationHttpsStore`, which built the trusted Https
store path from the store *type*.
- Minor: AddCertificate logs a warning when it adds a CA without a CRL
while `RejectUnknownRevocationStatus` is enabled; UpdateCertificate
refused for an insufficient SecureChannel raises
`CertificateUpdateRequestedAuditEvent` with Status=false.
### Kept on purpose
- PEM certificates are accepted (converted to DER).
- `Bad_NotWritable` when opening an already open TrustList (OPC 10000-20
§4.2.2) and for AddCertificate while a read handle is open (§7.8.2.6).
- No validation rules for DefaultUserTokenGroup / DefaultHttpsGroup
TrustLists (Part 12 defines them only for ApplicationCertificateType
groups).
### Notes for reviewers
- Existing tests in `ConfigurationNodeManagerPushTests` and
`PushCertificateRequestRegressionTests` signed pushed certificates with
CAs that were not in the TrustList; they now trust their CA first
(behaviour change above).
- Adding the Https certificate to
`Quickstarts.ReferenceServer.Config.xml` gives the Reference Server's
opc.https/opc.wss endpoints their own TLS certificate.
### Tests
-
`tests/Opc.Ua.Server.Tests/Configuration/PushComplianceRegressionTests.cs`
— duplicate add/rollback, PEM, v1/v3, CRL issuer/forged/CRL-only, audit
routing.
- `tests/Opc.Ua.Server.Tests/ApplicationSetupServerStateTests.cs` —
NoConfiguration startup state.
- `tests/Opc.Ua.Gds.Tests/PushComplianceTests.cs` — end-to-end over a
secure channel: TrustList transaction lifecycle, open-handle rules,
masked reads, duplicate/PEM add, unknown CRL issuer, unknown certificate
issuer, cross-Session CSR key, nonce rules, RemoveCertificate chain/CRL
handling. Mutation-checked: reverting the issuer, duplicate-add and CRL
fixes makes the corresponding tests fail.
- Additional cases in `PushCertificateRequestRegressionTests`,
`CertificateIdentifierTests`, `ApplicationConfigurationBuilderTests`.
Local runs (net10.0): Opc.Ua.Gds.Tests 1257 passed / 0 failed / 56
skipped; Opc.Ua.Server.Tests push/TrustList/audit/lifecycle subset 1209
passed; Core and Configuration test classes passed. net48 build of the
affected test projects succeeds; `dotnet format whitespace` clean.
## Related Issues
- None (findings from a manual Push Model compliance run).
## Checklist
- [x] 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.
- [ ] 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.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
c03c128384 |
Treat code analysis (CA) warnings as errors and fix all surfaced analyzer warnings (#4680)
# Description `TreatWarningsAsErrors` was already enabled, but `CodeAnalysisTreatWarningsAsErrors` was `false`, so Microsoft Code Analysis (`CAxxxx`) warnings were non-fatal. This PR sets it to `true` in `common.props` and fixes all 55 analyzer warnings the net10.0 build reported. **Production code** - `SamplingGroup.cs`: CA2213 is a false positive here because the detached `CancellationTokenSource` is disposed by `CancelSamplingLoop` once the loop completes. It is suppressed with a pragma, a justification and a TODO, following the existing pattern in `ServerBase.RequestQueue.cs`. **Tests** - CA1861: constant array arguments are moved to `static readonly` fields or replaced with `ArrayOf<T>` collection expressions. In the legacy interop server I used locals instead, because those arrays become node values that writes can change in place. - CA2025: tasks that finish before disposal (`Assert.ThrowsAsync`, or awaited before `using` scope exit) are suppressed with justification comments, following existing test patterns. - CA1812: classes created by the DI container are suppressed with a comment and a TODO. - CA2208: benchmark switch defaults now throw `InvalidOperationException`. - CA1845: `Substring(0, 8)` is replaced with `[..8]`. - CA1307: `string.Replace` is replaced with `Split`/`Join`, which also works on net48. - CA5394: `Random` is replaced with `RandomNumberGenerator`. **Docs** - `docs/DeveloperGuide.md` now says CA warnings also fail the build, and that any suppression needs a comment explaining why plus a TODO. **Validation** - I fixed the 55 warnings from a net10.0 build of `UA.slnx` (`CustomTestTarget=net10.0`). That build ran before the setting was turned on, and I have not rebuilt net10.0 since the fixes. - `CustomTestTarget=net48` build of `UA.slnx`: 0 warnings, 0 errors. - I also started a full build across all target frameworks with the setting on. Its log showed no errors or warnings while it ran, but I couldn't read its final result. - No tests have been run yet; CI should confirm the multi-TFM build and the tests. ## Related Issues - N/A ## Checklist - [ ] 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. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
21e04bdba5 |
Make Bad_CertificateChainIncomplete unsuppressible; add UpdateCertificate renewal regression tests (#4683)
# Description Follow-up to the master378 fix in #4678 (CTT: application certificate renewal must use the configured trust stores). Master already ignores the supplied `issuerCertificates` for the default application group (OPC 10000-12 §7.10.5), but it had no regression tests for CA-signed renewals. While adding them I found a real gap: **The validator treated `Bad_CertificateChainIncomplete` as suppressible.** OPC 10000-4 Table 100 (Build Certificate Chain) says: "An error during the chain creation may not be suppressed." §6.1.3 adds: "Processing fails with Bad_SecurityChecksFailed if an element in the chain cannot be found." UpdateCertificate for the application group accepts every *suppressible* error (as §7.10.5 requires). As a result it accepted a renewal signed by a CA that is **not** in the TrustList and was only passed in the ignored issuer list, which is exactly what §7.10.5 rules out. ## Changes - `CertificateValidationCore`: remove `BadCertificateChainIncomplete` from the suppressible status codes. An accept callback (`AcceptError`) is no longer asked about it. - `TrustList`: remove the local workaround that filtered out this error; plain "accept suppressible errors" is now enough. - Docs (`Certificates.md`, `CertificateManager.md`): drop it from the suppressible list, and fix a sample that suppressed it. ## Tests - `ConfigurationNodeManagerPushTests`: - A CA-signed renewal with the CA installed succeeds with a missing, empty, unrelated or malformed issuer list, and the trust stores stay unchanged. - A renewal whose CA is only supplied as an issuer is rejected with `BadCertificateChainIncomplete`; stores and the current certificate are untouched. - End to end on an isolated server: a CA-signed renewal with no issuer list is committed through ApplyChanges. - `ConfigurationNodeManagerPushValidationTests`: the group TrustList validator accepts a CA from the trusted or issuer store and rejects an uninstalled CA. - `CertificateValidationCoreTests` / `CertificateValidatorTest`: - An incomplete chain is unsuppressible and never reaches the callback. - A leaf trusted directly **stays valid when the peer supplies its CA** (§6.1.3: chain certificates "may be provided with the application Certificate"). - One existing test attached its approver only after validating, so it never exercised suppression; it now validates with the approver attached. - Tests that only passed because of the bug (`PushCertificateRequestRegressionTests`, the regenerated-key helper and the rollback test) now install the signing CA in the TrustList first. The two new rejection tests fail on master and pass with this change. ## Reviewer notes / behavior change This was suppressible in 1.5.x as well. Application code whose accept handler approved a CA-signed peer certificate without its CA (CA neither installed nor sent by the peer) will now get a rejection. Without a handler such certificates were already rejected, and the stack's own handlers (`ApplicationInstance`, push validation) never accepted this error. Worth a release note. Local runs (net10.0), all passing: | Suite | Passed | Failed | |---|---|---| | Server.Tests | 8310 | 0 | | Core.Tests (security/transport filter) | 2039 | 0 | | Security.Certificates.Tests | 358 | 0 | | Core.Security.Tests | 558 | 0 | | Client.Tests | 2761 | 0 | | Gds.Tests (PushTest and TrustList tests) | 345 | 0 | The net48 builds of Server.Tests and Core.Tests also succeed. ## Related Issues - Related to #4678 (master378 counterpart) ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
cc7908eaa1 |
WebApi: serve a generated OpenAPI document of the REST binding (opt-in) and map the session-less service set (#4657)
Reworked after the review: the OpenAPI document is generated from the code, not embedded; the route is opt-in; `StatusCode` is not accepted. The branch is a new history on top of master (the first version conflicted); master with #4674 is merged in. # Description The REST binding implements the routes of the OPC Foundation OpenAPI mapping (OPC 10000-6 §G.3), but it does not describe them: `GET /openapi.json` is answered with HTTP 405, and a client generator has nothing to read from the server. `docs/WebApi.md` listed a generated OpenAPI document as a deferred follow-up. The coded API is the source of truth, so this PR **generates** the document from the route table and from the structure definitions of the request and response types, and compares it with the document the model compiler publishes in tests. Nothing from `UA-Nodeset/OpenApi` ships in the package. ## Changes ### Service sets (`Opc.Ua.Core`) - `WebApiServiceSet` (`AllServices`, `Sessionless`) and `WebApiServiceRoutes.GetRoutes(serviceSet)`. `Sessionless` is the eight services of `opc.ua.openapi.sessionless.json` (OPC 10000-4 §6.3): Read, Write, HistoryRead, HistoryUpdate, Call, Browse, BrowseNext, TranslateBrowsePathsToNodeIds. ### Generator (`Opc.Ua.Core.Schema`, `Opc.Ua.Schema.OpenApi`) - `WebApiOpenApiGenerator.Generate(serviceSet, includeSchemas, serverUrl)` returns the OpenAPI 3.0 document as a `JsonObject`. It is built at runtime from `WebApiServiceRoutes` and the `StructureDefinition`/`EnumDefinition` of the message types (the same definitions the schema generation uses, resolved through `IDataTypeDefinitionResolver`). No reflection, so it is NativeAOT compatible; the AOT test project covers it. - **Two levels of detail**, because full schemas make big documents: | | Content | All 28 services | | --- | --- | --- | | default | paths, `operationId`s, every body a JSON object | about 12 KB | | `includeSchemas: true` | one component schema per message, structure and enumeration (100), for typed client generators | about 46 KB | The two published files are about 300 KB together. - The schemas describe the Compact JSON encoding, with the property names, types, formats and integer ranges of the published documents. DataValue is the one structure that differs: its status property is `Status`, as in Part 6 Table 42. - `OpenApiBuiltInSchemas` holds the schemas of the built-in types (StatusCode, LocalizedText, Variant, DataValue, DiagnosticInfo, ExtensionObject and the scalar types) as the encoder writes them. Abstract `Structure` fields are ExtensionObjects, abstract number fields are Variants. ### Serving the document (`Opc.Ua.Bindings.Https`) - `WebApiTransportOptions`: `ServiceSet`, `OpenApiDocumentPath` (default `null`: **off**) and `OpenApiIncludeSchemas` (default `false`). - `MapWebApiEndpoints(WebApiTransportOptions)` maps the routes of the service set and, when a path is set, the `GET` route of the document **in the same route group**. The route has no `AllowAnonymous` metadata. When an authentication scheme is registered, the contributor's `group.RequireAuthorization()` covers the document like `POST /read`; only `/findservers` and `/getendpoints` stay anonymous. The existing `MapWebApiEndpoints()` maps all routes as before. - The document is generated once per path base (listed as a relative `servers` URL; without a path base the list is omitted) by the `WebApiOpenApiGenerator` that `AddWebApiTransport` registers, or by a default one. - `Opc.Ua.Bindings.Https` references `Opc.Ua.Core.Schema` on net8.0 and later. ### Documentation - `docs/OpenApi.md` (new, linked from `docs/README.md`): serving the document, the two levels, generating clients, the JSON generated clients send, known issues, how the document is kept correct, troubleshooting. - `docs/WebApi.md`: new section "Service sets and the OpenAPI document"; the deferred follow-up is removed. `docs/SchemaGeneration.md` and both `NugetREADME.md` files describe the generator and the options. ## Comparison with the model compiler documents The two published documents are test resources of `Opc.Ua.Core.Schema.Tests` only (README next to them: source commit and update procedure). `WebApiOpenApiConformanceTests` compares the generated document with them for both service sets: - paths, `operationId`s, and the request and response schema of every operation; - the set of component schemas the operations reach: equal (100 for all services); - every enumeration (members and values); - for every structure: property names, types, formats, integer ranges, and inheritance. Everything both describe is equal. The test lists the differences and fails when a listed one disappears or a new one appears: | Difference | Generated | Published | Reason | | --- | --- | --- | --- | | DataValue status property | `Status` | `StatusCode` | Part 6 Table 42; UA-Nodeset#146 is open. The stack only knows `Status` (encoder and decoder). | | `description`, `default` keywords | not written | spec links, .NET defaults | documentation and defaults, not contract; Compact omits default values | `WebApiOpenApiWireFormatTests` validates what the binding really writes (the default request and response of all 28 routes, plus populated requests and responses with variants, status codes, references, identity tokens and notifications) against the generated component schemas. ## Tests - `Opc.Ua.Core.Schema.Tests` `OpenApi/`: `WebApiOpenApiGeneratorTests` (shape of the document, both levels, both service sets, servers list, sorting and determinism, concurrency, argument and resolver errors, every built-in type, arrays and ranks, enumerations, option sets, unions, base types, self-references, unknown definitions), `WebApiOpenApiConformanceTests`, `WebApiOpenApiWireFormatTests`, `OpenApiBuiltInSchemasTests`: 81 tests. - `Opc.Ua.Bindings.Https.WebApi.Tests`: `WebApiOpenApiDocumentEndpointTests` (TestServer with the startup contributor: off by default, configured path, both levels, service set maps and describes only the sessionless routes, path base, repeated requests, registered generator is used, **a request without credentials gets 401 when an authentication scheme is registered while one with credentials gets 200, the route carries `IAuthorizeData` and no `IAllowAnonymous`**, argument checks), options defaults and DI registration of the generator. - `Opc.Ua.Core.Tests`: `GetRoutes` for both sets and an undefined value. - `Opc.Ua.Aot.Tests`: the generator and a served document under the AOT fixture. - Results (macOS arm64): `Opc.Ua.Core.Schema.Tests` 260/260, `Opc.Ua.Bindings.Https.WebApi.Tests` 457/457, `Opc.Ua.Core.Tests` WebApi tests 110/110, each on net10.0 and net8.0 (Release). `Opc.Ua.Aot.Tests` (JIT run) SchemaAotTests 5/5, WebApiAotTests 8/8. - `Opc.Ua.Core`, `Opc.Ua.Core.Schema` and `Opc.Ua.Bindings.Https` and the three test projects build for net48, net8.0, net9.0 and net10.0 (Release) with 0 warnings and 0 errors (net48 builds only; the Windows legs of the fork CI run its tests). `dotnet format` whitespace, style and analyzers (`--severity info`) are clean on every changed file; the two `RCS1090` hits in `SchemaAotTests.cs` are on lines that master already has. - Patch coverage with `.azurepipelines/check-coverage.ps1` against master (`Opc.Ua.Core.Schema.Tests`, `Opc.Ua.Bindings.Https.WebApi.Tests` and the WebApi tests of `Opc.Ua.Core.Tests`, net10.0): 362/362 changed lines covered. - Fork CI (full matrix incl. macOS) and CodeQL on `f990af5db` (master with #4674 merged in): [CI run 37310622902](https://github.com/biancode/UA-.NETStandard/actions/runs/37310622902) green, [CodeQL run 37310626786](https://github.com/biancode/UA-.NETStandard/actions/runs/37310626786) green. The run on the earlier head `abda0e491` had one failure outside this PR: `McpServerProcessTests.HttpTransportAliasesExposeOnlyMcpRouteAsync("http")` timed out after 30 s on Windows net10.0; this PR does not touch `tools/`. ## Not in this PR - Authentication: the fix discussed in the review is @romanett's #4674. The document route uses whatever the REST routes use; this PR does not change authentication. - `StatusCode` tolerance in the decoder: dropped, the spec text is the reference. The first commit of the previous version is gone. - Samples. - Session-less invocation on the server side (OPC 10000-4 §6.3) is a separate draft PR. With `ServiceSet = Sessionless` this PR only limits the mapped routes and the document. ## Questions 1. `Opc.Ua.Bindings.Https` now references `Opc.Ua.Core.Schema` (net8.0 and later) because the generator lives next to the other schema generators and is trimmed away when unused. Fine, or should it move into `Opc.Ua.Core`? 2. Should `OpenApiIncludeSchemas` default to `false` as here, or should a host get typed clients out of the box? ## Related Issues - Fixes #4656 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
|
b0d8ca0d3a |
Server: session-less service invocation (OPC 10000-4 §6.3) with AddSessionlessInvocation (#4659)
Marc answered the five design questions (2026-10-05). This update
implements the answers; see "Design decisions" at the end.
# Description
OPC 10000-4 §6.3 lets a client invoke a subset of the services without
CreateSession/ActivateSession. The request then carries either an Access
Token in `RequestHeader.authenticationToken` or no token at all. The
server today only offers the low-level
`ISessionManager.ValidateSessionLessRequest` event. Without a handler,
every request whose token names no session is answered with
`Bad_SessionIdInvalid`, including a request without a token. That is the
code a client of an expired session expects; for a session-less request,
§6.3.1 has a server that does not support the invocation answer
`Bad_ServiceUnsupported`.
This PR adds built-in session-less invocation behind an opt-in and keeps
the existing event as the way to take over completely.
## Changes
### `AddSessionlessInvocation()`
(`src/Opc.Ua.Server/Hosting/OpcUaServerBuilderExtensions.cs`)
```csharp
services.AddOpcUa()
.AddServer(options => { /* … */ })
.AddSessionlessInvocation(o => o.AllowAnonymous = false) // the default
.AddJwtIssuer(/* … */); // validates the Access Tokens
```
- Registers `SessionlessInvocationOptions`.
`DependencyInjectionStandardServer` applies them to the `SessionManager`
in use, including one created by a registered session manager factory,
unless that manager already has options.
- Without dependency injection: `SessionManager.SessionlessInvocation =
new SessionlessInvocationOptions()`.
- Without either, nothing changes apart from the
`Bad_ServiceUnsupported` answer below.
### `SessionlessInvocationOptions` (new, `src/Opc.Ua.Server/Session/`)
- `AcceptAccessTokens` (default `true`): an Access Token in
`RequestHeader.authenticationToken`, carried as a String NodeId, is
handed to the server's identity registry (`IServerIdentityRegistry`, the
one ActivateSession uses) as an issued JWT token (OPC 10000-6
§7.6.5.2.3). The resulting identity runs the request. §6.3.1 requires an
encrypted channel for the token, so it is only accepted over
SignAndEncrypt or HTTPS.
- `AllowAnonymous` (default `false`): a request without a token runs as
an anonymous user. Without it, such a request is rejected with
`Bad_IdentityTokenInvalid`.
- `MaxConcurrentRequests` (default 64) and
`MaxConcurrentRequestsPerChannel` (default 16): the budget for
session-less requests, see below. Zero does not limit; a negative value
is rejected.
### Budget for session-less requests (`SessionlessRequestBudget`, new,
internal)
#4541 bounds channels and incomplete messages. A session-less request
has no Session to be accounted for, but it occupies a worker and keeps
its channel active. It gets a budget of its own:
- **Two limits**, checked and taken together under one
`System.Threading.Lock`: concurrent requests on the server, and on one
channel. A request refused by the channel limit takes no server-wide
place. The channel is the SecureChannel on opc.tcp. The HTTPS bindings
use one SecureChannel id per listener, so their requests are grouped by
peer address (IPv4-mapped IPv6 addresses count as IPv4).
- **Answer**: `Bad_ServerTooBusy` ("The Server does not have the
resources to process the request at this time."), the code #4541's
request queue uses when it has no room. `Bad_TooManyOperations` means
too many operations in one request, so it does not fit.
- **Lifetime**: the place is taken before the identity is checked, which
also bounds the validations that run at the same time. The lease is
attached to the `OperationContext` and returned when the context is
disposed, which every service does with `using`, however the request
ends (completed, failed, cancelled). A rejected token, a handler error
or exception, and a cancelled validation return it at once. A lease
releases once, however often it is disposed. Channel entries are removed
with their last request.
- **Scope**: it applies to every session-less request on a manager that
has `SessionlessInvocation` set, including requests a
`ValidateSessionLessRequest` handler decides. A handler without
`AddSessionlessInvocation` runs without a limit, as before. Requests
with a Session token and services outside the §6.3.1 set are not
counted.
- The limits are read for each request, so a change of the options
applies to the next request.
### `SessionManager`
- **Service sets:** a request without a session is limited to the
service sets §6.3.1 allows: View (without
RegisterNodes/UnregisterNodes), Attribute, Method, NodeManagement and
Query. Every other service keeps answering `Bad_SessionIdInvalid`.
`IsSessionlessService(RequestType)` is public.
- **Request without a token:** with session-less invocation not enabled
and no `ValidateSessionLessRequest` handler, it is answered with
`Bad_ServiceUnsupported` (§6.3.1). A token that names no session stays
`Bad_SessionIdInvalid`.
- **Precedence:** a registered `ValidateSessionLessRequest` handler
still decides first, unchanged apart from the service-set limit.
`ValidateSessionlessRequestAsync` is `protected virtual` for servers
that need their own rules.
- **Roles:** the identity of a session-less request goes through the
role resolution ActivateSession applies to the effective identity of a
Session (the `ResolveRoles` part of `AddMandatoryRoles`: the
`MustChangePassword` restriction, `IRoleManager.ResolveGrantedRoles`, so
mapped Access Token claims and the built-in AuthenticatedUser role).
This includes an identity a `ValidateSessionLessRequest` handler sets.
There is no client application certificate without a Session, so no
TrustedApplication role and no application-based mapping.
- **Cancellation:** cancelling an Access Token validation propagates as
`OperationCanceledException` so the endpoint maps it to the status of
the request lifetime (previously it became `Bad_UnexpectedError`). The
budget place is still returned.
- **Log:** a rejected Access Token is logged with a dedicated
`[LoggerMessage]` (`ServerEventIds.SessionManager + 18`, the next free
id after #4632).
### Documentation
- `docs/SessionlessInvocation.md` (new, linked from `docs/README.md`):
which services, enabling it, caller identity, the answers without the
feature, the request limits, the handler, calling the server over
opc.tcp and the REST binding.
- `docs/ResourceIsolation.md`: a short section "Requests without a
Session" pointing to the budget.
- `docs/DependencyInjection.md`: `AddSessionlessInvocation` in the
identity extension tables.
## Tests
- `Opc.Ua.Server.Tests/SessionlessInvocationTests` (mocked server
internals):
- `SessionlessRequestIsUnsupportedUnlessEnabled`,
`AnUnknownSessionTokenStaysBadSessionIdInvalid`,
`ServicesOutsideTheSessionlessSetStillNeedASession` (7 services),
`TheServiceSetLimitAppliesBeforeAValidateSessionLessRequestHandler`,
`AValidateSessionLessRequestHandlerTakesPrecedenceAsync`;
-
`SessionlessRequestWithoutIdentityIsRejectedUnlessAnonymousIsAllowedAsync`,
`SessionlessAccessTokenIsValidatedByTheIdentityRegistryAsync`,
`AnAccessTokenNoAuthenticatorHandlesIsRejected`,
`SessionlessAccessTokenRequiresAnEncryptedChannel` (None, Sign),
`SessionlessAccessTokenIsAcceptedOverSignAndEncryptAsync`,
`AccessTokensCanBeDisabled`,
`IsSessionlessServiceFollowsTheServiceSetsOfPartFour` (21 cases),
`ValidateSessionlessRequestRejectsMissingArguments`;
- wiring: `AddSessionlessInvocationRegistersTheConfiguredOptions`,
`AddSessionlessInvocationRejectsANullBuilder`,
`TheHostedServerAppliesTheOptionsToItsSessionManager`,
`TheHostedServerAppliesTheOptionsToAFactoryCreatedSessionManager`,
`TheHostedServerKeepsOptionsTheSessionManagerAlreadyHas`.
- End to end against the reference server,
`Opc.Ua.Sessions.Tests/SessionlessServiceInvocationTests` (opc.tcp,
`SessionClient` on a channel without CreateSession):
- `ReadWithAnAccessTokenNeedsNoSessionAsync` (SignAndEncrypt),
`AnAccessTokenOnAnUnencryptedChannelIsRejectedAsync`,
`ARequestWithoutAnIdentityIsRejectedAsync`,
`SubscriptionsStillNeedASessionAsync`;
- one sample per service set: `ViewServicesRunWithoutASessionAsync`
(Browse, TranslateBrowsePathsToNodeIds),
`AttributeServicesRunWithoutASessionAsync` (Write, HistoryRead),
`MethodServicesRunWithoutASessionAsync` (Call),
`NodeManagementServicesRunWithoutASessionAsync` (AddNodes). They check
that the service runs and reports its operation result, not a missing
session or an internal error, since #4632 hardened only
Browse/BrowseNext against a missing session. Query is not sampled:
`StandardServer` does not implement QueryFirst/QueryNext.
- REST binding,
`HttpsWebApiIntegrationTests.SessionlessReadOverRestRunsWithAnAccessTokenAsync`
(Compact and Verbose): `POST /read` with the Access Token as String
NodeId (`"AuthenticationToken": "s=<token>"`), and the same request
without a token.
- **Budget**, `Opc.Ua.Server.Tests/SessionlessRequestBudgetTests` (22
tests):
- the budget itself: server limit and release, channel limit that takes
nothing from the server, server limit checked first, zero does not
limit, a lease releases once, HTTPS grouped by peer address (IPv4-mapped
form too), 200 concurrent callers against the limits (exactly the limit
granted, none above the channel limit, parallel release leaves zero and
no tracked channel), 16 callers acquiring and releasing in a loop
(high-water mark never above the limit);
- through `SessionManager.ValidateRequestAsync`: `Bad_ServerTooBusy`
over the server and the channel limit, disposing a context twice
releases once, places returned after a rejected Access Token, an
unencrypted channel, a missing identity, a cancelled validation (while
it waits, the place is taken), a handler error and a handler exception;
the budget also limits requests a handler decides, and a handler-only
server has none; requests outside the service set and requests a
disabled server refuses take no place; changed limits apply to the next
request and new options start a new budget;
- options: finite defaults, negative values rejected.
- **Budget end to end**,
`Opc.Ua.Sessions.Tests/SessionlessServiceInvocationTests.SessionlessRequestsRunInABudgetOfTheirOwnAsync`
(opc.tcp, requests held in the Access Token validation): the third
request over a limit of two gets `Bad_ServerTooBusy`; after release,
five sequential reads and five reads of an unknown node (operation
result bad) all pass; a channel limit of one refuses the second request
on that channel while another channel's request is admitted.
`ARequestWithoutATokenIsUnsupportedWhileTheFeatureIsOffAsync`:
`Bad_ServiceUnsupported` for a request without token on a server without
the feature, `Bad_SessionIdInvalid` for a token that names no Session.
- **Fail without the change**: the places-returned tests fail when the
budget does not release on failure (4 of 22 fail with the release in
`SessionManager` removed; the other budget tests use API this change
adds). `SessionlessRequestIsUnsupportedUnlessEnabled` and
`TheServiceSetLimitAppliesBeforeAValidateSessionLessRequestHandler` are
unchanged from the first version.
- **Review follow-up (`e9a35980b`)**:
`SessionlessAccessTokenReceivesTheRolesMappedFromItsClaimsAsync`,
`SessionlessAccessTokenWithoutTheMappedClaimIsNotGrantedTheRoleAsync`,
`AnAnonymousSessionlessRequestIsGrantedTheAnonymousRoleOnlyAsync`,
`TheIdentityOfAValidateSessionLessRequestHandlerReceivesTheMappedRolesAsync`,
`AValidateSessionLessRequestHandlerThatSetsNoIdentityKeepsNoIdentityAsync`;
end to end
`ARoleMappedNodeIsReadWithTheRolesMappedFromTheAccessTokenAsync` (the
ServerDiagnosticsSummary, readable for SecurityAdmin only, is denied for
a token whose claim is not mapped, readable once the rule maps it,
denied for another token) and
`ACancelledAccessTokenValidationAnswersTheTimeoutAndReturnsItsPlaceAsync`.
The anonymous and no-identity tests guard unchanged behavior. With the
`SessionManager.cs` change removed the three claim-mapping tests, the
end-to-end role test (`Bad_UserAccessDenied`), the end-to-end
cancellation test (`Bad_UnexpectedError` instead of `Bad_Timeout`) and
`ACancelledValidationReturnsItsPlaceAsync` fail. Lines of this commit
were not re-measured for patch coverage; the figure below is for
`496f84fce`.
- **`ClientTest`**: the three assertions that accepted
`Bad_ServiceUnsupported` too are back to master (`Bad_SessionIdInvalid`
only), so the PR no longer touches that file. Since #4646 a closed
`Session` refuses a request without authenticationToken itself; the
server code is not reached. The three tests pass with the single code
(12 cases on net10.0).
- Results on macOS: `Opc.Ua.Server.Tests` full run on net10.0 8076
passed / 9 skipped / 1 failed
(`ACancelledValidationReturnsItsPlaceAsync`, which asserted the old
`Bad_UnexpectedError` mapping; updated to the propagated cancellation
and passing); the session-less, SessionManager and role filters 368
passed / 0 failed on net10.0 and on net8.0, and the session-less
invocation and budget tests 73 passed on net10.0 after the last test was
added. `Opc.Ua.Sessions.Tests` with the
HTTPS/WebApi/WSS/OpenApi/Sessionless/ClientTest filter: 400 passed / 241
skipped / 0 failed on net10.0 and on net8.0 (the earlier host-name
failures were an environment issue of the test machine and are gone).
- Builds: `Opc.Ua.Server`, `Opc.Ua.Server.Tests` and
`Opc.Ua.Sessions.Tests` for net48, net8.0, net9.0 and net10.0 and
`Opc.Ua.Server` in Release net10.0: 0 errors, no new warnings (the
CA2213 in `SamplingGroup.cs` and the CA1861/CA2025 in other test files
are on master too).
- `dotnet format` (whitespace, style, analyzers `--severity info`, one
`--include` per changed file): no finding on a changed line. What
remains is on master lines (`SessionManager.cs` 552, 1374, 2205 and
`DependencyInjectionStandardServer.cs` 228) and the line-ending finding
of a macOS checkout.
- Coverage (`.azurepipelines/check-coverage.ps1` against master, only
the session-less tests of `Opc.Ua.Server.Tests` on net10.0): patch
105/116 changed lines (90.5 %). The 11 uncovered lines are Session-bound
lines of `ValidateRequestAsync` and the lockout logger that moved with
the change; the rest of the suite covers them. The project floor is not
meaningful for this subset.
- Fork CI and CodeQL: previous head `0fba9f21d` (master incl. #4674
merged): [CI run
37307824769](https://github.com/biancode/UA-.NETStandard/actions/runs/37307824769)
green (one net48 timing test, `KeepAliveHandlerV2Async`, failed in the
first attempt and passed on the job rerun; the PR does not touch
subscriptions), [CodeQL run
37307829138](https://github.com/biancode/UA-.NETStandard/actions/runs/37307829138)
green. Current head `e9a35980b`: [CI run
37364178979](https://github.com/biancode/UA-.NETStandard/actions/runs/37364178979),
[CodeQL run
37364182697](https://github.com/biancode/UA-.NETStandard/actions/runs/37364182697)
(results pending).
## Design decisions
Marc's answers (2026-10-05) and what the PR does:
1. **Built in** when `AddSessionlessInvocation` was used. Done.
2. **Service-set limit for all paths**: it applies before a
`ValidateSessionLessRequest` handler too. Done, unchanged.
3. **`Bad_ServiceUnsupported`** for a request without a token on a
server without the feature. Done, unchanged. A token that names no
Session stays `Bad_SessionIdInvalid`.
4. **Identity model**: an Access Token in the RequestHeader, validated
as an issued JWT token through the identity registry, anonymous only by
opt-in. Unchanged. The REST authentication is #4674, now in master.
5. **Separate budget** for session-less requests: added, see "Budget for
session-less requests". Open: no limit per identity yet (identities are
claims-defined, so the keys would be unbounded); say if you want one.
## Related Issues
- Fixes #4658
- Relates to #4657 (the REST binding's session-less service set), #4646
(a closed `Session` no longer sends requests without token), #4541
(resource isolation) and #4674 (REST authentication)
## Checklist
- [x] 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.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. (Affected
suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered
by the fork CI's Windows net48 legs.)
- [ ] 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: Marc Schier <marcschier@users.noreply.github.com>
|
||
|
|
7212607f16 |
OPC UA Part 3 compliance: permissions, Write semantics, NodeManagement, NodeSet DataTypes, locales (#4675)
# Description Fixes the High and Medium findings of an OPC UA Part 3 (OPC 10000-3, Address Space Model) compliance audit. Each finding was checked against the full spec text (Parts 3, 4, 5, 6, 14 and 18) before it was fixed. Every fix comes with regression tests that fail without it. ### Role permissions (Part 3 §8.55, Part 4 §5.12.2.2) - The type-hierarchy exemption in `MasterNodeManager.ValidateRolePermissions` / `ValidateAccessRestrictions` now covers only Browse, Read and ReadRolePermissions. Type nodes stay visible to generic clients, which was the intent of #3705; every other permission bit is now enforced on them. - `ReceiveEvents` is enforced on the EventType node. The standard NodeSet grants it on `AuditEventType` and its subtypes only to SecurityAdmin, so only SecurityAdmin sessions now receive audit events. - Call checks the `Call` permission and AccessRestrictions on the Object as well as on the Method. - When a Call's `methodId` is the Method declaration of the ObjectType, the Object's Method with the same BrowseName is permission-checked and invoked. The declaration is used only if the Object has no such Method. This applies to `AsyncCustomNodeManager`, `CustomNodeManager2` and the sample node manager. ### Write service (Part 3 §5.6.2, §8.40, §8.57; Part 4 §5.11.4) - The synchronous `OnWriteValue` path (fluent `OnWrite`) checks DataType and ValueRank first (`Bad_TypeMismatch`). After an IndexRange write it merges the slice into the cached value instead of replacing it. - StatusWrite / TimestampWrite are enforced (`Bad_WriteNotSupported`). - Undefined enumeration values and invalid OptionSet writes return `Bad_OutOfRange`. OptionSet writes merge only the valid bits into the stored value. - New `ReportPropertyValueChanged` / `ReportSemanticChange` let server code raise SemanticChangeEvents for changes it makes itself. - Required follow-up (Part 14 §6.2.11.1): if an external target rejects a timestamp, the PubSub subscriber's TargetVariables writer retries without it. ### NodeManagement and Call arguments (Part 3 §5.5.2, §5.7.3, §6.4.2; Part 4 §5.8, §5.12.2) - AddNodes creates the Mandatory InstanceDeclarations of the TypeDefinition and rejects abstract or Interface types (`Bad_TypeDefinitionInvalid`). - AddReferences, DeleteReferences and DeleteNodes bump NodeVersion and raise GeneralModelChangeEvents. - Clients may omit trailing optional input arguments (`HasOptionalInputArgumentDescription`). ### NodeSet DataTypes (Part 3 §5.8.3, §8.48; Part 6 Annex F.12) - `UANodeSet.Import` completes structure definitions with the base type's fields (base fields first). It creates definitions for structures without fields and sets `DefaultEncodingId`. `StructureDefinition.FirstExplicitFieldIndex` marks the subtype's own fields. - `DataTypeState` no longer mutates the stored definition on read. ### Locales (Part 3 §5.2.5, §8.5; Part 4 §5.4) - Read, Browse and data change notifications return the translation that best matches the session's LocaleIds, or all translations for `mul`. The selection lives in the server dispatcher and is built on `ResourceManager`: the new `TranslateValue` calls the overridable `Translate`. - Locale ids are matched case-insensitively, and fallback translations carry their real locale. ### Client complex types (Part 6 §5.1.7) - The Reflection.Emit builder encodes only `IsOptional` subtyped fields as ExtensionObject. It also supports OptionSet subtypes: an unsupported type is skipped instead of aborting the whole load. - `TypeInfo.GetXmlName` uses the data type's XML name for dynamic structures and OptionSets (the XML body element was `<Structure>`). Both builders now produce identical XML. ## Behaviour changes All are documented in `docs/MigrationGuide.md`. - **Audit events:** only SecurityAdmin receives them. The CTT "Auditing Connections" unit needs a SecurityAdmin user (e.g. sysadmin); `AuditingOperationTests` was updated. - **Write timestamps and status:** clients writing a SourceTimestamp or a non-Good StatusCode get `Bad_WriteNotSupported` unless the Variable has StatusWrite/TimestampWrite. No variable in `src/` sets these bits. - **AddNodes:** it rejects abstract types (including `BaseVariableType` and `BaseEventType`), and new instances now come with child nodes. - **Call:** the Object needs the Call permission. The Object's own Method runs even when the call names the type declaration. ## Not included - Low-severity findings: missing validation on optional services and author-side NodeSet rules. - LocalizedText nested inside structures or Variant arrays, and in HistoryRead or Call outputs. ## Testing net10.0, Debug, one suite at a time on the merged branch. All suites below pass: | Suite | Passed | |---|---| | Types | 11430 | | Core.Encoders | 2947 | | Core.Schema | 179 | | Core.Security | 558 | | Client | 2761 | | Server | 8210 | | InformationModel | 1080 | | Features | 145 | | History | 536 | | Di | 455 | | Machinery | 167 | | Robotics | 661 | | Robotics.Intent | 22 | | Pumps | 75 | | Scales | 122 | | Positioning | 69 | | ISA95 | 144 | | Vision | 484 | | PubSub | 1490 | | PubSub.Adapter | 151 | | PubSub.Server | 177 | | Redundancy.Server | 856 | | Lds | 173 | | Configuration | 241 | | WotCon | 1496 | - **Client.ComplexTypes:** one test wrote a SourceTimestamp to a variable without TimestampWrite. It is fixed in this PR, and the suite has not been re-run since. - **Core:** 6068 passed, 1 failure in `ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly`. That telemetry test passed when re-run alone, so it looks flaky. - **Still running:** Sessions, Subscriptions (Classic, Durable, standard) and Interop. Results will follow on this PR. - `UA.slnx` builds with 0 errors for net10.0 and net48, with no new analyzer warnings in changed files. ## Related Issues - Part 3 compliance audit (no tracking issue) ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
8bdc13ce27 |
WebApi: enforce AddWebApi*Auth on the HTTPS listener's own host (REST routes were served unauthenticated) (#4674)
# Description `AddWebApiBasicAuth` / `AddWebApiBearerAuth` / `AddWebApiMutualTlsAuth` registered their ASP.NET Core authentication schemes only on the **application** `IServiceCollection`. `HttpsTransportListener` serves the REST routes from its **own** Kestrel host with a separate DI container (own-host and shared-host paths), into which only `IHttpsListenerServiceContributor.ConfigureServices` registrations flow. `WebApiHttpsStartupContributor.Configure` checked for schemes in the listener container, found none, and therefore skipped `UseAuthentication()` / `UseAuthorization()` and `RequireAuthorization()`. Result: on a real listener the REST routes (`/read`, `/write`, `/call`, sessions, subscriptions, …) were served **without** the configured HTTP credential. The existing unit tests only checked the application container, so they passed. Present since the WebApi binding was introduced (#3880). ## Changes - **Record + replay.** Each `AddWebApi*Auth` opt-in still registers its scheme on the application container (so hosting `MapWebApiEndpoints()` in your own ASP.NET Core app keeps working), and also records it as an internal `WebApiListenerAuthRegistration`. `AddWebApiTransport()` now hands the application `IServiceProvider` to `WebApiHttpsStartupContributor`, whose `ConfigureServices` replays the recorded schemes, the policy scheme and the application's `ISessionlessIdentityProvider` into the listener container. Auth middleware and `RequireAuthorization()` are now installed on the pipeline that actually serves the routes. - Side effects fixed by the same change: `UseJwtClaimIdentityProvider()` / Bearer claims mapping now reaches the dispatcher, and the WSS `opcua+openapi+<accesstoken>` upgrade finds the Bearer scheme instead of always failing closed. - **No-credentials fallback scheme.** Previously the policy selector returned `null` for a request without a credential; ASP.NET Core's `PolicySchemeHandler` then throws on challenge (→ 500). The selector now forwards to a new internal `WebApiNoCredentialsAuthenticationHandler` that authenticates nobody and answers the challenge with **401** plus the `WWW-Authenticate` challenges of the registered Basic / Bearer schemes. - **Selector only picks registered schemes.** A `Bearer` header when only Basic is registered (or vice versa) used to forward to a non-existent scheme (→ 500); it now falls through to the no-credentials scheme (→ 401). ## Reviewer notes - Only options configured through the opt-in callbacks are replayed. Options added separately via `services.Configure<JwtBearerOptions>(scheme, …)` on the application container do not reach the listener container. - mTLS-only: a request without a client certificate gets 401 without a `WWW-Authenticate` header (there is no HTTP challenge for client certificates). - Discovery routes (`/findservers`, `/getendpoints`) keep their `AllowAnonymous` metadata. ## Tests New `RealHttpsListenerIntegrationTests.Authentication.cs` builds the services exactly as an operator would (`AddOpcUa().AddWebApiTransport().AddWebApi*Auth(...)`) and posts `/read` through a real `HttpsTransportListener` + Kestrel TLS endpoint: | Auth | Credential | Expected | |---|---|---| | Basic | none / wrong password / Bearer token | 401 + `WWW-Authenticate: Basic`, not reaching the server | | Basic | valid | 200 | | Bearer | none / `not.a.jwt` / JWT with wrong key / Basic | 401 + `WWW-Authenticate: Bearer`, not reaching the server | | Bearer | valid JWT | 200 | | mTLS | none / Basic | 401, not reaching the server | | Basic | `/findservers` without credential | 200 | With the replay disabled, all 9 negative cases fail with 200 (reproducing the bug). Locally on net10.0: `Opc.Ua.Bindings.Https.WebApi.Tests` 450/450, WebApi/OpenApi tests in `Opc.Ua.Sessions.Tests` 17/17; `Opc.Ua.Bindings.Https` builds for all target frameworks with no warnings. ## Related Issues - No tracking issue; reported externally. ## Checklist - [ ] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
b03575b4e2 |
Run the interop tests against six foreign OPC UA stacks and extend feature coverage (#4644)
## Proposed changes Runs the interop fixtures added in #4630 against peers built on six other open source OPC UA stacks, so the 2.0 client and server are tested against independent implementations, not only the released 1.5.378 stack. It also extends the fixtures beyond read/write/browse to events, subscriptions, identity and more services. ### Peers (`tests/Opc.Ua.Interop.Peers`) | Peer | Stack | Toolchain | Roles | | --- | --- | --- | --- | | `node-opcua` | node-opcua 2.186.15 (MIT) | Node.js 24 | server, client | | `milo` | Eclipse Milo 1.1.7 (EPL-2.0) | Java 17+, Maven | server, client | | `asyncua` | opcua-asyncio 2.0.1 (LGPL-3.0) | Python 3.12 | server, client | | `open62541` | open62541 1.5.8 (MPL-2.0), OpenSSL backend | C, CMake | server, client, ECC | | `async-opcua` | async-opcua 0.19.0 (MPL-2.0) | Rust | server, client | | `gopcua` | gopcua 0.9.1 (MIT) | Go | client only | Every peer speaks the command line and stdout contract of `Opc.Ua.Interop.LegacyPeer` (`server` / `client --checks`, `PEER-INFO {json}`, `RESULT {json}`). The stacks are fetched at build time and run as separate processes; nothing is linked into or shipped with any package. ### Harness - **Peer selection:** `OPCUA_INTEROP_PEER_HOST` + `OPCUA_INTEROP_PEER_HOST_ARGUMENTS` choose the host (`node`, `java -jar`, `python`) for the peer named by `OPCUA_INTEROP_LEGACY_PEER`. Native peers (open62541, async-opcua, gopcua) run without a host. Unset, the 1.5.378 peer runs exactly as before, so the main CI is unchanged. - **`PEER-INFO {json}`:** stack, version, application URI, software version and, optionally, the security policies the peer implements. `LegacyServerInteropTests` compares the ApplicationUri and SoftwareVersion against it. `EccInteropTests` runs policies beyond the four 1.05 ECC policies only against a peer that declares them. - **`expected-differences.json` per peer:** confirmed defects of a stack, keyed by check name, `Fixture.Test` or a `*` prefix, each with the source file or spec section that shows it. A listed failure is reported as inconclusive with its reason; a listed test that passes produces a warning. The 2.0 client tests apply it through the fixture-level `[PeerDifferences]` action. ### New coverage (all peers, including the 1.5.378 peer) - **16 client checks**, run by each peer's client against the 2.0 server (`LegacyClientFeatureCheckAsync`, Basic256Sha256 SignAndEncrypt): EventSubscription, ConditionRefresh, AlarmAcknowledge, DeadbandFilter, QueueOverflow, Triggering, Republish, TransferSubscription, WrongPasswordRejected, X509UserToken, RegisterNodes, HistoryReadRaw, NodeManagement, IndexRange, FindServers, SessionReconnect. - **`ServerFeatureInteropTests`**: the 2.0 client against each peer server. It covers RaiseEvent, ConditionRefresh, absolute deadband, queue overflow bit, triggering, republish errors, transfer to a new session, X509 user token, RegisterNodes, IndexRange, AddNodes/DeleteNodes, FindServers and session reconnect. Every peer server gained a `RaiseEvent` method and accepts X509 user tokens. - **ECC:** with open62541's OpenSSL backend, `EccInteropTests` adds `ECC_nistP256_AesGcm` and `ECC_nistP256_ChaChaPoly` (both directions; SignAndEncrypt anonymous and username, Sign, and token renewal across two renewals). It also adds `ECC_curve25519`/`ECC_curve448` cases, which are ignored while 2.0 marks those policies unsupported. ### CI and docs - **Workflow `interop-foreign.yml`:** opt-in (nightly, `workflow_dispatch`, PRs labelled `interop`), not a required check. It builds all six peers on Windows, Linux and macOS (open62541 from source on Linux only, against the distro OpenSSL), with dependency caching. - **`tests/Opc.Ua.Interop.Peers/README.md`:** how to install the toolchains, build each peer (including Windows without Visual Studio), run the fixtures against it, maintain expected differences and add a new stack. `DeveloperGuide.md` links to it. ## Results Run locally, sequentially, net10.0, Windows. Not passed = expected differences, inconclusive by design (e.g. AddNodes on a server without NodeManagement), or ECC cases a peer or the platform does not support. | Peer | Fixtures | Total | Passed | Failed | | --- | --- | --- | --- | --- | | 1.5.378 | whole interop project | 220 | 218 | 0 | | node-opcua | server + client + feature | 145 | 143 | 0 | | Eclipse Milo | server + client + feature | 145 | 141 | 0 | | asyncua | server + client + feature | 145 | 133 | 0 | | open62541 (OpenSSL) | server + client + feature + ECC | 199 | 175 | 0 | | async-opcua | server + client + feature | 145 | 137 | 0 | | gopcua | client | 104 | 81 | 0 | After the AEAD cases were added, `EccInteropTests` against the 1.5.378 peer ran separately: 54 total, 26 passed, 0 failed. The new cases are ignored there because the peer does not declare them. ## Findings No 2.0 defects beyond the two now fixed in #4654: - Browse with a null ViewId and a Timestamp now returns `Bad_ViewTimestampInvalid`. - Subscription transfer no longer fails when the optional `Server.GetMonitoredItems` is unavailable. After #4654 merges, `TransferSubscriptionToNewSessionAsync` can drop its expected differences for asyncua. Upstream defects found, all listed in the peers' `expected-differences.json` with evidence; issues are still to be filed: - **All five non-.NET stacks:** cannot handle the TestData `ScalarStructureDataType` (fields of the abstract Enumeration/Structure/BaseDataType). Milo encodes the abstract `Enumeration` field as a Variant (Part 6 5.2.4 says Int32). - **open62541:** - **Sign-only ECC keys:** derives encryption key and IV into the HKDF length for Sign-only ECC (Part 6 6.8.1), so Sign mode fails for the four 1.05 curves. The AEAD policies are not affected. - **HMAC mismatch:** reported as `Bad_InternalError`. - **ConditionRefresh:** needs the experimental A&C option. - **Response too large:** closes the connection instead of answering `Bad_ResponseTooLarge`. - **Array limit:** no MaxArrayLength. - **gopcua:** - **RsaPss:** `RSAOAEPMinPaddingSHA256` is 130 instead of 66, which breaks `Aes256_Sha256_RsaPss`. - **Nil LocalizedText:** encoding a nil `*LocalizedText` panics. - **asyncua server:** - **Queue overflow:** no Overflow bit. - **SetTriggering:** not implemented. - **Republish:** answers Good for an unknown sequence number or subscription. - **GetMonitoredItems:** not implemented. - **IndexRange:** ignored on Read and Write. - **Reconnect:** closes a session without subscriptions on transport loss. - **Limits:** no BrowseNext; operation limits not enforced; no `Bad_ResponseTooLarge`. - **async-opcua:** - **X509 user token (client):** treats an empty token SecurityPolicyUri as None. - **Reconnect (client):** overwrites the session nonce with the channel nonce on reconnect. - **ConditionRefresh (server):** not implemented. - **Overflow bit (server):** set without the DataValue InfoType bit. - **ECC:** no ECC policies. - **Milo:** - **Wrong password:** `BadIdentityTokenInvalid` instead of `BadUserAccessDenied` / `BadIdentityTokenRejected`. - **Array limit:** accepts arrays above its advertised MaxArrayLength. - **AddNodes:** answers `BadNotSupported`. ## Related Issues - Follows #4630. Related fixes: #4654. ## Types of changes - [x] Test / CI only ## Checklist - [x] Interop fixtures pass locally against all seven peers (net10.0, Windows). - [x] `dotnet format whitespace --verify-no-changes` on the changed C# files. - [ ] `interop-foreign.yml` has not run on GitHub yet; it needs a manual dispatch or the `interop` label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
11ad24e73e |
Kubernetes readiness tests: give each test a port of its own (fixes the Windows "conflicts with an existing registration" flake) (#4673)
# Description `KubernetesReadinessServerHttpTests.StartIsIdempotentAndDisposeStopsListenerAsync` failed on the Windows CI legs with `HttpListenerException: Failed to listen on prefix 'http://localhost:57023/readyz/' because it conflicts with an existing registration on the machine` (error 183, `ERROR_ALREADY_EXISTS`). `HttpListener` raises that error only when http.sys already holds the exact URL prefix, and the only code that registers `http://localhost:<port>/readyz/` in the test process is the readiness tests themselves; three sibling tests of the `ParallelScope.All` fixture were running when it failed. The tests took their port from a `TcpListener` probe that is stopped again before the readiness server registers its prefixes, so tests that run in parallel can draw the same port. On Windows the probe cannot rule this out either, because an http.sys registration is an entry in the URL namespace of the kernel driver and not a socket bound to the probed loopback address. The fix is test-only: - A shared `ReadinessTestServer` helper hands out loopback ports that are never reused within the test run. Every probe stays open until a port is found that was not handed out before, so the OS cannot give the same port back on the next probe. The readiness tests can therefore no longer collide with one another, whatever the scheduling or the OS port allocation. - A port that another process takes between the probe and the start is handled by retrying the start on a fresh port, at most five times. Only `HttpListenerException` from the start is retried; anything else is rethrown at once, and a failed server is disposed before the next attempt. The production `KubernetesReadinessServer` needs no change: `Start` is idempotent under its lock (the failure was on the first call), `HttpListener.Start` releases its registrations when adding a prefix fails, and `DisposeAsync` removes them synchronously through `HttpListener.Close`. ## Changes - `tests/Opc.Ua.Redundancy.Kubernetes.Tests/Health/ReadinessTestServer.cs` (new): `GetUniqueFreePort()` and `StartAsync(createServer, startServer)`, which returns the started server together with its port. - `tests/Opc.Ua.Redundancy.Kubernetes.Tests/Health/KubernetesReadinessServerHttpTests.cs`: the HTTP tests start their servers through the helper; the local `GetFreePort()` is gone. - `tests/Opc.Ua.Redundancy.Kubernetes.Tests/Health/KubernetesReadinessStartupTaskTests.cs`: `OnServerStartedStartsReadinessServerAsync` starts the server through the startup task inside the helper; the local `GetFreePort()` is gone. ## Tests - **New** `StartFailsWhenAnotherServerRegisteredTheSamePrefixAsync`: a second server on the port of a running one fails to start with `HttpListenerException`, and the first server keeps answering `/readyz`. This is the collision behind the flake; on macOS it produces exactly the message of the Windows CI failure (checked locally with a temporary assertion on the message). - **New** `StartMovesToAFreshPortWhenThePrefixIsAlreadyRegisteredAsync`: the first attempt of the helper is pointed at an occupied port; the helper moves to a fresh port on the second attempt and the server answers there. - **New** `UniqueFreePortsAreNeverHandedOutTwice`: 32 ports drawn concurrently are distinct. - `Opc.Ua.Redundancy.Kubernetes.Tests` passed 103/103 on net10.0 and on net8.0 (macOS), and 20 consecutive runs per framework (40 runs) all passed. - `Opc.Ua.Redundancy.Kubernetes` builds for net8.0, net9.0 and net10.0 with 0 warnings; the test project builds for net8.0 and net10.0 with 0 warnings, and as the empty legacy shell with `CustomTestTarget=net48`. - `dotnet format` (whitespace, style, analyzers at info severity) on the changed files: no findings (whitespace checked with the files converted to CRLF, as `.editorconfig` asks; an LF checkout reports `ENDOFLINE` for every line of every file). - The Windows behaviour (http.sys error 183 and its URL-namespace semantics) is reasoned from the `System.Net.HttpListener` sources, not reproduced locally; the fork CI covers the Windows legs. - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37203410921](https://github.com/biancode/UA-.NETStandard/actions/runs/37203410921) and [CodeQL run 37203412518](https://github.com/biancode/UA-.NETStandard/actions/runs/37203412518), both green, including all Windows legs. ## Notes for review - The retry only covers the probe-to-start window for ports taken by other processes; collisions between the tests themselves are ruled out by the never-reused ports, not by retrying. The `HttpListenerFactory` of dotnet/runtime's own `System.Net.HttpListener` tests takes the same approach (ports never reused within the process, plus a retry on `HttpListenerException`), and `ServerFixtureUtils.IsPortUnavailable` in this repository retries server starts on a taken port as well. - `DisposingAnUnstartedServerDoesNotBindItsPort` keeps its own occupant socket on the wildcard address; that server is never started, so it does not register a prefix. ## Related Issues - Fixes #4671 - Follows up #4613. ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suite on net10.0 and net8.0 locally on macOS; the Kubernetes tests do not target .NET Framework.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
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> |
||
|
|
b25c1f6ad4 |
Asset Management Basics: the OPC 10000-110 companion SDK (#4662)
# Description Stacked on #4650. Until that is merged, this PR also shows its commits; the six commits of this PR start after `fix(server): keep conformance unit aggregation idempotent and ordered with registrations`. This adds OPC 10000-110 Asset Management Basics 1.01.1 (`http://opcfoundation.org/UA/AMB/`) as a companion SDK, in the pattern of the DI, Machinery, Pumps and Scales packages. The tracking issue #4649 describes the scope, the design and the open questions. In short: | Package | Content | |---|---| | `Opc.Ua.AMB` | the model generated by the source generator from the unmodified NodeSet, plus the contracts server and client share (`AssetFaultSeverity`, `AssetLocationKind`, `MaintenanceStateKind`, `DocumentationLinkProperties`) | | `Opc.Ua.AMB.Server` | a sidecar node manager next to the owner of the Device Integration address space (`AddOpcUaDi`, `AddMachinery`, `AddPumps` or `AddScales`), the asset registry `IAssetManagement` with a fluent builder, `AddAssetManagement()` hosting, memory and file system stores, the AMB conformance units and server facet | | `Opc.Ua.AMB.Client` | `AmbClient`: discovery, identification, health alarm and maintenance streams, documentation links, locations, structure and snapshots, plus `AddAssetManagementClient()` | **Sidecar.** An asset stays in the node manager that owns it. Everything AMB adds to it (`DeviceHealth`, health alarms, maintenance conditions, the DocumentationLinks AddIn, AssetId and the other properties) is created there. The AMB node manager serves the AMB model, the alias categories `Assets`, `AssetsByProductInstanceUri` and `AssetsByAssetId` below `0:Aliases`, the location hierarchies below `0:Locations` and the server-specific alarm and condition types. Owners register assets with the shared registry from their setup, in either startup order. **Implemented conformance units** (Table 54): Asset Identification, Configurable Asset Identification, Discovery by ProductInstanceUri and by AssetId, Health Status Base, Alarms, Root Causes and Alarm Categories, Version Information, Operation Counters (inspected only; AMB defines no counters), DocumentationLinks Base, Edit Base and Edit Advanced, Requirements, Capabilities, Classification, Current and Future Maintenance Activities, Local Time, the three location properties and both location object hierarchies, Sub-assets and Asset relations. The two history units (Health Tracking, Past Maintenance Activities) are not part of this PR. `docs/AMB.md` has the matrix with the rule the server applies for each unit and whether a test checks it over a session. **Dependency on #4650.** The health alarms and maintenance conditions are created through `CreateAlarm<TState>`, and the conformance units follow the registered assets through the recomputation in `ConformanceUnitsManager`. ## Changes Six commits, each building on its own: 1. **`feat(amb)`: model package** (`src/Opc.Ua.AMB/`, about 2.4k lines, of which the NodeSet and the two identifier tables are 1.6k). - The NodeSet is the publication from `UA-Nodeset/latest/AMB`, byte for byte (git blob `21f9475b646e9c5508171277ffe5b74edbeb0a58`). `.gitattributes` checks it out with LF endings, because a test pins its SHA-256. - The published identifier table is vendored unchanged; a derived table pins the symbol-to-NodeId mapping, and the generator fails the build when they disagree. `tools/nodesets/README.md` documents the one new kind of renamed symbol (top-level instances below `0:Aliases` and `0:Locations`). 2. **`feat(amb)`: server package** (`src/Opc.Ua.AMB.Server/`, about 10.7k lines). Node manager, registry and builders, discovery, health, maintenance (`MaintenanceEventStateMachine` with all transitions), documentation links, locations, version information, classification (IRDI dictionary entries below `Server/Dictionaries`) and structure, stores, conformance evaluation, hosting. Log messages use the EventId block 8300 to 8379, after Pumps (8100) and Scales (8200). 3. **`feat(amb)`: client package** (`src/Opc.Ua.AMB.Client/`, about 3.0k lines). Calls AddLink and RemoveLink through the generated `DocumentationLinksTypeClient`. 4. **`test(amb)`: test suite** (`tests/Opc.Ua.AMB.Tests/`, 163 tests, about 8.8k lines; commit 6 adds one more). 5. **`docs(amb)`: guide and samples**: `docs/AMB.md`, links from `README.md`, `docs/README.md`, `docs/Samples.md` and `docs/AliasNames.md`; `samples/AMB/AmbServer` (AMB next to `AddMachinery`: two DI devices and a Machinery press as assets, with a simulation) and `samples/AMB/AmbClient`. The sample server listens on 62553, which no other sample uses. 6. **`feat(amb)`: tell the links users added from the links of the manufacturer.** OPC 10000-110 gives a client no way to see which links `RemoveLink` accepts (§10.5.4: only those `AddLink` created). As §10.5.1 lets vendors add metadata Properties to the links, the server gives every link `AddLink` created, and every such link restored after a restart, the `Boolean` Property `UserLink` (`true`), qualified with its namespace of server-specific types, with a NodeId derived from the persisted link identifier; declared links carry none, and `RemoveLink` deletes it with the link. `DocumentationLinkRecord` gets the additive `IsUserLink`, which `ReadDocumentationLinksAsync` fills with one Browse over all links and one Read of the markers found (comparing the name only, as the namespace is configurable; `false` for a server that does not mark links). The name is shared as `DocumentationLinkProperties.UserLink`. `UA.slnx` and `.azurepipelines/expected-packages.txt` get the new projects and packages in the commit that adds them. All projects target `net48;net8.0;net9.0;net10.0` through the repository properties (`LibTargetFrameworks`, `TestsTargetFrameworks`, `AppTargetFrameworks`); `IsAotCompatible` and `PublishAot` are set from net10.0 on, as in the other companion projects. ## Tests - **`Opc.Ua.AMB.Tests`: 164 tests**, most of them against a hosted server over real client sessions: - the model: the SHA-256 of the NodeSet and the generator behaviours AMB is the first companion model to rely on; - each building block in-process and over a session: discovery through every category with NodeVersion and model change events, AssetId writes, health alarms and maintenance activities through all transitions with acknowledgement, AddLink and RemoveLink, locations and structure, the advertised conformance units and the facet; - a restart with the file system stores (links and AssetId survive); - the `UserLink` marker: present on an added link only, the same NodeId after a restart, removed with `RemoveLink`, and `IsUserLink` on the client in the snapshot, after `AddLink`, after a restart and after `RemoveLink`; - composition with `AddOpcUaDi` (both startup orders), `AddPumps` and `AddMachinery`; - `AmbClient` end to end against a hosted server, including a Machinery machine as an asset. - **Results (macOS, local):** 164 passed / 0 failed on net10.0 and on net8.0. .NET Framework is covered by the fork CI's Windows legs. - **Coverage** of the three AMB assemblies (coverlet with `tests/coverlet.runsettings.xml`, net10.0, generated `*.g.cs`, `*TypeClient` proxies and `*Log` classes excluded): lines 93.9 % (4890 of 5206), branches 84.0 % (1294 of 1539); model 100 %, client 94.5 %, server 93.4 %. - **Build:** the three packages, the tests and the two samples build without warnings for net48, net8.0, net9.0 and net10.0; `UA.slnx` builds in Release (its warnings are all in files this PR does not touch, as on master). `dotnet format` (whitespace, style, analyzers with `--severity info`) reports nothing on the 95 changed C# files. - **Samples:** `AmbServer` and `AmbClient` ran against each other on net10.0: the client discovered the three assets, printed their snapshots and the location hierarchies, added and removed a documentation link and streamed alarms and maintenance. - Fork CI (full matrix incl. macOS) and CodeQL: green on the same five package commits before the rebase onto the current #4650 head — [CI run 37144486107](https://github.com/biancode/UA-.NETStandard/actions/runs/37144486107), [CodeQL run 37144487873](https://github.com/biancode/UA-.NETStandard/actions/runs/37144487873). The run on the current head (rebased, plus the user-link commit; AMB tests 164/164 locally on net10.0) is in progress: [CI run 37203517457](https://github.com/biancode/UA-.NETStandard/actions/runs/37203517457). ## Notes for review - **Size.** About 26.8k lines, larger than each of the companion PRs #4577 to #4580. Without changes to the code it splits into the model, server and server tests, and the client, end-to-end tests, samples and docs on top; the issue asks which you prefer. - **Interfaces on types.** §9.4.1 and §12.1 ask for alarm and condition types that implement `IRootCauseIndicationType` and `IMaintenanceEventType`. The server creates subtypes of the DI alarm types in its own type namespace (`urn:opcua-netstandard:amb:types`); `UseServerDefinedAlarmTypes = false` uses `HasInterface` on the instance instead, and then does not report the units that need the type-level interfaces. - **Conformance unit names** follow Tables 54 to 56; the profile database has no AMB entries yet. The QualifiedNames are in namespace 0, as DI and Machinery publish theirs (question 5 in the issue). - **Generator gap.** Children a NodeSet gives to a top-level instance (here the `NodeVersion` properties of the alias categories) get no identifier constants. The fix changes the identifiers of other generated models, the reference server's among them, so it is not part of this PR; `AmbModelTests.NodeVersionHasNoIdentifierConstant` records it and fails once the generator emits the constants. - **net48.** The code that the SDK so far only built for .NET 8 and later uses the `NET5_0_OR_GREATER` pattern of `JsonEncoder` for `Enum.IsDefined` and API forms that .NET Framework has (`string.Format`, `Guid.ToString(format, provider)`); the tests use a small async file helper instead of `File.WriteAllTextAsync`. - **User links.** The `UserLink` Property is metadata of this server, not an AMB name: commit 6 explains why and how a client uses it. Changed lines of that commit covered: 69/70 (the one line left is the loop continuation for a link that has other Properties but no marker). - **Known limitations** are listed in the guide: the `DeviceHealth` SourceTimestamp across a restart, removing optional maintenance members once set, `PatchIdentifiers`, locations only at the deepest level, and the history units. ## Related Issues - Part of #4649 - Stacked on #4650 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (The AMB suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
15f9a5ebcc |
Machinery client: enumerate each machine, component and process value once (#4672)
# Description `MachineryClient` browses the `Machines` folder, the `Components` add-in, the `MachineryBuildingBlocks` organizer, the energy resources and the process values with `HierarchicalReferences` and subtypes, and turned every returned `ReferenceDescription` into a `MachineEntry`. Browse returns one description per reference, not per node, and `HasEventSource` and its subtype `HasNotifier` are hierarchical references. A machine that the `Machines` folder organizes and that is also registered as a notifier of the folder - how a server exposes its event hierarchy below the `Server` object once alarms are attached - was therefore returned twice by `EnumerateMachinesAsync` and `DiscoverMachinesAsync`. The same held for a component that is an event source of its `Components` folder and for a process value that is an event source of `Monitoring`. The fix: - The shared browse helper reports each target NodeId once and keeps the first description. Its `BrowseName`, `DisplayName` and `TypeDefinition` belong to the node and are the same for every reference that leads to it. - All hierarchical references are still followed. Restricting the browse to `Organizes` would lose machines, components and process values a server references with another hierarchical reference type such as `HasComponent`, so an address space without duplicate references returns exactly what it returned before. - `EnumerateProcessValuesAsync` looks below the machinery item and below its `Monitoring`; a process value reachable from both is now reported once as well. The public API does not change. ## Changes - `src/Opc.Ua.Machinery.Client/MachineryClient.cs`: `BrowseChildrenAsync` skips a NodeId it has already returned; the remarks of `EnumerateMachinesAsync` say so. - `src/Opc.Ua.Machinery.Client/MachineryClient.ProcessValues.cs`: `EnumerateProcessValuesAsync` skips a process value below `Monitoring` it has already returned below the item. Every enumeration of the client goes through `BrowseChildrenAsync`: `EnumerateMachinesAsync`, `DiscoverMachinesAsync`, `EnumerateComponentsAsync`, `EnumerateBuildingBlocksAsync`, `EnumerateEnergyResourcesAsync`, `EnumerateMeteringPointsAsync`, `EnumerateSubMetersAsync`, `EnumerateProcessValuesAsync`, and the child walks of `ReadLifetimeCountersAsync`, `EnumerateEquipmentAsync`, `ReadMeteringPointAsync` and `ReadPublishedResultsAsync`. ## Tests - **New, end-to-end** (`MachineryEndToEndTests.NodesThatAreAlsoEventSourcesOfTheirParentAreEnumeratedOnceAsync`): the test press links the machine to `Machines` with `HasNotifier`, the `HydraulicUnit` component to `Components` and the oil-temperature process value to `Monitoring` with `HasEventSource`, each in addition to its organizing reference, and expects each node once from `EnumerateMachinesAsync`, `DiscoverMachinesAsync`, `EnumerateComponentsAsync` and `EnumerateProcessValuesAsync` over `opc.tcp`. - **New, unit** (`MachineryClientTests.ANodeReachedThroughTwoReferencesIsEnumeratedOnceAsync`): a browse result that carries the same machine under `Organizes` and `HasNotifier`. - **Fail without the fix:** with `src/Opc.Ua.Machinery.Client` from master, all four assertions of the end-to-end test fail - two machines, two NodeIds, two components and two process values instead of one each. - `Opc.Ua.Machinery.Tests` passed 167/167 on net10.0 and 167/167 on net8.0 (macOS). - `Opc.Ua.Machinery.Client` builds for net48, net8.0, net9.0 and net10.0 with 0 warnings. - `dotnet format` (whitespace, style, analyzers) on the changed files: nothing on the changed lines (the `ENDOFLINE` findings concern the whole of every file in a checkout with LF line endings). - Changed lines covered: 8/8 (100 %). - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37201766371](https://github.com/biancode/UA-.NETStandard/actions/runs/37201766371) and [CodeQL run 37201768280](https://github.com/biancode/UA-.NETStandard/actions/runs/37201768280), both green. ## Notes for review - No extra request: the de-duplication happens on the result of the browse the client already made. - Of the sibling companion clients, `PumpsClient.EnumeratePumpsAsync` and `ScalesClient.EnumerateScalesAsync` already de-duplicate across their two roots. `EnumeratePumpsUnderAsync`, `EnumerateScalesUnderAsync`, `EnumerateSystemScalesAsync` and the DI device discovery follow the same browse pattern without de-duplication; they are not changed here. ## Related Issues - Fixes #4670 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
dbee7b9988 |
Robotics client: fill the engineering units and ranges of the axis values and the load mass, and the gear ratio (#4661)
# Description Follow-up to #4619. It was prepared stacked on #4619 and is now one commit on top of master, with #4619's review changes in place. `AxisSnapshot.Engineering`, `LoadSnapshot.MassEngineering` and `GearSnapshot.GearRatio` are part of the snapshot contracts and documented in `docs/Robotics.md` (the URML capability manifest takes the joint limits from them), but `RoboticsClient` never set them: every axis came back without units or limits, every load without the unit of its mass and every gear without its ratio. OPC 40010-1 declares `ActualPosition` (mandatory), `ActualSpeed` and `ActualAcceleration` (optional) below the axis's DI `ParameterSet`, and the load's `Mass`, as `AnalogUnitType` variables with a mandatory `EngineeringUnits` and an optional `EURange` property, and `GearRatio` as a mandatory `RationalNumber` variable of the gear. The client now reads them: - **Axis:** `ReadAxisAsync`, and with it `ReadSystemAsync`, resolves the `EngineeringUnits` and `EURange` of the three axis values with one TranslateBrowsePathsToNodeIds and reads them with one Read per axis (none when nothing resolves) into `PositionUnit`, `SpeedUnit`, `AccelerationUnit` and `Limits.Position/Speed/Acceleration`. - **Load:** `Mass` is resolved and read together with its `EngineeringUnits` and `EURange` (`MassEngineering`), replacing the separate resolve and read. - **Gear:** `GearRatio` is read in the same request as `Pitch`. - Loads and gears cost no extra request. - A property the server does not publish, whose read fails or whose value has a bad status stays `null`. The structures go through the existing `ToStructure<T>`, which checks `StatusCode.IsGood` as for `MotorTemperatureEngineering`. The rest of the snapshot is still returned. - Because `Mass` and `Pitch` are now part of that request, a failed `Mass` or `Pitch` read keeps the server's status in the `DataValue`, like the axis values and `MotorTemperature`. Before, `Session.ReadValueAsync` threw and failed the whole snapshot read. The public API does not change. ## Changes - `src/Opc.Ua.Robotics.Client/RoboticsClient.Topology.cs`: - new `ReadAxisEngineeringAsync`, called from `ReadAxisAsync`; - `ReadLoadAsync` and `ReadGearAsync` use the existing `ReadChildrenAsync`. - `docs/Robotics.md`: - describes the members and their null cases; - adds them to the values that carry no NodeId; - names `AxisSnapshot.Engineering` instead of the non-existent `AxisSnapshot.Limits` in the URML section. ## Tests - **New, unit** (`Opc.Ua.Robotics.Tests`, `RoboticsClientFacadeTests`): - `EngineeringValuesAndTheGearRatioComeFromTheVariablesTheyDescribe`: the values are checked against same-named decoys below the axis and a `GearRatio` in namespace 0. - `AbsentOrUnreadableEngineeringValuesAndGearRatiosStayNull`: a missing `EURange`, a load without engineering properties, a failing unit read and a failing ratio read. - `BadQualityEngineeringValuesAndGearRatiosReadAsNoStructure`: a range, a unit and a ratio with a bad status read as `null`. - `AFailedMassOrPitchReadKeepsTheStatusAndTheSnapshot`. - `AnAxisWithoutAParameterSetHasNoEngineeringValues`. - **New, end-to-end** (`RoboticsClientServerE2eTests.EngineeringValuesAndGearRatiosAreReadAsync`): - the test cell now publishes a unit and range for `ActualSpeed`, a unit without a range for `ActualAcceleration`, a range for the flange load's mass and a tool load with a mass only; - the test reads the units, ranges and gear ratios back through `ReadSystemAsync` and `ReadAxisAsync`. - **Fail without the fix:** with `src/Opc.Ua.Robotics.Client` from master, five of the six new tests fail. Four get `null` units, ranges or ratios. `AFailedMassOrPitchReadKeepsTheStatusAndTheSnapshot` fails with `ServiceResultException` `Bad_NodeIdUnknown`. `AnAxisWithoutAParameterSetHasNoEngineeringValues` passes there by design: it pins that an axis without a `ParameterSet` still reads without error. - `Opc.Ua.Robotics.Tests` passed 661/661 on net10.0 and 601/601 on net8.0 (macOS). - `Opc.Ua.Robotics.Client` builds for net48, net8.0, net9.0 and net10.0 with 0 warnings. - `dotnet format` (whitespace, style, analyzers) on the changed files: nothing on the changed lines beyond the `IDE0002` info that the existing `RoboticsSessionHarness.Ua(Opc.Ua.BrowseNames.…)` calls in the same test class also get. - Changed lines covered: 52/52 (100 %). - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37147764118](https://github.com/biancode/UA-.NETStandard/actions/runs/37147764118) and [CodeQL run 37147766527](https://github.com/biancode/UA-.NETStandard/actions/runs/37147766527). CodeQL is green; CI is green except `Opc.Ua.Interop.Tests` (`LegacyClientExtendedCheckAsync("ComplexTypes")` / `LoadAndDecodeLegacyComplexTypesAsync`), which is flaky on master since #4630 (details in #4617), and except one Windows run of `StartIsIdempotentAndDisposeStopsListenerAsync` in `Opc.Ua.Redundancy.Kubernetes.Tests` (HttpListener prefix already registered), which this change does not touch either. ## Notes for review - `ReadAxisAsync` costs one TranslateBrowsePathsToNodeIds and at most one Read more per axis. The other snapshots keep their request count. - `AxisSnapshot.Engineering`, `LoadSnapshot.MassEngineering` and `GearSnapshot.GearRatio` are read once and carry no NodeId, like `MotorTemperatureEngineering`. ## Related Issues - Fixes #4660 - Follows up #4619 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
3690341c80 |
DI: wire the PrepareForUpdate and Installation methods per spec and advertise only the facets that are met (#4617)
# Description
**Software update methods** (`fix(di): wire PrepareForUpdate
Abort/Resume and Installation Resume`)
- New internal `SoftwareUpdateTransitions` (one per `SoftwareUpdate`)
checks the current state and moves it under one lock, so concurrent
calls cannot both pass a check.
- `Prepare` (§8.4.8.3) is accepted only from `Idle`. The application's
`OnPrepare` callback gets a token that `Abort` cancels; the token source
is disposed only after both the Prepare call and a racing Abort are done
with it, so a handler still running after Abort can use its token.
- `Abort` (§8.4.8.4) moves `Preparing` to `Idle` (`PreparingToIdle`),
cancels the preparation in flight and reports `Failed` ("Aborted")
through `OnPrepareStateChanged`; the aborted `Prepare` call returns
`Bad_InvalidState`. Outside `Preparing` it returns `Bad_InvalidState`.
- `Resume` (§8.4.8.5) no longer runs the Prepare callback. It moves
`PreparedForUpdate` through `Resuming` to `Idle` and is rejected with
`Bad_InvalidState` outside `PreparedForUpdate` or while the installation
is `Installing`.
- `Installation.Resume` (§8.4.9) moves `Error` to `Idle`
(`ErrorToIdle`),
`Bad_InvalidState` otherwise.
- The dispatcher knows the `Resuming` state and the `ResumingToIdle`
transition, so `CurrentState` / `LastTransition` show their names.
**Advertising** (`fix(di): advertise only the DI facets whose mandatory
units are met`)
- `DI Offline`, `DI SU UpdateStatus`, `DI SU FileSystem Loading`,
`DI SU Installation for File System` and `DI SU Installation for Cached
Loading` are no longer claimed (see the issue for why each is unmet).
- The facets are derived from the advertised units with the mandatory
units of the DI 1.05 profile database, so a facet cannot be listed
without them. Today this leaves Locking and Software Update Base;
DeviceIntegrationHost, FileSystem, Direct and Cached Loading come back
by themselves once the missing pieces are built.
- `docs/DeviceIntegration.md` documents both tables and the missing
pieces.
Tests: `SoftwareUpdateMethodTransitionTests` (9 tests) and
`DiNodeManagerConformanceTests` (5 tests) are new;
`DiConformanceFacetTests`
and `PumpAddressSpaceComplianceTests` expected the old claims and are
updated. All 9 transition tests and 8 of the 10 conformance tests fail
against master; the other two are the unchanged locking test and a guard
that every advertised facet carries its mandatory units (master
over-claims
the units too, so the guard alone holds there).
Behaviour change for users: a plain DI server no longer lists the
DeviceIntegrationHost facet, and Direct/Cached/Package software updates
list only Software Update Base.
Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run
36779552603](https://github.com/biancode/UA-.NETStandard/actions/runs/36779552603),
[CodeQL run
36779556125](https://github.com/biancode/UA-.NETStandard/actions/runs/36779556125).
## Related Issues
- Fixes #4616
## Checklist
- [x] 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.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] 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: Marc Schier <marcschier@users.noreply.github.com>
|
||
|
|
a3166fa560 |
WebApiClient: report HTTP failures as StatusCodes and accept gzip responses (#4648)
# Description `WebApiClient` threw the exceptions of `HttpClient` for every HTTP failure except 429/503. It now throws `ServiceResultException` with a matching StatusCode, as the transport channels already do. It can also ask for and read gzip compressed JSON (OPC 10000-6 §7.4.5). Two commits: the error mapping, then gzip. ## Changes ### HTTP failures (`src/Opc.Ua.Client/WebApi/WebApiClient.cs`, new internal `WebApiHttpErrors`) | HTTP / failure | StatusCode | |---|---| | 400 | `Bad_DecodingError` | | 401 | `Bad_IdentityTokenInvalid` when the client sent no credentials (bearer token or basic credentials), otherwise `Bad_UserAccessDenied` | | 403 | `Bad_UserAccessDenied` | | 404, 405, 501 | `Bad_ServiceUnsupported` | | 408, 504 | `Bad_Timeout` | | 413 | `Bad_RequestTooLarge` | | 415 | `Bad_DataEncodingUnsupported` | | 429, 503 | `Bad_ServerTooBusy` with the Retry-After hint (unchanged) | | 500 | `Bad_InternalError` | | any other status | `Bad_CommunicationError` | | elapsed `RequestTimeout` / `HttpClient.Timeout` | `Bad_RequestTimeout`, as in `HttpsTransportChannel` and `WebApiTransportChannel` | | failure below HTTP (connection, TLS handshake) | the code `HttpsTransportChannel.MapRequestFailure` assigns, e.g. `Bad_NotConnected` for a refused connection | - The message names the HTTP status, the reason phrase and the route. - For an HTTP status, the inner exception is an `HttpRequestException` naming it (with `StatusCode` set on .NET 5 and later). For a failure below HTTP, it is the original `HttpRequestException`; for a timeout, the `OperationCanceledException`. - A cancellation of the caller's token stays an `OperationCanceledException`. - `WebApiTransportChannel` sends through `WebApiClient`. For an HTTP error status it now reports the mapped code instead of `Bad_UnknownResponse`; its codes for timeouts and connection failures do not change. - `docs/WebApi.md`: new section "Client errors". ### gzip (`WebApiClientOptions.AcceptCompressedResponses`, `src/Opc.Ua.Core/Stack/Https/HttpResponseBodyReader.cs`) - **Requesting gzip:** `AcceptCompressedResponses` (default `false`) sends `Accept-Encoding: gzip`; on a shared `HttpClient` it goes on each request instead of the default headers. `WebApiTransportChannel` passes the option through. - **Reading gzip:** `HttpResponseBodyReader` inflates a body with `Content-Encoding: gzip`. - `MaxMessageSize` applies to the inflated size, so a small compressed body cannot expand past it. - A corrupt gzip body is `Bad_DecodingError`. - A content coding other than gzip or identity is rejected with `Bad_DecodingError` instead of being decoded as if it were plain. - **Stream ownership:** the reader releases only its inflating wrapper. The response stream stays with the `HttpContent` the caller disposes. - `Opc.Ua.Core` references `System.IO.Compression` on net48 explicitly (it compiled through the expanded .NET Standard facades before, but should not depend on them). - `Opc.Ua.Client` grants `InternalsVisibleTo` to `Opc.Ua.Bindings.Https.WebApi.Tests` for the internal shared-`HttpClient` constructor. - `docs/WebApi.md`: new section "Compressed responses". ## Tests - **New:** `WebApiClientErrorAndCompressionTests` in `Opc.Ua.Bindings.Https.WebApi.Tests` (stub `HttpMessageHandler`, no server): - error mapping: `HttpErrorStatusSurfacesAsServiceResultException` (14 cases, every row of the status table), `ThrottledRequestSurfacesAsServerTooBusyWithRetryAfter` (429, 503), `ConnectionFailureMapsLikeTheTransportChannel`, `TlsHandshakeFailureMapsLikeTheTransportChannel`, `ElapsedRequestTimeoutSurfacesAsBadRequestTimeout`, `CallerCancellationStaysOperationCanceled`; - gzip: `AcceptCompressedResponsesAdvertisesGzipAndDecompressesAsync`, `SharedClientCarriesAcceptEncodingPerRequestAsync`, `GzipResponseIsDecompressedEvenWhenNotAskedForAsync`, `GzipBodyIsBoundedByMaxMessageSizeAfterInflating`, `CorruptGzipIsADecodingError`, `UnsupportedContentEncodingIsRejected`, `IdentityContentEncodingIsReadAsIsAsync`, `ResponseStreamIsLeftToItsContentAsync` (plain and gzip). - **Updated** (they expected the old exceptions): - `WebApiClientTests.InvokeAsyncThrowsServiceResultExceptionOnNon2xxStatus` (was `InvokeAsyncThrowsHttpRequestExceptionOnNon2xxStatus`): `Bad_InternalError` with the `HttpRequestException` as inner exception; - `WebApiResponseQuotaRegressionTests.RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync`: the elapsed timeout is `Bad_RequestTimeout` with the `OperationCanceledException` as inner exception. - **Fail without the fix:** with `WebApiClient.cs` and `HttpResponseBodyReader.cs` from master, 24 of the 29 new test cases fail, as do `InvokeAsyncThrowsServiceResultExceptionOnNon2xxStatus` and three of the six `RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync` cases. The five that pass there guard behaviour that must not change: throttling (2), caller cancellation, identity coding, and a plain body's stream. With a reader that disposes the response stream, both `ResponseStreamIsLeftToItsContentAsync` cases fail. - `Opc.Ua.Bindings.Https.WebApi.Tests` 436/436 and `Opc.Ua.Client.Tests` 2733/2733 on net10.0 and net8.0 (macOS). - `Opc.Ua.Client` and `Opc.Ua.Core` build for net48, net8.0, net9.0 and net10.0 with 0 warnings; `dotnet format` (whitespace, style, analyzers) is clean on every changed file; changed lines covered 112/112 (100 %) with the two suites above (`.azurepipelines/check-coverage.ps1` against master). - Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run 37128530025](https://github.com/biancode/UA-.NETStandard/actions/runs/37128530025) (154 jobs; one net48 job, `ReconnectAsyncSwapsFaultedLeaseEntryAsync` in `Opc.Ua.Client.Tests`, timed out on the first attempt and passed on rerun; this change does not touch reconnect), [CodeQL run 37128531971](https://github.com/biancode/UA-.NETStandard/actions/runs/37128531971). ## Notes for review - `HttpResponseBodyReader` is shared with the binary `HttpsTransportChannel`, so rejecting unknown content codings applies there too. Before, such a body was decoded as plain bytes and failed later with a less helpful error. - The failure-below-HTTP and timeout codes reuse what the transport channels report, so `WebApiTransportChannel`, which sends through `WebApiClient`, keeps its codes for those cases. Its own `HttpRequestException` path no longer sees exceptions from `WebApiClient`; I left it in place. ## Related Issues - Fixes #4647 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
52f8520baa |
Detect resource and limit abuse in the encoder/decoder fuzz harness (#4666)
## Why The encoder/decoder fuzz targets only flagged crashes, unhandled exceptions and encoding-fidelity mismatches. They could not observe a decode that stays within the exception contract but consumes far more memory, CPU or stack than its input justifies, or that materialises values larger than the context's encoding limits. None of the resource-related findings of the codec audit would have been caught. ## What Adds four shared oracles to the fuzz harness (`fuzzing/Common/Fuzz/FuzzOracles*.cs`), applied by every host (libFuzzer, AFL, `--replay`, Tools playback and the NUnit replay): - **Allocation** — wraps the Binary/JSON/XML decode cores, the built-in-type targets, the PubSub UADP/JSON decoders and the UADP reassembler; fails when a decode allocates more than `64x` its input plus `8 MB`, whether it returns or throws. Uses the per-thread allocation counter (the AppDomain counter on .NET Framework). - **Time** — a per-input budget of `1 s + 100 ms/KB`, re-run twice and reported only if the fastest run is still over, so JIT/GC/CI noise does not trip it (`OPCUA_FUZZ_TIME_BUDGET_SCALE`, `0` disables). - **Stack** — every input runs on a worker thread with a `256 KB` stack, so recursion that comes close to the limit of a normal thread is caught in-process as a crash rather than near 200 levels. `StackTestcases` inputs replay through every target in a child process (`--fuzz-replay-all`), since such a crash cannot be caught. - **Decoded limits** — walks a decoded value for strings, identifiers, ByteStrings and arrays beyond the context's `MaxStringLength`/`MaxByteStringLength`/`MaxArrayLength`; matrices are additionally boxed the legacy way so a value no consumer can materialise is reported. The fuzz context uses `4096/8192/4096` so small inputs reach the limits. Supporting changes: - `ArrayOf<T>`/`MatrixOf<T>` are not `IEnumerable`, so the limit walker unpacks them via `ToArray`/`ToArrayOf` (otherwise it silently traversed nothing). - `Decimal` is registered in the fuzz context so the targets exercise `Decimal.Decode`. - **B1-2 bug fix**: `BinaryDecoder.ReadExtensionObject` dereferenced a null element for an XML body that is not an element; now rejected with `BadDecodingError`. Found by replaying the nightly crash corpus under the new exception contract; 5 corpus inputs are added as gating reproducers. - Reproducers for B1-1, T1-3, X1-1 and U1-3, built in process by documented builders (`ResourceReproducerTests`, `PubSubReproducerTests`) and checked in to the corpus, where every target replays them under the oracles. Each was verified by reverting the matching fix locally and confirming the guard fires (e.g. B1-1 then allocates ~21 MB from 481 input bytes). - `Fuzzing.md` / `CrashCorpus.md` document the oracles and the private-input log policy. ## Testing Green on net10.0 and net48: | Suite | net10.0 | net48 | |---|---|---| | `Opc.Ua.Encoders.Fuzz.Tests` | 8130 | 8130 | | `Opc.Ua.PubSub.Fuzz.Tests` | 149 | n/a (modern-only) | | `Opc.Ua.Fuzzing.Tests` | 154 | 154 | | `Opc.Ua.Types.Tests` (encoders) | 3698 | builds | Also verified: `Network`/`Certificates` fuzz test suites, all fuzz hosts and Tools projects build, the fuzz manifest validator passes, `dotnet format whitespace` is clean, and the full ~22k-input nightly crash corpus replays with no oracle false positives. Stacked on `romanett/encoder-decoder-spec-security-83c9f5`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
2871355d18 |
GDS: implement the AliasName Server facet (OPC 10000-17 Annex C) (#4668)
## Summary Implements the **GDS AliasName Server facet** (OPC 10000-17 Annex C) in `Opc.Ua.Gds.Server`, so that the CTT conformance unit *GDS AliasName Discovery* passes apart from two CTT script defects (documented, Mantis drafts in `plans/ctt-issues.md`). Spec requirements: - **C.1:** aggregate the AliasNames of registered Servers that expose the `ALIAS` capability (OPC 10000-12 Annex D). Pull all AliasNameCategory instances and merge those whose BrowseNames have the same Name. Provide the ExpandedNodeId of every referenced node and the ServerUri of the Server that contains it. - **C.2:** on registration, merge the Server's AliasNames into a single tree for TagVariables/Topics, combine aliases that share a name, and add the ServerUri to `ServerArray`. - **C.3:** on unregistration, remove that Server's aliases and categories (but keep those other Servers still provide), remove its AliasFor references, and remove its ServerUri from `ServerArray`. ### Design - **`AliasNameAggregator`** (`Opc.Ua.Gds.Server.AliasNames`) holds one snapshot per registered application and builds the merged view: - Categories are keyed by their BrowseName path below `Aliases`. Aliases are merged by name within a category; their targets are the union over all sources. - Targets are stored as namespace-URI ExpandedNodeIds. The ServerIndex is resolved against the server's `ServerArray` each time the view is built, because removing a ServerUri renumbers the entries after it. - Only ServerUris the aggregator added itself are removed again. - Also implements `FindAlias`/`FindAliasVerbose`, recursive over sub-categories. - **`SessionAliasNameSourceReader`** reads a source over a client Session by browsing the `Aliases` (i=23470) hierarchy and the AliasFor references. - It browses instead of calling `FindAlias` because the category structure has to be aggregated too, and a source whose aliases come from a NodeSet (like the CTT's embedded server) does not implement `FindAlias`. - It connects anonymously, to the most secure endpoint first, falling back to SecurityMode None. It runs GetEndpoints once per DiscoveryUrl. - **`ApplicationsNodeManager`** (new partial `ApplicationsNodeManager.AliasNames.cs`): - Hooks RegisterApplication (now `OnCallAsync`), UpdateApplication (background re-read) and UnregisterApplication. - Materializes the merged view as `AliasNameType` and `AliasNameCategoryType` nodes in the GDS application-record namespace, organized by the well-known TagVariables/Topics/Aliases objects. Each category gets a `FindAlias` answered from the merged list. - Changes and node updates are serialized, so a read that finishes after a concurrent unregister cannot re-add the source. - RegisterApplication waits at most `AliasNameRegistrationWait` (5 s) for the read; a slower or unreachable source is finished in the background and never blocks clients past their call timeout. - Optional periodic re-read via `AliasNameRefreshInterval`; it also retries sources that could not be read. - **`IAliasNameStoreRegistry.RegisterContributor`/`UnregisterContributor`** (Opc.Ua.Server): a read-only contributor's aliases are added to the `FindAlias`/`FindAliasVerbose` results of the categories it reports, next to the owning store's. This lets the aggregated aliases share the well-known categories with the server's own aliases, which matters for the reference server, an AliasName Server and a GDS at once. Contributors never receive Add/Delete calls. - **Opt-in:** `GlobalDiscoveryServerConfiguration.EnableAliasNameAggregation` is off by default, because the GDS then opens client connections to the DiscoveryUrls of registered applications. `Ctt.ReferenceServer.Config.xml` enables it. - `Opc.Ua.Gds.Server` now references `Opc.Ua.Client`; the `Opc.Ua` meta-package already depends on both. ### CTT (1.05.513 with the patched scripts), p14-gds, reference server `--ctt`, embedded CTT source + 2 extra AliasName sources | GDS AliasName Discovery | before | after | after, with the C54/C55 script fixes | | --- | --- | --- | --- | | initialize | Skipped (no sources) | Pass | Pass | | 001 | Error | Error (C55) | Pass | | 002 | Error | Error (C54) | Warning (RegisterApplication > 100 ms) | | 003 | Pass | Pass | Pass | | 004 | Error | Error (C54) | Pass | | 005 | Skipped | Error (C55) | Pass | | 006, 007, 009, 011, 012, 014, 015 | Skipped | Error (C54) | Pass | | 008 | Skipped | Pass | Pass | | 010, 013 | Skipped | Error (C55) | Pass | - **C54:** the script compares the GDS's AliasFor target (ServerIndex > 0, namespace URI, as Annex C.1 and Part 4 §7.16 require) with the source's local NodeId using `equals()`, which can never match. - **C55:** "no remote aliases" is checked as "no aliases", so it fails on the reference server's own aliases. Both are documented with draft Mantis texts (not filed). The remaining script checks pass in 15/15 test cases. No regressions: - Application Directory and Query Applications are unchanged against the baseline. The only difference is one ActivateSession timing warning in one run. - p09 (AliasName CUs) passes 32/32, identical to the baseline. ## Tests - `AliasNameAggregatorTests` (unit) covers: - merging across Servers; - categories kept while still referenced and removed after the last source; - ServerArray add, remove and renumbering; - pre-existing ServerUris kept; - local targets; - FindAlias/FindAliasVerbose scope, pattern and reference-type filter. - `GdsAliasNameAggregationTests` (end-to-end) registers a second in-process reference server with `ALIAS` and reads it with the real session reader. It checks: - aggregated nodes and remote AliasFor targets; - the ServerArray entry; - FindAlias on TagVariables and on the aggregated category; - that unregistering removes everything; - that an unreachable source still registers; - that a Server without `ALIAS` is ignored. - Registry contributor tests in `AliasNameStoreRegistryTests`. - Opc.Ua.Gds.Tests: 1246 passed, 0 failed, 56 skipped. Opc.Ua.Server.Tests AliasNames: 198/198. InformationModel and Features alias tests pass. net48 compiles. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
35c08874ca |
Server view parameter status codes; client transfer fallback when GetMonitoredItems is unavailable (#4654)
Two defects found by interop testing the 2.0 stack against foreign OPC
UA stacks (node-opcua, Milo, asyncua, open62541, async-opcua, gopcua).
## 1. Server: view timestamp/version without a ViewId returned
Bad_ViewIdUnknown
A Browse with a `ViewDescription` whose `ViewId` is null but whose
`Timestamp` (or `ViewVersion`) is set got `Bad_ViewIdUnknown` from the
node managers. `ViewDescription.IsDefault` is false, so
`ValidateViewDescription` looked up the null `ViewId` and failed with
"unknown view".
Spec (OPC 10000-4):
- §7.44: a null `viewId` means the entire AddressSpace. Timestamp and
viewVersion each select a version of the View, and "either the Timestamp
or the viewVersion parameter may be set by a Client, but not both".
- Table 178 / Browse service results (§5.9.2.3):
`Bad_ViewTimestampInvalid` = "the view timestamp is not available or not
supported", `Bad_ViewVersionInvalid` likewise, and
`Bad_ViewParameterMismatch` = the view parameters are not consistent
with each other.
Fix: a new internal `ViewDescriptionValidator.ValidateParameters`
handles the checks that don't depend on the View node:
- timestamp **and** version set → `Bad_ViewParameterMismatch` (before
this, a known view got `Bad_ViewTimestampInvalid`)
- null ViewId + timestamp → `Bad_ViewTimestampInvalid`
- null ViewId + version → `Bad_ViewVersionInvalid`
It is called at service level in
`NodeManagerServiceDispatcher.BrowseAsync`, because these are Browse
*service* results. It is also called in
`CustomNodeManager2.ValidateViewDescription`,
`AsyncCustomNodeManager.ValidateViewDescription` and the
`DiagnosticsNodeManager` override, so a node manager called directly
answers the same way. TranslateBrowsePaths and RegisterNodes take no
ViewDescription, and BrowseNext reuses the validated view from the
continuation point.
## 2. Client: transfer failed when the optional Server.GetMonitoredItems
is unavailable
After `TransferSubscriptions` returns Good, the client calls
`Server.GetMonitoredItems` (i=11492) to map server handles to client
handles. That call is only made for a subscription that is not live in
this client: one restored from saved state, or a clone. Its own live
subscriptions skip it. `GetMonitoredItems` is an **Optional** method of
ServerType (OPC 10000-5 §6.3.1, §9.1), and several stacks don't provide
it: asyncua 2.0.1 answers `BadNothingToDo`, open62541 built without
`UA_ENABLE_XML_ENCODING` answers `BadInternalError`, and others answer
`BadMethodInvalid` or `BadNotSupported`. When the call failed, the whole
transfer was reported as failed ("SubscriptionId N could not be moved")
even though the server had moved the subscription. For non-"definitive"
codes such as BadNothingToDo or BadInternalError, the transferred server
subscription was also left behind as an orphan.
### Chosen fallback
The status of a failed call is now classified
(`GetMonitoredItemsFallback.IsTransientFailure`):
- **Transient** (timeout, busy server, too many operations, lost
connection, channel or session): behaviour is unchanged. The transfer
fails and nothing is deleted, so it can be retried.
- **Anything else** (method missing, not supported, BadNothingToDo,
BadInternalError, unusable output): the server does not provide the
method. Then:
1. **The client knows server ids for its items** (the usual case:
restored state, or a reconnect of its own subscription). Use that
mapping and the transfer succeeds. A transfer keeps item ids and client
handles, which is why OPC 10000-4 §6.8 tells clients to store the
SubscriptionId together with the MonitoredItem client and server
handles. Items saved without an id were never created on the server, and
the normal apply path creates them, as before.
2. **No server ids are known** (a clone of a live subscription,
`CloneSubscriptions`). Notifications would still arrive, because clones
copy the client handles. But the items could never be modified or
deleted, and recreating them would duplicate them on the server, since
stale items can't be deleted without their ids. So:
- classic engine, when the caller just moved the subscription
(`Session.TransferSubscriptionsAsync`): delete the transferred server
subscription and **recreate it in place** with the same items and client
handles. The transfer reports success, but notifications the server
still had queued for the old subscription are lost. The subscription
gets a new id.
- classic `Subscription.TransferAsync` (also used to adopt the session's
own subscriptions, so nothing was moved): fails as before, and nothing
is deleted or created.
- new engine (`MonitoredItemManager.TrySynchronizeHandlesAsync`): fails
as before. Its callers (`SubscriptionManager`) already recreate the
subscription in that case.
Both engines now treat an unusable GetMonitoredItems answer (wrong
output arguments) the same way as an unavailable method. The two
existing
`TryCompleteTransferAsyncShouldReturnFalseWhenResponseWrong{1,2}Async`
tests were updated and renamed to match. The existing classic test that
expected a "definitive" failure to delete the subscription now checks
the fallback to the known ids instead.
Warnings are logged when the fallback is used (`Subscription + 71..73`,
`MonitoredItemManager + 6`).
## Tests
- Server:
`AsyncCustomNodeManagerTests.BrowseAsyncNullViewIdWithVersionParametersReturnsViewParameterErrorAsync`
(sync and async node manager, timestamp, version, both) and
`MasterNodeManagerDeterministicTests.BrowseAsyncNullViewIdWithVersionParametersThrowsViewParameterError`
(service level).
- Classic client (`ClassicSubscriptionCoverageTests`): fallback to known
ids for every non-transient code, while transient codes still fail; a
Bad method result (asyncua style); clone recreated in place (old
subscription deleted, items created with the original client handles);
clone adopted through `TransferAsync` is neither deleted nor recreated.
- New engine (`SubscriptionTests`): known ids used for Bad /
BadMethodInvalid / BadNotSupported / BadNothingToDo / BadInternalError;
transient failure and unknown ids still fail.
Run locally (net10.0, sequential):
- Opc.Ua.Client.Tests: 2745 passed / 0 failed
- Opc.Ua.Server.Tests: 8089 passed / 0 failed / 5 skipped
- Opc.Ua.Subscriptions.Durable.Tests: 36 passed / 11 skipped
- Opc.Ua.InformationModel.Tests (View/Browse filter): 289 passed / 4
skipped
- Opc.Ua.Sessions.Tests (Transfer/Reconnect filter): 155 passed / 103
skipped
- Opc.Ua.Subscriptions.Tests (Transfer/Durable filter): 15 passed / 1
skipped
- net48 build of both test projects: clean
The interop test that found (2),
`ServerFeatureInteropTests.TransferSubscriptionToNewSessionAsync`, is in
the uncommitted foreign-interop work and isn't on master yet.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
|
||
|
|
3bb0cd8d55 |
Fix ComplexTypes interop and TCP channel dispose CI flakes (#4663)
# Description
Fixes two intermittent CI failures on master.
## Interop: `LegacyClientExtendedCheckAsync("ComplexTypes")` /
`LoadAndDecodeLegacyComplexTypesAsync`
Flaky since #4630. The node reported as "not decoded" is always
`Data_*_UserScalar_VariantValue` (ns=5;i=2340 on the 2.0 reference
server, ns=3;i=3104 on the 1.5 one). Its DataType is the
BaseDataType-derived `VariantDataType`, and `TestDataSystem` fills it
with `DataGenerator.GetRandomVariant()`. About 2.4 % of the time that
draw is an `ExtensionObject` with a random TypeId (e.g. `s=香蕉`) that no
client can decode. Both checks counted every ExtensionObject of a
variable with a non-ns0 DataType as a structure.
**Fix:** both checks now count only variables whose DataType derives
from `Structure`. They browse the `HasSubtype` closure of `Structure` to
decide this.
- In the 2.0 test, the browse-with-continuation loop moved into a shared
`BrowseAsync` helper.
- In the 1.5 peer, the existing Structure-subtype walk in
`DiagnoseEncodingNodesAsync` moved into `BrowseStructureSubtypesAsync`,
which both places now use.
Seen in master CI for
|
||
|
|
6f55500577 |
Server diagnostics: let every session browse its own subscriptions in SubscriptionDiagnosticsArray (#4664)
# Description `Server.ServerDiagnostics.SubscriptionDiagnosticsArray` denied Browse and Read to every session except a SecurityAdmin on a SignAndEncrypt channel. A regular session, such as the CTT's default SecurityMode None session, got `Bad_UserAccessDenied` when it browsed the array, so it could not find its own subscription diagnostics. Part 5 §6.3.5 lets a session see its own diagnostics, and the CTT maintainer asked that the server return only the entries a session may see instead of blocking the whole array ([Mantis 11445](https://mantis.opcfoundation.org/view.php?id=11445)). **Change** (`DiagnosticsNodeManager.OnReadUserRolePermissions`): non-admin sessions now get `Browse` on the server-wide `SubscriptionDiagnosticsArray`. Each subscription diagnostics node already carries the permissions of its owning session, and Browse drops references to targets the caller may not browse. So a session browsing the array gets only its own subscriptions, and an administrator gets all of them. **Intentionally unchanged:** - Reading the array **value** stays admin-only. The value holds every session's subscriptions. Making it readable with per-session filtering would also need two other fixes: within one second of a diagnostics scan, `OnReadDiagnosticsArray` returns the stored, unfiltered array, and monitored items on the array receive the unfiltered scan value. - `SessionDiagnosticsArray`, `SessionSecurityDiagnosticsArray` and `ServerDiagnosticsSummary` keep their permissions. No session nodes are linked under `SessionDiagnosticsArray`, so opening Browse there would return nothing. **Note for reviewers:** CTT `Subscription Durable/012.js` now gets past the Browse and Read in Step 3, but it still fails because of a script bug (missing braces on line 37 set `result = false` unconditionally). That part is a CTT fix tracked in Mantis 11445. ## Related Issues - Mantis 11445 (CTT Subscription Durable 012.js, Step 3) ## Tests - New end-to-end test `DiagnosticsTests.SubscriptionDiagnosticsArrayBrowseReturnsOnlyOwnSubscriptionsAsync`. Two anonymous SecurityMode None sessions each create a subscription. The fixture session's Browse returns only its own subscription, reading the array value returns `BadUserAccessDenied`, and the sysadmin session sees both subscriptions. Against the old permissions, the test fails with `BadUserAccessDenied` on the Browse. - `DiagnosticsNodeManagerTests.ReadUserRolePermissions_AdminAndSessionHandlingAsync` now also checks that a non-admin gets Browse only on the subscription array, an admin can Read it, and the two session arrays still give a non-admin no permissions. - Local net10.0: `DiagnosticsTests` + `BaseInfoEnabledFlagToggleTests` 14/14 passed, `DiagnosticsNodeManagerTests` 22/22 passed. The test projects build on net48. ## Checklist - [x] 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. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
89bbc3bd75 |
Server: public CreateAlarm<TState> fluent overload, and conformance units that follow their contributors (#4650)
# Description Two changes to `Opc.Ua.Server`, one commit each. Both are independent of any companion model; the Asset Management Basics contribution (#4649) is the first code that needs them, but any companion node manager can use them. **1. `CreateAlarm<TState>` on `INodeBuilder`.** - The fluent alarm surface offers `CreateLimitAlarm`, `CreateExclusiveLimitAlarm` and `CreateOffNormalAlarm`. The pipeline behind them (`AttachAlarm`) is private, and `FluentNodeRegistration` is internal. - A node manager that needs any other condition type cannot reach that pipeline from outside the assembly. Examples are an alarm type of a companion specification, a `TripAlarmType` or `DiscrepancyAlarmType`, or a server-specific subtype that implements an interface such as `IRootCauseIndicationType`. Such a node manager has to copy the attach, notifier and release logic, or it loses the automatic release on teardown. - `CreateAlarm<TState>(INodeBuilder, QualifiedName, Func<NodeState, TState>)` exposes the same pipeline for a state the caller's factory creates. The alarm is initialized, enabled and attached with `HasCondition`. The parent becomes its `SourceNode`, `SourceName` and event notifier. A fluent node manager releases all of it on teardown. - A factory that returns `null` fails with `InvalidOperationException` before anything is changed. The three existing helpers never take that path, so they behave as before. **2. `ConformanceUnitsManager` recomputes the advertised units from its contributors.** - On master the manager reads a contributor once, in `Register`, and adds its units and profiles to two sets that only grow. - A unit or profile that a contributor stops reporting therefore stays in `ConformanceUnits` and `ServerProfileArray` until the server restarts. One that it starts reporting after registration is never published. - The ServerCapabilities describe the current configuration of the server (OPC 10000-5 §6.3.2), so both have to follow the contributor. This matters for contributors whose support changes at runtime, for example a node manager whose units depend on the instances registered with it, or `HistorianConformanceContributor` after `RefreshAsync`. - The manager now keeps the registered contributors and reads all of them again on every `PublishAsync`: - `Register` is idempotent; - `Unregister` is new; - publishing is serialized (`SemaphoreSlim`), so the address space ends with the set read last; - the contributors are read outside the manager's lock. - `DiagnosticsNodeManager.PublishConformanceUnitsAsync` keeps the profiles that were in `ServerProfileArray` before the first publish, which are the configured ones, and replaces the contributed ones on every publish. **Behaviour change for custom contributors.** A profile or unit that no contributor reports any more is now withdrawn on the next publish. Registering a contributor twice no longer reads it twice; the result is the same set as before. A contributor whose sets are fixed at startup sees no difference. The DI, Machinery, Pumps, Scales, Robotics and Vision node managers register once at startup and publish the same sets as before (see Tests). ## Changes - `src/Opc.Ua.Server/Fluent/AlarmBuilderExtensions.cs`: `CreateAlarm<TState>`; the null check on the factory result in `AttachAlarm`. - `src/Opc.Ua.Server/Server/ConformanceUnitsManager.cs`: contributor list, `Unregister`, recompute in `PublishAsync`, publish gate, `Dispose` releases the gate. - `src/Opc.Ua.Server/Diagnostics/DiagnosticsNodeManager.cs`: configured profiles captured on the first publish, contributed profiles replaced. `IDiagnosticsNodeManager.cs` and `HistorianConformanceContributor.cs`: documentation only. - `docs/NodeManagers.md`: `CreateAlarm<TState>` next to the dedicated helpers. `docs/Profiles.md`: how contributed units and profiles are published and withdrawn. ## Tests - **New, part 1** (`Opc.Ua.Server.Tests`, `Fluent`): - `AlarmBuilderExtensionsTests`: `CreateAlarmAttachesTheFactoryStateLikeTheDedicatedHelpers`, `CreateAlarmAcceptsAConditionThatIsNoAlarm`, `CreateAlarmNullArgsThrow`, `CreateAlarmRejectsAFactoryWithoutAnAlarm`, `CreateAlarmOnVariableThrowsBadTypeMismatch`; - `FluentAlarmRegistrationIntegrationTests.CreateAlarmIsBrowsableAndFindableAsync`; - `NodeAttachBuilderExtensionsTests`: `GenericAlarmWiringIsUndoneOnTeardownAsync` (release on teardown). - **New, part 2** (`Opc.Ua.Server.Tests`): - `ConformanceUnitsManagerTests`: `PublishWithdrawsWhatAContributorNoLongerReportsAsync`, `RegisteringAgainReadsTheContributorOnceAsync`, `UnregisterWithdrawsTheUnitsOfTheContributorAsync`; - `DiagnosticsNodeManagerTests.PublishConformanceUnits_KeepsDeclaredProfilesAndWithdrawsContributedOnesAsync`. - **Fail without the change:** with the `src/` files of master, the part 1 tests and `UnregisterWithdrawsTheUnitsOfTheContributorAsync` do not compile (`CreateAlarm` and `Unregister` do not exist). With that test left out, `PublishWithdrawsWhatAContributorNoLongerReportsAsync` (the withdrawn unit is still published), `RegisteringAgainReadsTheContributorOnceAsync` (the contributor is not read on publish) and `PublishConformanceUnits_KeepsDeclaredProfilesAndWithdrawsContributedOnesAsync` (the contributed profile stays) fail; the seven existing `ConformanceUnitsManagerTests` pass. - **Existing contributors:** the conformance, facet, profile and compliance tests of the companion suites pass unchanged on net10.0: `Opc.Ua.Di.Tests` 33, `Opc.Ua.Machinery.Tests` 19, `Opc.Ua.Scales.Tests` 5, `Opc.Ua.Robotics.Tests` 128 (incl. `RobotIntentHostingComplianceTests`), `Opc.Ua.Vision.Tests` 9 (`VisionNodeManagerIntegrationTests`), and the whole `Opc.Ua.Pumps.Tests` suite (73). They check the units, facets and profiles each node manager advertises against what it built, and pass as they do on master. - `Opc.Ua.Server.Tests`: 7966 passed / 9 skipped / 0 failed on net10.0 (full suite); the affected tests (`Fluent`, `ConformanceUnitsManagerTests`, `DiagnosticsNodeManagerTests`, the historian and other conformance tests, 934) pass on net10.0 and net8.0 (macOS). - `Opc.Ua.Server` builds for net48, net8.0, net9.0 and net10.0 without new warnings (the only one, CA2213 in `SamplingGroup.cs`, is on master since #4629); `dotnet format` (whitespace, style, analyzers) reports nothing on the changed files; changed lines covered 70/70 (100 %) by `.azurepipelines/check-coverage.ps1` with the `Opc.Ua.Server.Tests` run on net10.0. - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37135986886](https://github.com/biancode/UA-.NETStandard/actions/runs/37135986886) and [CodeQL run 37135988487](https://github.com/biancode/UA-.NETStandard/actions/runs/37135988487). CodeQL is green; CI is green except `test-windows-net48 (11/32)`, the `Opc.Ua.Interop.Tests` legacy ComplexTypes check ("1 of N structures were not decoded") that came with #4630 and fails the same way on other open pull requests; this change does not touch ComplexTypes. ## Notes for review - The two commits are independent. If you prefer to discuss the behaviour change of part 2 separately, part 1 can go in on its own. - `ConformanceUnitsManager` stays a non-sealed public class, as on master. - The Robotics Intent and Vision node managers also merge their facet URIs into `ServerProfileArray` directly, at startup and when a controller host is added. They report the same URIs through `IConformanceContributor`, so a later publish keeps them. The entries they write before the first publish count as configured, as everything else that is in the array at that point. - `IServerInternal` does not expose the manager since the service-locator cleanup (#4184). A contributor that changes at runtime reaches it through `ServerInternalData.ConformanceUnitsManager`. If you would rather offer a narrower hook (for example a `RepublishConformanceAsync` on the server), we can add it in this PR. ## Related Issues - Part of #4649 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
4c1c3fd244 |
Aggregates: fix remaining CTT aggregate server defects and seed Bad data for the CTT (#4655)
Fixes the remaining CTT Aggregate failures of the reference server that are server defects or missing test data, using the patched CTT 1.05.513 scripts (all known Mantis defects fixed) as the oracle. ## Server fixes (`src/Opc.Ua.Server`) - **Real-valued bounds for TimeAverage/TimeAverage2/Total/Total2.** `SlopedInterpolate` converted every interpolated bound to the raw data type, so on Int32 nodes these Double aggregates used rounded bounds (ramp 0..23.8: 11.9034 instead of 11.9). The conversion (Part 4 §7.7.3) is now only applied for aggregates that return the raw data type (Interpolative, Start/EndBound, DeltaBounds, Minimum2/Maximum2/Range2, ActualTime2); `AverageAggregateCalculator` keeps the real bound (Part 13 §3.1.8, Table 50). - **Non-numeric values are interpolated stepped.** Boolean, String and other non-numeric values cannot be interpolated with a sloped line (Part 13 §5.4.2.3, Table 50). The calculator now uses stepped interpolation for them whatever the Stepped property says: a String EndBound between "23" and "24" is "23" (was "23.8"), a Boolean bound between false and true is false, and Boolean DurationInState no longer counts the region before the first value as true. - **TreatUncertainAsBad = false in Average and Count.** Both used only strictly Good values for the result but counted Uncertain values as Good for the status. They now include Uncertain values when TreatUncertainAsBad is false (Part 13 §4.2.1.2), like StandardDeviation and NumberOfTransitions already did. - **Interpolative honors TreatUncertainAsBad for a raw value at the interval start**, as the calculator already did for the raw values around a bound. - **NumberOfTransitions**: a Bad result carries no aggregate bits, like Count. - **Processed reads get the bounding values they need** (`HistorianDispatcher`). The raw read only returned the raw value at or beyond each end of the request. When that value is Bad (or Uncertain with TreatUncertainAsBad), the interpolated bound needs the nearest Good value further out, so an interval starting on a Bad value returned `BadNoData` and the next interval lost its first region. The dispatcher now reads outwards to the nearest Good values (reusing the ReadAtTime bound search), only when the end value is not Good. ## Reference server test data and CTT template - Every scalar history node also gets 400 values at a fixed date (`2026-01-01T00:00:00Z`) with a block of five Bad values, laid out for the CTT's Bad-data search (`HAAggregateHelper.GetBadData`). StartOfArchive is the fixed date. The values relative to the server start are unchanged. - `samples/UAReferenceServer.ctt.xml`: all ten `.../Aggregates/StartOfBadData*` settings point at the block (`2026-01-01T00:25:01.234Z`), and `RequestedNodeId` is enabled with namespace 2 (needs the C34 fix). - `plans/ctt-issues.md` / `plans/ctt-testing.md`: updated configuration notes, the oracle differences, U4, and new U5 (TreatUncertainAsBad = false). Also documents a CTT defect: the Bad-data search's "reverse" read sends EndTime = 0001-01-01 (null), which a server reads forward. ## CTT results (p06 Aggregates, 1267 test cases) | | Error cases | | --- | --- | | Before (patched scripts, origin/master) | 253 | | This PR, patched scripts | 125 | | This PR, patched scripts + proposed Int32 oracle corrections for TimeAverage/Total(2) and Range2 | 51 | The 125 include 78 TimeAverage/TimeAverage2/Total/Total2 and 4 Range2 test cases on Int32 nodes where the server now returns the Part 13 value but the patched oracle still truncates the bounds. The remaining 51 are spec questions: U4 (DeltaBounds, 9; TimeAverage2/Total2 with an Uncertain value after the start bound, 8) and U5 (TreatUncertainAsBad = false, 34). p07 Historical Access: no change (0 errors). ## Tests - New `AggregateBoundDataTypeTests` (real bounds on Int32, Interpolative still rounds, Boolean/String stepped bounds, Boolean DurationInState, Average with TreatUncertainAsBad). - `AggregateCttRegressionTests`: direct and live (dispatcher) interval starting on a Bad value, Interpolative with TreatUncertainAsBad, NumberOfTransitions Bad result; the Count test now expects Uncertain values to be counted with TreatUncertainAsBad = false. - `AggregateCalculatorCoverageTests`: an Uncertain raw value at the start with TreatUncertainAsBad is interpolated over. - `Opc.Ua.Server.Tests` and `Opc.Ua.History.Tests` pass on net10.0; net48 builds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
87122b5a24 |
Certificate validation: issuer store completes the chain but is no trust anchor (fix #4604 CTT regression) (#4653)
# Description #4604 ("Trust leaf-only certificate chains from configured issuers") made the issuer walk in `CertificateValidationCore.GetIssuersAsync` set `isTrusted = true` for every CA it resolved from the **issuer** list (`TrustedIssuerCertificates`, `UserIssuerCertificates`, `HttpsIssuerCertificates`). That turns the issuer list into a trust anchor, so the server accepted: - client application certificates issued by a CA that is only in `pki/issuer`: CTT *Security General > Security Certificate Validation 045* ("issued certificate of a CA that is not trusted but available") expects `BadSecurityChecksFailed` on OpenSecureChannel and got `Good`; - user X509 tokens issued by a CA that is only in `pki/issuerUser`: CTT *Security User Token > Security User X509 014* expects `BadIdentityTokenRejected` on ActivateSession and got `Good`. Both passed in the CTT run of 2026-09-29 (before #4604) with the same PKI and scripts. ## Spec reasoning OPC 10000-4 §6.1.3 (Determining if a Certificate is trusted): a certificate is trusted if it, or a CA certificate in its chain, is in the **Trust List**. The **Issuer List** only supplies CA certificates needed to build the chain and is not a trust anchor. OPC 10000-12 (TrustList: TrustedCertificates vs IssuerCertificates) uses the same split. Before #4604 the stack followed this, and #4604 flipped the tests that pinned it (`ExplicitIssuerChainAloneDoesNotEstablishTrustAsync`, `VerifyMissingCRLNoTrustAsync`, the GDS `X509TestUtils` issuer-only assertion). ## Interop trade-off The scenario from #4600 is a peer that sends only its leaf certificate while the receiver has the CA configured. That case **still works** when the receiver trusts the CA, which is the spec-compliant configuration: - CA (root or intermediate) in the trusted store, the other CAs in the issuer store or the trusted store: the issuer walk completes the chain from the stores and the trusted CA makes it trusted. No chain from the peer is needed. - CAs only in the issuer store: rejected (`BadCertificateUntrusted` → `BadSecurityChecksFailed` / `BadIdentityTokenRejected`). To help the operators from #4600, the `BadCertificateUntrusted` result now says, when the chain was built but no CA in it is trusted, that issuer certificates only complete the chain and that the CA has to be added to the trusted certificates. I did not add a configuration switch that makes the issuer list a trust anchor. It would turn off a §6.1.3 check, make every CA a GDS pushes as an issuer trusted, and need a new `SecurityConfiguration` field. The comment on #4600 also points at other causes (X509Store vs directory store behaviour, `.cer` files ignored by the directory store). Those are separate and unchanged here: the directory store has read only `*.der`/`*.pem` since 1.5.378. If an opt-in is still wanted, it can be added later as an explicit, default-off `SecurityConfiguration` flag. ## Changes - `CertificateValidationCore.GetIssuersAsync`: remove the `isTrusted = true` for issuer-store CAs (the walk still continues, so a trusted CA further up still anchors the chain). - Clearer `BadCertificateUntrusted` message when the chain was built without a trusted CA. - Restore the three tests #4604 flipped. - New `LeafOnlyChainFromStoresIsTrustedOnlyByTrustedCaAsync` (directory stores, leaf-only, Peers and Users): root and/or intermediate trusted → `Good`; both only in the issuer store → `BadCertificateUntrusted`. ## Verification - `Opc.Ua.Core.Tests` `Security` namespace (net10.0): 1024 passed, 0 failed, 8 skipped. - CTT (isolated PKI, server without `-a`): compared with the 2026-10-03 run of master `efad0ddef` on the same PKI and unmodified scripts: - s01 user token: *Security User X509 014.js* Error → **Pass** - s02 certificate validation: *Security Certificate Validation 045.js* Error → **Pass** - s03 cert management, s04 roles, s05 rest: no result changes. The only differences in s01–s05 are three CTT warnings ("ActivateSession ResponseHeader.Timestamp delay > 100 ms", first ActivateSession of a part). - `Opc.Ua.Gds.Tests` `ClientTest` (net10.0): 106 passed. It uses the restored `X509TestUtils` issuer-only assertion. - `Opc.Ua.Core.Tests` builds for net48 with 0 warnings. ## Related Issues - Fixes the CTT regression from #4604 (#4600). ## Checklist - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
2a619cdf05 |
Client: never select an OpenAPI endpoint in SelectEndpoint, and keep a closed Session from sending requests without authenticationToken (#4646)
# Description Two client fixes, one commit each. **1. `CoreClientUtils.SelectEndpoint` skips OpenAPI endpoints.** - An HTTPS listener with the REST binding announces its OpenAPI endpoint with the same URL and message mode as its binary mode-None endpoint. `SelectEndpoint` matched on the URL scheme and the message mode only, so the first of the two in the GetEndpoints result won, in the main pass and in the HTTPS fallback for `useSecurity`. - The OpenAPI mapping (OPC 10000-6 §G.3) is the REST binding, not the binary transport the caller connects with. With the default channel bindings no channel factory is registered for the OpenAPI profile (`ClientChannelManager` maps it to `opc.https+webapi`), so a Session cannot be created on it; with the REST channel registered, the Session silently runs over REST. - Following OPC 10000-4 §5.5.4 (select by `TransportProfileUri`), both passes now skip endpoints with the HTTPS or WSS OpenAPI transport profile. Endpoints without a `TransportProfileUri` are selected as before. **2. A closed Session does not send requests without authenticationToken.** - A request with a null `authenticationToken` is a session-less invocation (OPC 10000-4 §6.3.1). - After `CloseAsync(closeChannel: false)`, a request sent through the same `Session` object went out that way. A server that supports session-less invocation processed it as one instead of rejecting it. - `Session` now records that a session was created (`SessionCreated` with a non-null cookie; `OpenAsync` now calls it through the override instead of `base.SessionCreated`). From then on `UpdateRequestHeader` fails a request without token locally with `Bad_SessionIdInvalid`, and the request is never sent. CreateSession and ActivateSession are exempt, so reopening and reactivating keep working. A Session that was never created behaves as before. ## Changes - `src/Opc.Ua.Client/CoreClientUtils.cs`: `IsOpenApiEndpoint`, used in both selection passes; the `<returns>` documentation says that an OpenAPI endpoint is never returned. - `src/Opc.Ua.Client/Session/Session.cs`: overrides of `SessionCreated` and `UpdateRequestHeader` (both virtual in `SessionClient` / `ClientBase`), one `volatile bool` field. ## Tests - **New, part 1** (`Opc.Ua.Client.Tests`, `CoreClientUtilsSelectEndpointTests`): `SelectEndpointSkipsTheOpenApiEndpointListedFirst`, `SelectEndpointWithSecurityFallbackSkipsTheOpenApiEndpoint`, `SelectEndpointSkipsTheWssOpenApiEndpoint`, `SelectEndpointReturnsNullWhenOnlyOpenApiEndpointsMatch` (with and without `useSecurity`), `SelectEndpointKeepsTheFirstBinaryEndpointWithoutTransportProfile`. - **New, part 2** (`Opc.Ua.Client.Tests`, `SessionTests` with the channel mock): `RequestAfterCloseFailsLocallyWithBadSessionIdInvalidAsync` (the Read never reaches the channel), `ActivateSessionIsStillSentAfterCloseAsync`, `RequestAfterANewSessionIsCreatedCarriesItsTokenAsync`, `RequestBeforeASessionIsCreatedIsNotRefusedAsync`. - **Updated:** `Opc.Ua.Sessions.Tests` `ClientTest.ReconnectSessionOnAlternateChannelAsync` closes the Session and then the channel. The read that follows now fails with `Bad_SessionIdInvalid` before it reaches the closed channel, and the test expects that code instead of the codes of the closed channel (on master it gets `Bad_NotConnected`). - **Fail without the fix:** with `CoreClientUtils.cs` and `Session.cs` from master, all five SelectEndpoint tests that involve an OpenAPI endpoint (6 cases) fail, as does `RequestAfterCloseFailsLocallyWithBadSessionIdInvalidAsync`, and `ReconnectSessionOnAlternateChannelAsync` fails with `Bad_NotConnected`. `SelectEndpointKeepsTheFirstBinaryEndpointWithoutTransportProfile` and the three other Session tests pass there by design: they pin behaviour that must not change. - `Opc.Ua.Client.Tests` 2743/2743 and `Opc.Ua.Sessions.Tests` 672 passed / 287 skipped / 0 failed on net10.0 and net8.0 (macOS). - `Opc.Ua.Client` builds for net48, net8.0, net9.0 and net10.0 with 0 warnings; `dotnet format` (whitespace, style, analyzers) reports nothing on the changed lines; changed lines covered 26/26 (100 %) with the two suites above. - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37128535726](https://github.com/biancode/UA-.NETStandard/actions/runs/37128535726) (154 jobs) and [CodeQL run 37128537502](https://github.com/biancode/UA-.NETStandard/actions/runs/37128537502). CodeQL is green; CI is green except `Opc.Ua.Interop.Tests` `LegacyClientExtendedCheckAsync("ComplexTypes")` / `LoadAndDecodeLegacyComplexTypesAsync` ("1 of N structures were not decoded"), which came with #4630 and fails the same way on other open pull requests; this change does not touch ComplexTypes. ## Notes for review - Part 2 changes the code a caller sees for a request on a closed Session: `Bad_SessionIdInvalid` is raised locally instead of whatever the server or the closed channel answers. Reconnect paths are not affected, because they use ActivateSession or a new Session object. If you prefer to keep part 2 out, its commit can be dropped without affecting part 1. - The test framework's own endpoint selection (`ClientFixture`) is left as it is; the upstream test servers do not need the change. ## Related Issues - Fixes #4645 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
c801ac1d64 |
Robotics client: expose value NodeIds, operation states and the missing identification and motor fields (#4619)
# Description A Robotics snapshot returns the values it read, but not the variables they came from. A client that shows a snapshot and wants live updates has to resolve every value again before it can subscribe. This PR adds that information to the snapshots, along with the operation state and the DI and motor fields the contracts did not cover. Every addition is an init property with a default; no existing member changes. - **Value NodeIds:** every snapshot value comes with the NodeId of its variable, or `NodeId.Null` when the server does not publish it: - `AxisStateSnapshot.ActualPositionId`/`ActualSpeedId`/`ActualAccelerationId` - `MotionDeviceSnapshot.SpeedOverrideId`, `LoadSnapshot.MassId`, `GearSnapshot.PitchId`, `TaskModuleSnapshot.IsReferencedId` - `SafetyStateSnapshot.EmergencyStopId`/`OperationalModeId`/`ProtectiveStopId`, `SafetyFunctionSnapshot.ActiveId`/`EnabledId` - `TaskControlSnapshot.ExecutionModeId`/`TaskProgramLoadedId`/`TaskProgramNameId` Each id comes from the path resolution the value read already does, so it costs no extra request. - **Operation state:** `ControllerSnapshot.SystemOperationId`. `ControllerSnapshot` and `TaskControlSnapshot` add `CurrentStateId`, the `CurrentState` variable of the SystemOperationStateMachine or TaskControlStateMachine, and `CurrentState`, the `RoboticsOperationState` it named when read. `CurrentState` is `null` when the state machine is absent, its read fails, or it names a state other than Idle, Ready or Executing; the rest of the snapshot is still returned. - **Identification:** `RoboticsComponentIdentification.HardwareRevision`, `SoftwareRevision`, `ManufacturerUri`, `ProductInstanceUri` (DI properties). - **Motor:** `MotorSnapshot.MotorTemperature` with `MotorTemperatureEngineering` (EngineeringUnits, EURange), `BrakeReleased` and `EffectiveLoadRate`, each with its NodeId. They are read from the motor's DI `ParameterSet` with one TranslateBrowsePathsToNodeIds and one Read per motor. - The reads use the path helpers from #4609 and the generated proxies; the per-node structure is unchanged. `docs/Robotics.md` describes the new members with an example. Tests: - **`RoboticsClientServerE2eTests`** (hosted server over opc.tcp). The fixture now publishes the identification revisions and URIs, the motor's unit, range and load rate, the gear pitch, and a SystemOperation that starts in Ready. The new tests: - compare the ids with the server's own nodes, and check that reading each id returns the snapshot value; - subscribe to an `ActualPositionId` and receive changing values; - follow the task control's `CurrentState` through LoadByName and UnloadProgram; - read the identification and motor fields. - **`RoboticsClientFacadeTests`:** check the ids against the ParameterSet children next to same-named decoys, read `CurrentState` alongside its `Id` child, and cover an unknown state, a failing CurrentState read, and a controller and task control without operations. - On the base branch, the new tests do not compile because the members are missing. Local results: - `UA.slnx` Release builds on all target frameworks with no new warnings. - `Opc.Ua.Robotics.Tests`: 650/650 on net10.0 and 590/590 on net8.0. - Changed-line coverage against #4609: 100 % (168/168, `check-coverage.ps1`). - Fork CI (full matrix incl. macOS) and CodeQL on the current head (`3dbb91392`, with master merged in): green — [CI run 36976259619](https://github.com/biancode/UA-.NETStandard/actions/runs/36976259619) (189 jobs), [CodeQL run 36976262134](https://github.com/biancode/UA-.NETStandard/actions/runs/36976262134). ## Related Issues - Fixes #4618 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework via the fork CI's Windows legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
5aa38a6bfa |
DI: roll back a device whose registration fails (#4638)
# Description `DiNodeManager.CreateDeviceAsync` attaches the new device to its parent before it registers it. When the registration threw, the device stayed attached: the name was taken for good, retries failed with `BadBrowseNameDuplicated`, and indexed nodes stayed in the address space. `RegisterDeviceAsync` now undoes a failed registration before it rethrows, the same way `MachineryBuildScope.RollbackRegistrationAsync` does and #4636 does for Pumps and Scales: - The registered subtree is deleted with `DeleteNodeAsync`: its nodes and the references to it. Only the node this call registered is deleted. - A device that never got registered is detached from its parent. - The cleanup runs with `CancellationToken.None`, so a cancelled creation is undone as well. - A failing cleanup is logged and does not replace the original exception. The message is `DiNodeManagerLog.DeviceRollbackFailed`, in a new `DiServerEventIds.DiNodeManager` block (40). `DiNodeManager` has no creation gate. The name stays taken until the device is detached, so a concurrent creation of the same name is still rejected and never sees the half-registered device. `DiNodeManager` is not sealed, so no production test seam is needed. The tests use real failures where there are some: a cancelled token, and a child added by the device factory whose `OnAfterCreate` throws. The rest goes through a `DiNodeManager` subclass: a failure once the device itself is indexed (`AddBehaviourToPredefinedNodeAsync` throws for a child), and a cleanup that fails (`RemovePredefinedNodeAsync` throws). Tests: - New: `DiNodeManagerRegistrationRollbackTests` with `ACancelledDeviceCreationIsDetachedSoTheNameCanBeReusedAsync`, `ADeviceWhoseChildFailsToCreateIsDetachedSoTheNameCanBeReusedAsync`, `ADeviceThatFailsOnceRegisteredIsDeletedSoTheNameCanBeReusedAsync` and `AFailingRollbackDoesNotReplaceTheOriginalFailure`. All four fail without the rollback. - `Opc.Ua.Di.Tests` 437/437 on net10.0 and net8.0. - `Opc.Ua.Di.Server` builds for net472, net48, netstandard2.1, net8.0, net9.0 and net10.0 with 0 warnings. - Fork CI (full matrix incl. macOS) and CodeQL: green on `8828be6c7` (the same change on master `029ac0f33`) — [CI run 36892624508](https://github.com/biancode/UA-.NETStandard/actions/runs/36892624508) (189 jobs), [CodeQL run 36892628971](https://github.com/biancode/UA-.NETStandard/actions/runs/36892628971). On the current head CodeQL is green ([run 37016180722](https://github.com/biancode/UA-.NETStandard/actions/runs/37016180722)) and CI is in progress ([run 37016174455](https://github.com/biancode/UA-.NETStandard/actions/runs/37016174455)). One intermediate run failed once in `SubscriptionTests.DurableSubscriptionSnapshotSurvivesRetransmissionQueueRecycling` (`Opc.Ua.Server.Tests`, which this PR does not touch); it passed in the green run above. ## Related Issues - Fixes #4637 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
96e97100a6 |
Pumps and Scales: roll back a pump or scale whose registration fails (#4636)
# Description `PumpsNodeManager.CreatePumpAsync` and `ScalesNodeManager.PublishAsync` attach the new node to its parent before they register it. When registration threw, the node stayed attached: the name was taken for good, retries failed with `BadBrowseNameDuplicated`, and indexed nodes stayed in the address space. Both now undo a failed registration before they rethrow, the same way `MachineryBuildScope.RollbackRegistrationAsync` does: - The registered subtree is deleted with `DeleteNodeAsync`: its nodes, its root notifier and the references to it. Only the node this call registered is deleted. - A node that never got registered is detached from its parent. - A failing cleanup is logged and does not replace the original exception. - The rollback runs inside the creation gate, so a concurrent creation of the same name never sees the half-registered node. `PumpsNodeManager` is sealed, and nothing a caller supplies runs while a pump is registered. So both managers get an internal, null-in-production test seam in the style of the existing `*ForTest` seams: `PumpRegisteredForTest` and `PublishedForTest`. Scales is also tested with a real failure, a node added through `IScaleBuilder.Scale` whose create lifecycle throws. Tests: - New: `AFailedRegistrationIsRolledBackSoTheNameCanBeReusedAsync` (Pumps), `AScaleThatFailsToRegisterIsDetachedSoTheNameCanBeReusedAsync` and `AScaleThatFailsOnceRegisteredIsDeletedSoTheNameCanBeReusedAsync` (Scales). All three fail without the rollback. - `Opc.Ua.Pumps.Tests` 74/74 and `Opc.Ua.Scales.Tests` 121/121 on net10.0 and net8.0. - Both libraries build for net472, net48, netstandard2.1, net8.0, net9.0 and net10.0 with 0 warnings. - Fork CI (full matrix incl. macOS) and CodeQL: green on `0787372dc` (the same change on master `c4e99c6e3`) — [CI run 36976324157](https://github.com/biancode/UA-.NETStandard/actions/runs/36976324157) (189 jobs), [CodeQL run 36976326761](https://github.com/biancode/UA-.NETStandard/actions/runs/36976326761). The branch is now on a newer master with no overlapping files; the run on the current head is in progress: [CI run 37016161199](https://github.com/biancode/UA-.NETStandard/actions/runs/37016161199). ## Related Issues - Fixes #4635 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
efad0ddeff |
Server subscriptions: fix spec defects in Subscription/MonitoredItem handling found by audit (#4629)
# Description Fixes defects found in a spec-grounded audit of the server **Subscription / MonitoredItem** implementation (`src/Opc.Ua.Server/Subscription/**`, `NodeManager/MonitoredItem/**`, `EventManager`, monitored-item service dispatch and filter validation). Each behaviour change was checked against OPC 10000-4 (§5.13, §5.14, §7.21, §7.22), Part 5 (subscription diagnostics) and Part 9 (ConditionRefresh). Every fix has a regression test. The audit found 56 defects (1 Critical, 3 High, 20 Medium, 32 Low). A follow-up max-effort code review of the fix branch found 27 more issues, mostly introduced by the first round of fixes. All but one are fixed here. ### Most important fixes - **Critical — remote hang:** an `AggregateFilter` with `processingInterval = NaN` was not revised, and `AggregateCalculator` looped forever while holding the node-manager lock, so one CreateMonitoredItems request blocked the node manager. Non-finite, ≤0 and sub-tick intervals are now revised (Part 4 §5.13.2.1), and the calculator rejects them as a backstop. - **Monitored-item limits:** new `ServerConfiguration.MaxMonitoredItemCount` / `MaxMonitoredItemsPerSubscription`; Bad_TooManyMonitoredItems is returned when a limit is hit, and ServerCapabilities now report the limits. Both default to `0` (no limit), so existing behaviour is unchanged. - **Sampling groups:** a client could get one dedicated thread per distinct sampling interval above 1 h; intervals above the last rate group are now rounded, and the loop is async. An item could also end up in two sampling groups and keep being sampled after it was deleted. - **Subscription state machine** (Part 4 §5.14.1.2): - the lifetime counter resets while a Publish is queued and counts otherwise; - the lifetime and keep-alive stay within `MaxSubscriptionLifetime`; - LatePublishRequestCount is counted once per entry into LATE; - messages held while publishing is disabled are no longer sent, and are not offered by Transfer/Republish either; - ConditionRefresh returns Bad_RefreshInProgress for any refresh already queued or running on the subscription. - **Triggering** (Part 4 §5.13.1.6): - linked items are triggered by an item still draining a backlog, by the first value after re-enable, and by an item created Disabled; - links are persisted for durable subscriptions; - Disabled/Sampling items follow the publish rules in the shared pipeline, so custom `IMonitoredItem` implementations follow them too. - **Queues** (Part 4 §5.13.1.5, §7.21): - no duplicate value after a queue-size increase; - EventQueueOverflow events are placed where the events were lost, including on queue shrink; - the duplicate-event check scans the newest entries; - EventQueueOverflowCount and MonitoringQueueOverflowCount are maintained correctly; - changing select clauses rebuilds queued events in the new layout, and is safe on durable queues and on failure. - **Filters:** - per-clause EventFilterResult instead of rejecting the whole item (Part 4 §7.22.3); - corrected status codes: Bad_DeadbandFilterInvalid, Bad_MonitoredItemFilterInvalid, Bad_EventFilterInvalid, Bad_MonitoringModeInvalid, Bad_TimestampsToReturnInvalid; - aggregate filter in subscriptions: processing interval at least 2× the revised sampling interval, revised StartTime on the requested grid, ServerTimestamp = end of interval (Part 4 §7.22.4). - **Lifecycle and races:** - expired subscriptions are always cleaned up, even when the session closes during cleanup; - Create/Transfer racing CloseSession returns Bad_SessionClosed; - session diagnostics counts no longer drift; - a large but valid Publish TimeoutHint no longer fails the request; - a ready subscription is served by queued Publish requests instead of going Late; - restored persisted state is validated. - **Access:** a value change notification is checked against each subscriber's UserAccessLevel, not the writer's. - **Redundancy / durable store:** - the first-unsent sequence number is mirrored; - sequence numbers are no longer reused after a second failover; - triggering links are stored (definition format 6, written only when links exist, so older replicas can still read link-less subscriptions). ### Behaviour changes reviewers should know - With the default config, publishing intervals above `MaxSubscriptionLifetime / 3` (1,200,000 ms) are revised down, so lifetime ≥ 3×keep-alive fits the configured maximum. - The revised keep-alive is capped at a third of the max lifetime. - Idle subscriptions expire after exactly MaxLifetimeCount cycles without a queued Publish. - A partly invalid EventFilter select list is accepted with per-clause errors. The public `EventFilter.Validate` keeps Status Good if at least one clause is valid; callers that cannot return per-clause results must also check the new `EventFilter.Result.HasSelectClauseErrors`. There is a migration note in `docs/migrate/2.0.x/sessions-subscriptions.md`. - Aggregate processing intervals may be revised higher (≥ 2× sampling interval); aggregate ServerTimestamp is now the interval end. - Sampling intervals above 3,600,000 ms are rounded to multiples of 900,000 ms (default rate groups). ### Public API Additive only: - `ServerConfiguration.MaxMonitoredItemCount` and `MaxMonitoredItemsPerSubscription`; - `EventFilter.Result.HasSelectClauseErrors`; - `IStoredSubscriptionTriggering`, `ISubscriptionRetransmissionSendStateStore`, `SubscriptionRetransmissionState.FirstUnsentSequenceNumber` (optional store capabilities, detected with `as`); - new `EventQueueHandler` constructor overloads. ### Known open items (not in this PR) - **Part 4 §5.13.2.1:** CreateMonitoredItems should create an item even when the user lacks Read permission and report the error in Publish. This is not done: the create-time RolePermissions check is the only role check on the initial value, so following the spec would leak values until every node manager re-checks per sample. - **Sampling loop:** it runs on thread-pool threads; node managers with blocking synchronous reads should not use sampling groups heavily. The alternative, a dedicated thread per group, was the thread-exhaustion issue fixed here. - **Restarts:** a pending QueueSize-1 value and a pending overflow flag are not kept across a restart; that needs persistence interface extensions. - **Limit defaults:** decide whether the new limits should get finite defaults (currently 0 = unlimited). ## Related Issues - No tracking issue; the findings come from an internal audit of the server subscription code. ## Checklist - [ ] 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. - [x] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. ### Local verification (net10.0, all green) | Suite | Passed | Failed | |---|---|---| | Opc.Ua.Server.Tests | 7963 | 0 | | Opc.Ua.Subscriptions.Tests | 643 | 0 | | Opc.Ua.Subscriptions.Classic.Tests | 57 | 0 | | Opc.Ua.Subscriptions.Durable.Tests | 36 | 0 | | Opc.Ua.Redundancy.Server.Tests | 847 | 0 | | Opc.Ua.Core.Tests (filter tests) | 283 | 0 | net48 build of `Opc.Ua.Server.Tests`: 0 errors. net48 tests and the full UA.slnx were not run locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
d98eb6b28c |
Fix server session defects found by a spec-checked review of CreateSession, ActivateSession, session lifecycle and continuation points (#4632)
# Description
Fixes defects found by a spec-checked review of the server session
implementation: CreateSession, ActivateSession, the session lifecycle
(per-request validation, CloseSession, Cancel, timeout, shutdown) and
session continuation points. Each finding was checked against OPC
10000-4 (§5.7, §7.9, §7.32), OPC 10000-5 (session diagnostics and audit
events) and OPC 10000-6 §6.8.2.
There is one commit per finding, and each commit message cites the spec
section. A max-effort code review of the fixes then found 15 more
issues, mostly regressions the first round introduced; they are fixed in
the `revfix` merges.
## What changed
**CreateSession**
- At MaxSessionCount the oldest session that was never activated is
closed to make room (§5.7.2.1). Sessions that are activated, already
closing, or have an activation in flight are never evicted. The victim
is claimed atomically, so a late activation commit on it is rejected.
- `requestedSessionTimeout` NaN is revised. A malformed `endpointUrl` no
longer fails the call with Bad_UnexpectedError. `serverUri` is ignored,
as §5.7.2.2 requires.
- Unnamed sessions get distinct names.
- Session diagnostics now report `MaxResponseMessageSize` and
`ServerUri`, and `SessionSecurityDiagnostics.Encoding` uses the
specified names ("UA Binary", "XML", "JSON").
- AuditCreateSessionEvent carries SecureChannelId and the request
certificate, also on failure.
- Every rejected CreateSession is counted, and only once. A rejection
during server startup no longer turns into Bad_UnexpectedError.
- If an `OnApplicationCertificateError` override accepts an error, the
certificate is now kept and the remaining checks still run. A
certificate accepted this way does not grant TrustedApplication or
application-based roles.
**ActivateSession**
- EphemeralKey handling follows OPC 10000-6 §6.8.2:
- A key used to decrypt a UserName or IssuedToken secret is retired and
replaced in the same commit.
- A key that was not used is kept, and nothing is returned.
- ECDHPolicyUri is honoured; an unsupported policy gets
Bad_SecurityPolicyRejected.
- Request parameters are no longer echoed back.
- After the activation commits, a failure or cancellation no longer
faults the response, which would hide the new server nonce. Post-commit
callbacks run uncancellably.
- An activation that overlaps a session close or timeout fails with
Bad_SessionClosed instead of returning Good.
- The PolicyId of an AnonymousIdentityToken is validated.
- Status codes now follow §5.7.3.3:
- Bad_ApplicationSignatureInvalid for a mismatched client-signature
algorithm
- Bad_UserSignatureInvalid for a mismatched X509 user-token signature
algorithm
- Bad_IdentityTokenInvalid for an undecodable token or an unknown policy
- `ClientUserIdOfSession` keeps the user of the first activation.
**Lifecycle and requests**
- Requests on an expired or closing session are rejected with
Bad_SessionClosed (§5.7.2.1). A CloseSession on an expired session that
is not yet closing is still processed as the client's close and honours
`deleteSubscriptions`.
- Closing a session aborts its outstanding requests with
Bad_SessionClosed. A request that registers after that abort pass is
rejected too.
- Cancel completes the matching requests with
Bad_RequestCancelledByClient (§5.7.5.1). It also reaches requests still
waiting in the request queue, but only when both timestamps prove the
request was sent earlier.
- There is one AuditCancelEvent per Cancel call, carrying the client's
audit entry id and user.
- Every close path goes through a single close claim, so
SessionAbortCount, SessionTimeoutCount and the "Session/Terminated" and
"Session/Timeout" audits are each counted once.
- Teardown that started cannot be cancelled. Requests rejected by
session validation are counted.
**Continuation points**
- HistoryRead returns Bad_NoContinuationPoints instead of evicting
points of the same request (§7.9). Only the request that saved a point
pins it, so points from prior requests can still be freed.
- Points saved by a HistoryRead that faults are released.
- Session-less Browse and BrowseNext no longer dereference a null
session.
## Behaviour changes reviewers should know about
- At the session cap, CreateSession evicts a non-activated session
instead of returning Bad_TooManySessions. The eviction is counted in
SessionAbortCount.
- Clients see Bad_SessionClosed for requests on expired or closing
sessions, and Bad_RequestCancelledByClient for cancelled requests.
- `ActivateSessionProcessAdditionalParameters` is now called on every
successful activation, with or without an AdditionalHeader.
- If an `OnApplicationCertificateError` override returns instead of
throwing, CreateSession now accepts the certificate.
- Timeout and termination audits are now raised after the session
teardown.
- The only public API changes are additive overloads in `AuditEvents`
and `RequestManager`.
**Known partial fixes:**
- The session's `maxResponseMessageSize` is reported but not enforced;
only the channel limit applies.
- With overlapping ActivateSession calls on one session, an earlier
response can still carry a key that a later activation has already
replaced. Closing that fully needs a new `ISession` member.
## Related Issues
- The Redundancy.Server session-mirroring findings from the same review
are tracked separately in #4627.
## Tests
- Regression tests cover each finding. The new files are:
- `CreateSessionSpecRegressionTests`
- `ActivateSessionValidationRegressionTests`
- `StandardServerActivateSessionRegressionTests`
- `SessionLifecycleRegressionTests`
- `SessionRequestAdmissionRegressionTests`
- `HistoryReadContinuationDispatchTests`
Existing test classes were extended too.
- Local results on net10.0:
- `Opc.Ua.Server.Tests`: 7883 passed / 0 failed / 5 skipped
- `Opc.Ua.Sessions.Tests`: 790 passed / 0 failed / 379 skipped (same as
master)
- net48 Release build of `Opc.Ua.Server.Tests`: 0 errors / 0 warnings.
## Checklist
- [x] 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.
- [ ] 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.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
5d26681ac3 |
Machinery client: return the NodeId of each metering point measurement (#4642)
# Description `MachineryMeasurementValue` gets an additive init-only `NodeId`; the positional constructor is unchanged. `ReadMeteringPointAsync` fills it, and so do `ReadMainMeteringPointAsync` and `ReadSubMetersAsync`, which use it. The values are now read by the NodeIds the browse returned, in one batched Read, instead of translating each browse name back to a NodeId first. That is one round trip less, and value and NodeId come from the same node. `docs/Machinery.md` shows how to subscribe to the measurements. Tests: - New end-to-end test `MeteringPointMeasurementsCarryTheNodeIdOfTheirVariableAsync` against `MachineryIntegrationServer`, covering the main metering point and the sub-meter. It reads `BrowseName` and `Value` behind every returned NodeId. It fails on master. - `Opc.Ua.Machinery.Tests` 165/165 on net10.0; all target frameworks build with 0 warnings. - Fork CI (full matrix incl. macOS) and CodeQL: green on `afa00065a` (the same change on master `ce88955f7`) — [CI run 36826789991](https://github.com/biancode/UA-.NETStandard/actions/runs/36826789991) (189 jobs), [CodeQL run 36826793071](https://github.com/biancode/UA-.NETStandard/actions/runs/36826793071). Rebasing onto master with #4607 needed one conflict resolution in `MachineryEndToEndTests.cs` (both new tests kept); `Opc.Ua.Machinery.Tests` passes 165/165 on net10.0 locally afterwards and CodeQL is green on that state ([run 36979020583](https://github.com/biancode/UA-.NETStandard/actions/runs/36979020583)). The run on the current head is in progress: [CI run 37016200389](https://github.com/biancode/UA-.NETStandard/actions/runs/37016200389). ## Related Issues - Fixes #4641 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
|
fe6ba606c3 |
Fail sends racing PipeByteTransport.Close with BadConnectionClosed (#4643)
# Description Fixes an intermittent failure of `PipeByteTransportTests.CloseFailsBlockedAndQueuedSendsWithBadConnectionClosedAsync`, seen on windows-net10.0 in run [37025190491](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37025190491) (job 110899124242) on #4629, a PR that does not touch this code: ``` Expected: <ServiceResultException> But was: System.InvalidOperationException: Writing is not allowed after writer was completed. at System.IO.Pipelines.Pipe.DefaultPipeWriter.GetMemory(Int32 sizeHint) at Opc.Ua.Bindings.PipeByteTransport.SendChunkAsync(...) PipeByteTransport.cs:line 101 ``` **Cause:** `PipeByteTransport.Close()` completes the output `PipeWriter` without taking the send lock. A send can pass the closed check and get the lock just before `Close()` runs. In the test this is most likely the queued send: the blocked send releases the lock as it is cancelled, and the queued one gets it before its own cancellation takes effect. That send then calls `GetMemory` on the completed writer and leaks the pipe's `InvalidOperationException`. **Fix:** `Close()` sets the closed flag before it completes the writer. So when a write fails while the transport is closed, both `SendChunkAsync` overloads now turn the failure into `ServiceResultException(BadConnectionClosed)` and keep the original exception as the inner exception. `ServiceResultException` and `OperationCanceledException` are excluded, so cancellation by the caller's own token still surfaces as before. This translates the exception instead of taking the lock in `Close()`: a check before writing would still race, and taking the lock would stop `Close()` from waking a send blocked on backpressure. **Tests:** two new tests, one per `SendChunkAsync` overload, wrap the output writer so `GetMemory` calls `transport.Close()` first. This hits the race every time. Both failed before the fix with the exact CI exception and pass after it. All 28 `PipeByteTransportTests` pass on net10.0, and the original flaky test passed 30/30 in a loop. ## Related Issues - Flaky test seen on #4629 (CI run 37025190491) ## Checklist - [ ] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
fbdd0af13b |
Add 1.5.378 <-> 2.0 interop tests; return BadAttributeIdInvalid for unset optional security attributes (#4630)
# Description
Adds an interop test suite between this stack (2.0) and the released
**1.5.378** NuGet stack, in both directions, and fixes a 2.0 bug the
suite found.
## Interop suite
- **`tests/Opc.Ua.Interop.LegacyPeer`** is a console peer built from the
`OPCFoundation.NetStandard.Opc.Ua.*` 1.5.378 NuGet packages.
- It runs as a child process, because the two stacks share assembly
names.
- It deliberately does not import the repository build settings (its own
`Directory.Build.props/targets/Packages.props`).
- Server mode hosts a small interop server with tight limits, or the 1.5
Quickstarts reference server.
- Client mode runs named checks and prints one JSON result line per
check.
- **`tests/Opc.Ua.Interop.Tests`** builds the peer into `legacy-peer/`
and runs in every CI test leg as a normal `*.Tests.csproj`. It covers:
- sessions over None, the RSA policies and the ECC policies (Sign and
SignAndEncrypt), with anonymous and user name logon, plus
read/write/browse/call/subscribe;
- the existing 2.0 `ClientTest` workers run against the 1.5 reference
server, through the new `ClientTestFramework.ExternalServerUrl`;
- custom data types (structures, unions, optional fields, enums) loaded,
decoded and written back by each side's complex type system;
- certificate trust with auto-accept off: untrusted peers, CA-issued
certificates, revoked certificates;
- multi-chunk messages, encoding limits, response size limits, operation
limits, continuation points and security token renewal (RSA and ECC).
- Every peer check is reported as its own test result.
- `-p:LegacyStackVersion=<version>` tests another 1.5 release.
- Docs: `docs/DeveloperGuide.md`.
## 2.0 fix
**Unset optional security attributes now return
`Bad_AttributeIdInvalid`** (`NodeState.ReadNonValueAttribute`).
- Since
|
||
|
|
6b8b2eb79b |
Tag promoted NuGet package releases (#4605)
# Description
Promoted NuGet packages lacked a corresponding repository tag. The
promotion workflow now records each published candidate with an
immutable version tag and matching GitHub Release.
### Promotion tagging
- Creates a tag from the signed candidate manifest after both feeds
accept the package set.
- Creates a GitHub Release; preview channels are marked prerelease.
- Validates existing tags/releases on recovery and rejects mismatched
targets.
```text
2.0.0-preview.5 ->
|
||
|
|
b4ffc30654 |
Fix flaky DurableSubscriptionSnapshotSurvivesRetransmissionQueueRecycling (#4634)
## Problem `DurableSubscriptionSnapshotSurvivesRetransmissionQueueRecycling` failed on CI ([run 37015563079](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37015563079), test-windows-net10.0, on a PR that does not touch subscriptions) with `Assert.That(message.IsEmpty, Is.True)` → `False`. `SentMessageQueue.Clear()` calls `IPooledEncodeable.Reuse()` on each retained `NotificationMessage`, which resets it and returns it to `NotificationMessageActivator.Instance`, a process-wide pool. `SubscriptionTests` is `[Parallelizable]`, so another test could rent that instance and populate it before the assertion read it. ## Fix The queued message is a test-only `NotificationMessage` subclass whose `ReuseCore()` override counts the call and runs `ResetForReuse()` but does not return the instance to the pool. The test still checks that recycling happened (`ReuseCount == 1`, `IsEmpty`) and that the snapshot's encoded content survives. It now also checks that the snapshot holds a different reference from the queued message. The nested payloads are still recycled through the real pools. The other tests that read pooled objects after `Reuse()` (`NotificationEncodingLifetimeRegressionTests`, `PooledNotificationDispatchTests`) are already `[NonParallelizable]`. ## Testing `CustomTestTarget=net10.0 dotnet test tests/Opc.Ua.Server.Tests -f net10.0 --filter FullyQualifiedName~SubscriptionTests`: 77/77 passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
2f5aeb3da4 |
Fix Medium encoder/decoder defects found by the second codec review (#4625)
## Summary This is the follow-up to #4599. A second read-only review of `src/Opc.Ua.Types/Encoders` was run on master after #4599 merged. It found about 40 defects: none Critical or High, 10 Medium. This PR fixes the 10 Medium ones. Each fix follows the OPC 10000-6 clause named in the table, and each has regression tests. | Finding | Fix | Part 6 | |---|---|---| | **Regression from #4599:** a nested JSON array (`[[1,2]]`) decoded as an *empty* Byte/SByte array or ByteString, even with ParseStrict. Debug builds hit a `Debug.Assert` fail-fast instead. | Return false when `TryGetArrayElements` rejects the input | 5.4.5 | | A malformed `UaTypeId` was accepted as a raw JSON body with no type | Decoding error when ParseStrict is on; the raw-body fallback is kept when it is off | 5.4.2.16 | | With namespace mappings active, the Binary/XML decoders and `XmlParser` turned `nsu=…` ExpandedNodeIds into `ns=0` | Skip the index mapping when a NamespaceUri was decoded | 5.2.2.10 | | An ExtensionObject with no `Body` failed to decode when its field is in a companion namespace | `PopNamespace` before `EndField` (XmlDecoder and XmlParser) | 5.3.1.16 | | Null DiagnosticInfo/DataValue array elements were left out of the XML output, so array positions shifted | Pass `isArrayElement` so they are written as nil elements | 5.3.4 | | CR characters were lost in XML strings, and a string NodeId containing one changed | Write CR as `
`; this also works with caller-supplied writers | 5.3.1.5, XML 1.0 §2.11 | | A null element in a structure array/matrix caused a NullReferenceException in `JsonEncoder` | Write JSON `null` | 5.4.5 | | An ExtensionObject with a null ByteString/XmlElement body was written as encoding 0x01 with Length -1 | Write encoding 0x00 (no body) | 5.2.2.15 | | Default Number/Integer/UInteger structure fields threw BadEncodingError in Binary/XML. Integer arrays with values did not round-trip through XML. | `Variant.CreateDefault` returns a Variant default for these types; the XML decoders read such array fields as Variant arrays | 5.1.6 | ### Behaviour changes - Encoding a null-body ExtensionObject in binary now produces encoding `0x00`. `WriteExtensionObject_NullByteString_WritesTypeIdAndNoneEncoding` had pinned `Binary`, which contradicted its own name; it now expects `None`. - With ParseStrict, the JSON decoder rejects a present but unparsable `UaTypeId`. ### Review follow-up ( |
||
|
|
61edaa1ed0 |
Remove netstandard2.1 and net472 target frameworks (#4624)
# Description Drops support for **netstandard2.1** and **.NET Framework 4.7.2**. Libraries now target `net48`, `net8.0`, `net9.0` and `net10.0`. `Opc.Ua.Types` and `Opc.Ua.SourceGeneration.Core` still also target `netstandard2.0`, but only because the Roslyn source generators load them. There is no general netstandard2.0 support. **Preprocessor directives** (112 files) - Every `#if`/`#elif` mentioning `NETSTANDARD2_1*`, `NET472*`, `NET_STANDARD_TESTS`, `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING` or `NETSTANDARD*` was evaluated against the remaining TFM set (net48, net8/9/10, plus netstandard2.0 inside `Opc.Ua.Types`/`tools`) and simplified. Dead branches were removed. - Example: `NETSTANDARD2_1 || NET8_0_OR_GREATER` → `NET8_0_OR_GREATER`. - Example: `NET8_0_OR_GREATER || NET472_OR_GREATER` → removed (always true). - Netstandard-only code was removed: `UnixFileModeShim`, the CRDT reflection path in `ReplicatedGossipOptions.CreateReaderOptions`, and the netstandard2.1-only test variants. **Custom test logic** - `targets.props` drops the `net472`, `netstandard2.0` and `netstandard2.1` `CustomTestTarget` profiles, `NetStandardTests` and `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING`. Supported values are now `net48`, `net8.0`, `net9.0`, `net10.0`. - The `NET_STANDARD_TESTS` define is removed from 29 test projects. - The "build netstandard shells as net10.0" overrides are removed from 18 projects. - `RestrictForLegacyTfm` now only applies to `net48`. - `tests/customtest.bat` is updated to the new values. **Build / packaging / CI** - MSBuild TFM conditions are simplified: Kestrel package choice, MQTTnet v4 (net48 only), BouncyCastle, Di/OpenUsd/Vision/Stress test excludes, and `System.IO.FileSystem.AccessControl` (dropped from Core). - The MigrationAnalyzer nuspec drops its net472/netstandard2.1 lib groups. The Azure signing lists are updated. - `get-ci-matrix.ps1` removes the net472/netstandard test profiles (including the PR-gating `linux-netstandard2.1`) and the matching build legs. **Breaking API change (2.0 preview):** removed `HttpsTransportListener.IsWssTransportSupported` and `ReplicatedGossipOptions.IsGossipStateDecodingSupported`. They only returned `false` on netstandard2.1 and are now always `true`. The test guards that used them were removed as well. **Docs** - NuGet READMEs, `README.md`, `DeveloperGuide.md`, `PubSub.md`, the certificate/ECC/identity docs and the 2.0 migration guide (plus its `.agents` skill mirror) are updated. - The migration guide notes that 4.7.2 consumers must move to 4.8 and netstandard2.1 consumers to net8.0+. ### Reviewer notes - The `System.IO.FileSystem.AccessControl` `PackageVersion` pin is left in `Directory.Packages.props`. It has no direct reference any more, but transitive pinning is on, so it was kept to avoid shifting resolved versions. - The `#if` rewrite was done mechanically by a truth-table evaluator. The diff in the `.cs` files is limited to directive lines, removed dead branches, and stale-comment updates. ### Verified locally - `dotnet build UA.slnx -p:CustomTestTarget=net48`: 0 errors. - `dotnet build UA.slnx -p:CustomTestTarget=net10.0`: 0 errors. - Default multi-target builds (no `CustomTestTarget`) of Opc.Ua.Types, Bindings.Https, PubSub.Mqtt and Redundancy.Server succeeded. - net10.0 tests: - Opc.Ua.Redundancy.Server.Tests: 840/840 passed. - Opc.Ua.Types.Tests: 11363 passed, 1 skipped. - ModelDependencyScanner tests: 20/20 passed. - The full test suite was **not** run locally. ## Related Issues - None ## Checklist - [ ] 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. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
920ebda69b |
Fix transport channel and encryption audit findings (#4626)
## Summary Fixes the Critical, High and Medium findings of a read-only audit of the low-level transport channels (UA-TCP / UASC, HTTPS, WSS, Kestrel TCP, buffer management) and the encryption layer (key derivation, nonces, RSA/ECC, EncryptedSecret, security policies). Every finding was traced in source and checked against the specification (Parts 2, 4, 6, 7) before fixing. Scope: 2 Critical, 1 High, 21 Medium → **21 fixed, 3 partial**. Low findings are not part of this PR. ### Critical / High **T7-1: HTTPS session isolation (Critical).** - **Affects:** servers that expose a secured `opc.https` endpoint. - **Problem:** over HTTPS all clients share one SecureChannel (Part 6 7.4.1), so the session AuthenticationToken is the only thing that keeps sessions apart. The spec therefore requires it to be at least 32 random bytes. For secured channels the server issued tokens that are not hard to predict. - **Impact:** another client on the same HTTPS endpoint could act within a session that does not belong to it, with that session's identity and permissions. - **Fix:** `SessionManager` now always issues a random 32-byte opaque token, on every transport (Part 6 7.4.1, Part 4 7.35). **T1-1: secured client channels could lose their protection (Critical).** - **Affects:** opc.tcp clients connecting to Sign or SignAndEncrypt endpoints. - **Problem:** the client did not verify that the OpenSecureChannel response used the security policy and mode it had requested, as Part 4 5.6.2.1 and Part 6 6.7.4 require. - **Impact:** an attacker in the network path could make a secured channel continue without message security. The check on the server signature in CreateSession limits this on first connect. The session reconnect path has no equivalent check. Traffic of an existing session could then lose confidentiality and integrity without the application noticing. Other users of the channel (discovery, registration, custom service clients) were not protected at all. - **Fix:** a client channel for a secured endpoint is now bound to the requested policy and mode. A response with any other policy or mode, or without the server certificate, faults the channel, on connect and on reconnect. **T8-1: Kestrel opc.tcp listener availability (High).** - **Affects:** servers using `KestrelTcpTransportListener`. - **Problem:** the listener did not expire idle channels and did not reclaim unused ones when it reached `MaxChannelCount`, unlike the classic listener. Part 4 5.6.2.1 requires closing the oldest unused channel without a session before refusing new ones. - **Impact:** an unauthenticated peer could exhaust the channel slots and deny service to legitimate clients. - **Fix:** activity tracking, an inactivity sweep based on ChannelLifetime, and reclaiming the least recently used channel without a session at the limit. **E2-1: user token decryption errors (Medium, High where lockout is disabled).** - **Problem:** the result of a failed user identity token decryption could reveal more than Part 4 7.40.2.1 allows, which says every decryption error must look like `Bad_IdentityTokenInvalid`. - **Mitigation already in place:** the default failed-authentication lockout limits how this can be used. - **Fix:** clients now receive only `Bad_IdentityTokenInvalid`, with the detail logged on the server. Token decryption takes the same steps whichever check fails. ### Medium - **Key derivation / sequence numbers:** E1-1/T2-1 renewal XORs the previous secret only for SecureChannelEnhancements policies (Part 6 6.8.1 Step 2); T2-2 Sign-mode HKDF length uses enc key / IV length 0 (Part 6 6.8.1); T2-3 server chains a second renewal from the pending renewed token like the client; T2-4 receiver rejects sequence-number gaps (Part 2 5.1.14). - **OPN / client channel:** T1-2 (partial) OPN sender certificate checked against the policy's key family / curve and RSA key-length window (Part 4 6.1.3, Part 6 6.1); T3-1 renewal completion no longer blocks on a newer handshake while holding the gate; T3-9 reverse-connect listener bounded (1000 pending, 500/s, burst 1000). - **TCP listener / buffers:** T5-1 Basic128Rsa15 abuse tracker fails open for untracked IPs when full; T5-2 refused connections get `Bad_TcpNotEnoughResources` after the HEL (Part 6 7.1.2.3); T6-2 (partial) idle sockets no longer hold a receive buffer; T6-3 `AsyncResultBase` completes exactly once, reverse-connect timeout cancels the connect. - **HTTPS:** T7-3 chunked request bodies supported (Part 6 7.4.1); T7-4 mTLS CreateSession compares only the leaf certificate and answers a mismatch with a ServiceFault; T7-5 (partial) opcua+openapi WSS path gets the discovery-only gate; T7-6 default rate limiter partitioned per client IP; T7-7 shared Kestrel host owns its TLS certificate and rotation rebuilds it for all listeners. - **WSS / Kestrel TCP:** T8-2 SecureChannel and certificate audit events (Part 4 6.5.5); T8-3 queued senders fail with BadConnectionClosed on close instead of hanging; T8-4 WSS clients keep the mTLS client certificate alive; T8-5 WSS+JSON close bounded by timeout and cancellation. ### Partial / not fixed - T1-2: missing CertificateSignatureAlgorithm check per chain certificate, issuer key length ≥ issued key length, and checks on ECC issuers. - T6-2: `BufferManagerMemoryLimiter` still blocks the calling thread when real traffic exhausts the budget (needs an async TakeBuffer API). - T7-5: the fallback to any SecurityMode=None endpoint stays, because HTTPS listeners advertise `HttpsOpenApiTransport`. - Not covered: the HTTPS listener's WSS uacp channels likely need the same idle cleanup / reclaim as T8-1. - Not covered: the certificate/policy key check (T1-2) applies to UASC channels; the opc.https session path is a follow-up. ## Behaviour changes reviewers should look at **Wire-incompatible with older versions of this SDK (both conform to the spec now):** 1. Renewals on ECC_nistP256/P384, ECC_brainpoolP256r1/P384r1, ECC_curve25519/448 no longer mix in the previous secret; a renewal between this and an older SDK fails on the first message after renewal. 2. Sign-mode channels on those CBC+HMAC ECC policies derive different keys; Sign-mode channels to older SDKs fail. **Other:** 3. Every session AuthenticationToken is a 32-byte opaque ByteString NodeId, on every transport. 4. Any sequence-number gap faults the channel with `BadSequenceNumberInvalid` (client side previously reported `BadSecurityChecksFailed`). Only a reconnect OpenSecureChannel may skip ahead. 5. OPN peer certificates outside the policy's key-length window or with the wrong key family/curve are rejected (e.g. 4096-bit RSA on Basic256 / Basic128Rsa15). 6. Refused opc.tcp connections get an ERR after their Hello; a refused socket can stay open up to 2 s waiting for it. 7. Kestrel opc.tcp listener closes idle channels and, at MaxChannelCount, the oldest unused channel (BadNoCommunication) instead of refusing the new client. 8. HTTPS mTLS CreateSession mismatch → ServiceFault `Bad_SecurityChecksFailed` instead of HTTP 401; oversized HTTPS bodies → 413 instead of 500; default HTTPS rate limit is 100 req/s per client IP. 9. Certificate rotation on a shared (host, port) restarts the shared Kestrel host once; TLS connections on that port reconnect. 10. `EncryptedSecret.TryDecrypt/TryDecryptAsync` return false on the RSA branch instead of throwing (public API behaviour); `TryDecryptRsa` reports `Bad_SecurityChecksFailed` for padding/layout/signature failures. A KeyData whose encrypted length differs from what the policy and receiver key imply is rejected the same way, before decryption. 11. Client reverse-connect listener limits pending connections and connection rate; at its cap the oldest connection that has not sent a ReverseHello is closed to make room. On opc.wss the cap applies only to connections not yet handed off. 12. `ApplicationInstance` creates its own RSA application certificate with at least 2048 bits (except the RsaMin type), even when `MinimumCertificateKeySize` is lower; that setting only bounds accepted peers. 13. Kestrel opc.tcp listener: `ChannelLifetime` <= 0 disables the idle sweep; otherwise the sweep runs at most once per second. 14. Certificate rotation on a shared Kestrel host closes the listener's WebSocket connections and outbound reverse-connect channels even when the TLS certificate is unchanged; if the new host fails to start, a host with the previous TLS certificate is restored and the error is rethrown. Listeners on the same (host, port) share a Kestrel host only when their host-level settings (mutual TLS, admission limits, middleware) match; otherwise the listener uses its own host, as for a TLS certificate mismatch. 15. On a second renewal before the first renewed token was used, the server makes the first renewed token current (raising `TokenActivated` then) instead of dropping it; a reconnect chains from the current token. 16. Sign-only HKDF channels report empty IV/key plus the signing key length in channel diagnostics and the Wireshark keyset. 17. `UserNameIdentityTokenHandler.DecryptAsync` throws the specific RSAEncryptedSecret failure; the server still answers clients with a plain `Bad_IdentityTokenInvalid` and logs the cause. 18. Refused ReverseHello messages up to the maximum valid size also get the `Bad_TcpNotEnoughResources` reply; `TcpByteTransport` rents receive buffers at the chunk size. ## Tests New regression tests for every Critical/High fix fail before the fix. Tests that pinned old behaviour were updated: `TcpReconnectKeyAgreementTests`, `UaSCBinaryChannelSequenceRolloverTests`, `TcpMessageFailureRegressionTests`, `TcpTransportListenerDeterministicTests`. Combined branch, net10.0: | Suite | Passed | Skipped | Failed | |---|---|---|---| | Opc.Ua.Core.Tests | 6036 | 88 | 0 | | Opc.Ua.Bindings.Https.WebApi.Tests | 405 | 0 | 0 | | Opc.Ua.Server.Tests | 7806 | 5 | 0 | | Opc.Ua.Client.Tests | 2733 | 0 | 0 | | Opc.Ua.Gds.Tests | 1234 | 56 | 0 | | Opc.Ua.Sessions.Tests | 790 | 379 | 0 | net48 builds of the Core, Server, Client and WebApi test projects and the netstandard2.1 Release build: 0 errors, no new warnings. One telemetry test (`ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly`) failed once under parallel load and passes on rerun. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
c4e99c6e3a |
Trust leaf-only certificate chains from configured issuers (#4604)
# Description Leaf-only peer certificates failed validation when their intermediate and root CAs were configured in `TrustedIssuerCertificates`. The issuer walk built the chain but did not treat those configured issuers as trust anchors. - **Validation** - Mark issuers resolved from `TrustedIssuerCertificates` as trusted. - Preserve rejection of chains containing only peer-supplied, unconfigured issuers. - **Regression coverage** - Update explicit issuer-chain validation for Peers, Users, and HTTPS scopes. ## Related Issues ## Checklist - [ ] 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. - [ ] 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. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4600 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> |
||
|
|
029ac0f33f |
Kubernetes readiness: create the listener on Start and move the default probe port off 8080 (breaking default) (#4613)
# Description Two commits that can be taken separately. 1. **`fix(kubernetes): create the readiness listener only when started`** (not breaking). The managed HttpListener (Linux, macOS) binds its endpoint when a listener that has prefixes is closed. Because `KubernetesReadinessServer` created the listener in its constructor, disposing a server that was never started bound the probe port and threw "address already in use" if anything else held it. The listener is now created in `Start`, and `Start` after `DisposeAsync` throws `ObjectDisposedException` explicitly. With port 8080 occupied, master fails 4 of 99 tests in `Opc.Ua.Redundancy.Kubernetes.Tests`; with this commit all pass. New test: `DisposingAnUnstartedServerDoesNotBindItsPort`. 2. **`fix(kubernetes)!: move the readiness probe off the web-application port 8080`** — **breaking (default only):** `KubernetesReadinessOptions.Port` now defaults to the new `KubernetesReadinessOptions.DefaultPort` = 4852 instead of 8080. - 8080 is the de-facto default of web applications and reverse proxies, and the probe shares the pod (with hostNetwork, the node) with the application and its sidecars. - 4852 sits next to the OPC UA ports 4840/4843 and is unassigned in the IANA registry (4852–4866, checked 2026-09-30). One de-facto use we found: Beckhoff IPCs run an "IPC Diagnostics" OPC UA server on 4852 on Windows images; that only matters with hostNetwork on such a node. If you prefer another port from the unassigned range, commit 1 stands on its own. - Migration: set the manifest `containerPort` to 4852, or keep the old port with `UseKubernetesReadiness(o => o.Port = 8080)`. `docs/Kubernetes.md`, `NugetREADME.md` and a new `docs/MigrationGuide.md` section are updated. - The package has shipped only in 2.0.0 previews, so changing the default before GA avoids locking the collision in. Tests: `Opc.Ua.Redundancy.Kubernetes.Tests` 100/100 on net10.0 and net8.0. Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run 36769864949](https://github.com/biancode/UA-.NETStandard/actions/runs/36769864949) (full matrix incl. macOS), [CodeQL run 36769869626](https://github.com/biancode/UA-.NETStandard/actions/runs/36769869626). ## Related Issues - Fixes #4612 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
|
2e91ba36cd |
Report JobOrderList and JobOrderResponseList changes to monitored items (#4607)
# Description `JobOrderList` and `JobOrderResponseList` serve their value from a snapshot that the job-control change streams refresh. The refresh replaced the snapshot but never marked the variable as changed, so a client subscribed to a list received the first value and then nothing and had to poll. The refresh now stamps a new source timestamp on the list variable while it holds the refresh lock (the timestamp setter sets the Value change mask) and calls `ClearChangeMasks` after releasing it — the monitored items' change handlers read the list back through the read callback, which takes the same lock. A refresh that loses the generation check reports nothing, so an older snapshot never overwrites a newer notification. - `Isa95NodeManager.RefreshJobOrderListsAsync`: V1 and V2 `JobOrderList`. - `JobManagementBuilder` (`Opc.Ua.Machinery.Server`): `JobOrderList` and `JobOrderResponseList`. Both tests listen on the notification path monitored items use (`NodeState.StateChanged` with the Value mask) and fail without the change: - `MachineryPartsBuilderTests.TheJobListsReportChangesToMonitoredItemsAsync`: storing an order and receiving a response are both reported. - `Isa95EndToEndTests.ServerFixtureAndTypedClientExerciseIsa95Async`: the existing external-update step now also asserts that the change reaches `JobOrderList`; it used to verify only with repeated reads, which is why this went unnoticed. Local results on the branch: `UA.slnx` Release builds on all target frameworks with no new warnings; `Opc.Ua.ISA95.Tests` 144/144 and `Opc.Ua.Machinery.Tests` 163/163 on net10.0 and net8.0; changed-line coverage 100 % (`check-coverage.ps1`). Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run 36752366310](https://github.com/biancode/UA-.NETStandard/actions/runs/36752366310) (189 jobs incl. macOS), [CodeQL run 36752372259](https://github.com/biancode/UA-.NETStandard/actions/runs/36752372259). ## Related Issues - Fixes #4606 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
|
f20d94fcac |
Robotics: resolve client reads with namespaces and ParameterSet; call ResetToProgramStart on the ReadySubstateMachine (#4609)
# Description `Opc.Ua.Robotics.Client` read almost nothing from a real OPC 40010-1 server. Every member was looked up with a namespace-0 browse name, and process values were looked for directly below the component instead of below the DI `ParameterSet`. #4569 made motors, gears and drives resolve; the values, identification and enumerations of every snapshot still came back empty, and the Observe methods tried to monitor `NodeId.Null`. Separately, `ResetToProgramStartAsync` called the method on the `Ready` state, which has no such method. This is the minimal fix on the current structure; the generated proxies, the per-node reads, the motor and gear lookup by type and the IsDrivenBy drive lookup with its de-duplication stay as they are. The public API is unchanged. - **Reads:** the resolve helpers take a `QualifiedName[]` path built from the session's namespace table — DI browse names for identification properties, Robotics browse names for members of the component, and `DI:ParameterSet/<Robotics name>` for process values (ActualPosition/Speed/Acceleration, SpeedOverride, EmergencyStop, OperationalMode, ProtectiveStop, ExecutionMode, TaskProgramLoaded, TaskProgramName). A namespace the server does not publish makes the path not resolve instead of matching namespace 0. - **Observe methods:** `ObserveAxisAsync` and `ObserveSafetyAsync` subscribe only to values that resolve, and end when none do. - **ResetToProgramStart:** called on the TaskControlStateMachine's `ReadySubstateMachine`; if it is missing, the call fails with Bad_NotFound. - `docs/Robotics.md` describes how the snapshot reads map to the OPC 40010-1 layout. Tests: - `RoboticsClientServerE2eTests` (new, all target frameworks): a six-axis cell built with the Robotics builders and hosted over opc.tcp. It checks the `ReadSystemAsync` values, that `ObserveAxisAsync` delivers changing positions, and that the server's `ResetToProgramStart` handler runs. All three fail on master. - `RoboticsClientFacadeTests`: the scripted session now matches browse paths by the full QualifiedName, lays nodes out as the NodeSet does, and adds namespace-0 and wrong-level decoys; two new tests check the object ResetToProgramStart is called on and the Bad_NotFound case. Local results on the branch: `UA.slnx` Release builds on all target frameworks with no new warnings; `Opc.Ua.Robotics.Tests` 641/641 on net10.0 and 581/581 on net8.0; changed-line coverage 97.9 % (`check-coverage.ps1`). Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run 36755707494](https://github.com/biancode/UA-.NETStandard/actions/runs/36755707494) (189 jobs incl. macOS), [CodeQL run 36755711056](https://github.com/biancode/UA-.NETStandard/actions/runs/36755711056). Possible follow-ups, not part of this PR: reading the topology level by level in batches (one TranslateBrowsePathsToNodeIds, Browse and Read per level) and filling the axis engineering units and limits, the load mass unit and range, and `GearRatio`, which the snapshot contracts define but the client does not read yet. ## Related Issues - Fixes #4608 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
|
e0ab811c03 |
Scales and Machinery: advertise only the facets and units the server satisfies (#4615)
# Description The Scales and Machinery node managers publish their facets and conformance units through `IConformanceContributor`. This aligns what they advertise with the published tables. **Machinery** (`fix(machinery): advertise the ProcessIrregularity job result unit`) - Adds *Machinery Job Management Result ProcessIrregularity* to the units reported for `WithPredefinedParameters()`. OPC 40001-3 Table 24 lists it, the Jobs NodeSet carries it as a `<Category>`, and `MachineryJobParameters` already recognises the parameter. - New test `PredefinedParametersAdvertiseEveryParameterUnitOfTheBaseFacetAsync` checks all Table 24 parameter units. **Scales** (`fix(scales): advertise only the facets the runtime satisfies`) - Base Scale (and every kind facet), Scale System, Feeder Module and Printer Module include the Machinery Machine Identification facet (OPC 40200 Tables 164 to 167). While every published scale and scale system is organized into the `Machines` folder, the server now also advertises that facet and its mandatory units *Machinery Find Machines* and *Machinery Machine Identification*. When a scale is not in the folder (`OrganizeIntoMachinesFolder = false`, or the folder is missing) those facets are no longer advertised. Production preset and SI unit facets do not include Machine Identification and are unaffected. - The Totalizing Hopper, Catchweigher and AutomaticWeightPriceLabeler kind facets and units are no longer claimed: no controller drives them yet (known limitations in `docs/Scales.md`). The scales still count towards Base Scale. - *PackML State Information* (Table 164, optional) is advertised when a scale or scale system runs a PackML state machine, qualified with the PackML namespace as the table writes it. Machinery units use namespace 0, as `MachineryNodeManager` reports them. - `docs/Scales.md` updated (Machines folder, options table, conformance matrix, known limitations). - New tests: `ScaleFacetsBringTheMachineIdentificationTheyIncludeAsync`, `ScaleFacetsAreNotClaimedOutsideTheMachinesFolderAsync`, `KindFacetsWithoutAControllerAreNotClaimedAsync`, `PackMLStateInformationIsClaimedWithAPackMLStateMachineAsync`. Each new test fails against master and passes with the change. Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run 36779541067](https://github.com/biancode/UA-.NETStandard/actions/runs/36779541067), [CodeQL run 36779544537](https://github.com/biancode/UA-.NETStandard/actions/runs/36779544537). ## Related Issues - Fixes #4614 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. |
||
|
|
c189cd6fcb |
Include subtypes of AliasNameCategoryType in AliasNameClient.EnumerateSubCategoriesAsync (#4611)
# Description `AliasNameClient.EnumerateSubCategoriesAsync` kept a browsed child only when its TypeDefinition was exactly `AliasNameCategoryType`. OPC 10000-17 §6.3.1 declares `<SubAliasNameCategories>` as an OptionalPlaceholder of `AliasNameCategoryType` and does not rule out subtypes, so sub-categories modelled with a subtype were silently dropped. The exact type is still accepted without a round trip; any other type definition is checked against the type hierarchy with `Session.NodeCache.IsTypeOfAsync` (the pattern `RoleManagementClient` uses for `RoleType`). The node cache keeps the type references, so each distinct type is resolved once. The check runs on the first browse page and on every BrowseNext page. `docs/AliasNames.md` notes that subtypes are included. Tests: the mocked session harness gets a node cache that answers `IsTypeOfAsync` from a subtype map; a new test covers the exact type, a direct subtype, a subtype of a subtype, a FolderType child and an untyped child. It fails on master (`Missing: < "Vendor", "Derived" >`) and passes with the fix. `Opc.Ua.Client.Tests` 2733/2733 on net10.0 and net8.0; `Opc.Ua.Features.Tests` AliasNameIntegrationTests (real server) 7/7. Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run 36769850776](https://github.com/biancode/UA-.NETStandard/actions/runs/36769850776) (full matrix incl. macOS), [CodeQL run 36769855329](https://github.com/biancode/UA-.NETStandard/actions/runs/36769855329). ## Related Issues - Fixes #4610 ## Checklist - [x] 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
|
ce88955f70 |
Harden encoders/decoders against untrusted input and align them with Part 6 / Part 14 (#4599)
# Description
Hardens the encoders and decoders against untrusted input and aligns
them with OPC 10000-6 (1.05) and OPC 10000-14. This comes from an audit
of every codec reachable from a peer:
- Binary, JSON and XML decoders and encoders;
- built-in and complex types;
- UA-TCP/UASC framing;
- PubSub UADP and PubSub JSON.
The audit found 97 issues: 1 critical, 7 high, 39 medium and 50 low.
They are all fixed here, one commit per finding (IDs B1-, J1-, X1-, T1-,
S1-, U1-, P1-, E1-, E2- in the commit subjects). Every fix was then
checked against the spec a second time, and the deviations that re-check
found were fixed as well (`*-spec` commits).
## Security (decoders reachable from unauthenticated peers)
- **Allocation from length prefixes (B1-1, B1-5, U1-1, U1-3).**
- Array, string and byte-string lengths are checked against the bytes
left in the message before anything is allocated.
- Arrays read element by element grow as elements are decoded, so nested
length prefixes can no longer multiply memory.
- UADP discovery counts and chunk reassembly buffers are bounded the
same way.
- **Stack exhaustion.** Nesting is checked for remaining stack
(`RuntimeHelpers.EnsureSufficientExecutionStack` mapped to
`BadEncodingLimitsExceeded`), and the large Variant read/write methods
were split into scalar, array and matrix methods. Measured on 1 MB and
256 KB threads, no codec overflows at the default limits any more.
- **XML element depth (X1-1).** XmlElement values and raw
ExtensionObject bodies are copied node by node with a depth limit,
instead of with the recursive `InnerXml`, `ImportNode`, `InnerText` and
`DeepEquals` calls.
- **Limit bypasses:**
- JSON NodeId, QualifiedName and opaque strings now respect
MaxStringLength and MaxByteStringLength (J1-1).
- Matrix rank is limited to 32.
- Decimal size is bounded (T1-3).
- XML base64 is checked before it is decoded.
- XML arrays are checked before each element is read.
- **CPU and algorithmic cost:**
- UADP reassembly uses an O(1) overlap check with a per-message chunk
cap (U1-2).
- PubSub JSON matches fields to metadata through a name index (P1-2).
- LocalizedText "mul" text is parsed lazily (T1-5).
- Unknown SecurityTokenIds trigger a throttled background SKS refresh
instead of an inline call per datagram (U1-9).
- **PubSub metadata poisoning (P1-1).** Inbound metadata is registered
only for identities a configured reader expects, is capped, and an
unauthenticated message can no longer pin a version.
- **Exception contract.** Decoders fail with `ServiceResultException`
(BadDecodingError / BadEncodingLimitsExceeded) instead of letting
ArgumentException, InvalidOperationException, TypeLoadException and
similar escape.
- **UASC (S1-*):**
- OPN and CLO must be single final chunks.
- The certificate of an unsigned OPN is ignored.
- The padding and ExtraPadding bytes are validated.
- A renewed token is activated only after a chunk has been verified with
it.
- A policy change on renewal is rejected before the channel certificate
is touched.
- Secured chunk length arithmetic is bounded.
## Wire-visible behaviour changes
All follow the spec exactly; there is no reader for the old forms unless
noted.
- **UA Binary:**
- Variant type ids 26-31 decode as ByteString.
- Picoseconds without their timestamp are ignored, and values of 10000
and above are clamped to 9999.
- A Null-type Variant with the array flag consumes its length and
dimensions.
- **UA JSON:**
- Messages are ExtensionObjects with the body inline; there is no
`UaBody` envelope any more (5.4.9 / 5.4.2.16).
- The Variant `Value` is omitted exactly for NULLs of nullable types
(Table 41).
- RawData keeps nested UaType/UaTypeId and the StatusCode Symbol.
- Duplicate member names are rejected.
- `{}` decodes as a null ExtensionObject.
- Numbers and identifiers are parsed strictly and culture-independently.
- QualifiedName follows 5.4.2.14.
- Opaque bodies of known types carry the DataType id.
- Enumerations without a literal are written as the number.
- **UA XML:**
- The earliest DateTime is written as `0001-01-01T00:00:00Z`.
- Null typed scalars and null array elements are written with `xsi:nil`.
- An ExtensionObject without a body has no `Body` element.
- Strings XML cannot represent are rejected.
- Only the xs:dateTime and xs:date forms are accepted; a value without a
zone is still read as UTC, because published model files use it.
- Only the Part 6 Guid form is accepted.
- Booleans are case-sensitive in XmlDecoder.
- **UA-TCP:**
- OPNC/OPNA/CLOC/CLOA fault the channel. Older releases that split a
long-chain OPN into chunks are rejected, so this needs a release note.
- The sender truncates the certificate chain to fit a single chunk.
- The received certificate chain is sized from the chunk size.
- Hello buffer sizes down to 1024 bytes are honoured; below that, an ERR
with Bad_TcpInternalError is sent.
- A 4096-byte Error reason is accepted, and the SecurityPolicyUri limit
is 255 bytes.
- **PubSub UADP:**
- Chunk NetworkMessages use the Part 14 7.2.4.4.4 layout, and each chunk
is secured on its own with its own sequence number.
- The Sizes array is part of the secured payload.
- Padded RawData keeps its length prefix, and null values are written as
-1.
- DataSetMessages with the valid bit clear are skipped.
- Null discovery arrays are accepted.
- **PubSub JSON:**
- Variants use `UaType`/`Value`, with the 7.2.5.4.2 top-level collapse.
The deprecated `Type`/`Body` form is still read.
- Status is written as a `{Code,Symbol}` object; a numeric Status is
still accepted.
- Discovery messages follow Tables 188-192, with GUID MessageIds.
- Enumeration fields are written as `Name_Value`.
- Fields appended with a MinorVersion bump are accepted.
## Known deviations and limitations (documented, not changed)
- A known type whose ExtensionObject XML body fails to decode fails the
message, like a failing binary body. 5.2.2.15 only says the decoder
*should* continue, and continuing reopens the log-flooding path.
- Reserved Variant type ids 26-31 are passed to the application as
ByteString without the raw type id.
- JSON UaTypeId of an opaque body of an *unknown* type stays the
encoding id, because no type system is available to map it.
- PubSub JSON encoder:
- Structure fields without a subtype are still wrapped with a UaType,
and concrete multi-dimensional or abstract-rank fields still carry a
UaType (Table 186).
- Namespace-0 enumerations without EnumDataTypes are written as a
numeric string.
- Chunked transcoded frames have no writer group, so their sequence
numbers continue from the source number.
- Pre-existing and out of scope: the HTTPS JSON binding uses
Content-Type `application/opcua+uajson`, while 1.05 7.4.5 requires
`application/json`.
## Tests
- Every fix has a regression test.
- Tests that pinned non-conformant behaviour were updated, and their
commit messages say so.
- Fuzz seeds were regenerated where the wire changed.
Local results:
net10.0, 27 suites green (Types 11354, Core.Encoders 2933, Encoders.Fuzz
8112, Core 5955 + 1 known intermittent ActivityTraceFlags metrics test
that passes alone, Client 2732, Server 7802, Sessions 790, GDS 1234,
Subscriptions 641, SourceGeneration.Core 4266, PubSub 1490 and all
PubSub transport suites). Full UA.slnx build 0 warnings / 0 errors on
net10.0 and net48.
## Related Issues
- Follow-up: #4598 (possible throughput regression of per-element binary
array reads).
## Checklist
- [x] 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.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. (net10.0
suites run; net48 build only)
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
dadd609ec0 |
Scales: the OPC 40200 companion SDK with the OPC 30050 PackML model (#4580)
> Tracking issue #4576. #4577, #4578 and #4579 are merged, so this branch is > rebased onto `master` and contains only the Scales and PackML commits. ## Summary Last of the four PRs of #4576. Adds OPC 40200 Weighing Technology 2.00 as a model package plus a server and a client SDK, and the OPC 30050 PackML model it composes. | Package | Content | |---|---| | `Opc.Ua.PackML` | OPC 30050 PackML 1.01, plus `PackMLStateMachineController`, which drives a base state machine together with its nested machine and execute state machines (activation and deactivation of the sub-state machines, the PackML methods with an `Executable` attribute that follows the state, acting states left automatically or by the equipment) | | `Opc.Ua.Scales` | OPC 40200 2.00 (`http://opcfoundation.org/UA/Scales/V2/`), generated from the unmodified UA-Nodeset publication, plus the contracts client and server share | | `Opc.Ua.Scales.Server` | `AddScales()` / `ConfigureScales(...)`; all fourteen scale kinds and scale systems below `DeviceSet` and `Machines`; a runtime that does the weighing arithmetic (ranges, rounding to d or e, zero band, overload/underload, zero, tare, preset tare, registration), production presets with DI locking, statistics, recipes and recipe files, Annex C events and alarms over a `HasNotifier` hierarchy, and the methods of every scale type | | `Opc.Ua.Scales.Client` | discovery, weight reads and streaming, zero/tare/register, products, PackML state and commands, type-specific methods, events and alarms, snapshots | `docs/Scales.md` includes a conformance matrix against the 21 OPC 40200 server facets with test evidence, the places where the specification and the NodeSet disagree, and known limitations. ## Review follow-ups Separate commits after the docs/samples commit, each with a test that fails without it where the defect can be observed. - Copilot review: `CurrentProductsChanged` fires only when the set of current products changes; the new public collection APIs (`ScalesServerOptions.AdditionalNamespaceUris`, `ScaleNamespaceIndices.SearchOrder`, `ScalesModel.AllKinds`) are `ArrayOf<T>`. - Own review with the rubric in `.github/copilot-instructions.md`: - PackML raises `StateChanged` after its lock is released (queued, in order), reads `StartParameters` under the lock, and reports the handler's status when an acting state cannot complete. - Scales: concurrent first creations share one runtime (the lock service refused a second attach); two creations of the same name cannot both publish; a removed product id stays reserved until its node is gone; the zone, target-count and vehicle handlers change products under the preset lock; the product factory and `configure` delegate run outside it; the recipe upload reads `ActiveRecipe` under the lock; resetting the statistics also resets the tolerance percentage. - The client lets a failed browse call propagate instead of reading it as "no children". ## Testing - `tests/Opc.Ua.Scales.Tests`: 115 tests on net8 and net10 and with the netstandard2.1 libraries, including the PackML runtime; the client/server tests connect a real session over `opc.tcp` to a hosted `ScalesNodeManager`. - `UA.slnx` builds for net472, net48, netstandard2.0/2.1 and net8/9/10 with SDK 10.0.401 and no new warnings. - Changed-line coverage of this layer (`check-coverage.ps1`): 94.59 % (4585 of 4847 changed lines, against `master`). - Fork CI, full PR matrix incl. macOS: in progress, [run 36676067489](https://github.com/biancode/UA-.NETStandard/actions/runs/36676067489); CodeQL: in progress, [run 36676070427](https://github.com/biancode/UA-.NETStandard/actions/runs/36676070427). - On the current `master` (with Pumps merged), `UA.slnx` builds for every target framework and the Scales and Pumps tests pass. - Beyond the in-repo tests the implementation has been validated end to end against a separate, non-public OPC UA client and server. ## Reviewing Commits: PackML → Scales model → server → client → tests → docs and samples → review follow-ups. |
||
|
|
0b9af4db34 |
Fix flaky BuilderDisposesOwnedHttpProviderAndChannelManagerAsync on net48 (#4597)
## Problem `ManagedSessionResourceOwnershipTests.BuilderDisposesOwnedHttpProviderAndChannelManagerAsync` failed on net48 in [run 36598770373](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36598770373) (job `test-windows-net48 (5/30)`, on PR #4595, which only touches `version.json`). It failed after 12.1 s with a `TaskCanceledException`. This was not a disposal problem. The TRX shows that `ConnectAsync` never finished: - The shared channel entered `TransportConnecting` and never reached `Ready`. - After 10 s the test's connect deadline cancelled `ConnectionStateMachine.WaitForConnectedAsync`, with `ClientChannelManager.GetCoreAsync` still waiting on the open. - The open was still running after the shutdown token was cancelled, so it was stuck on something that ignores cancellation. - On passing runs this test takes about 0.4 s on CI. ## Root cause While opening, `WebApiTransportChannel` sends a sessionless `GetEndpoints` to fill in the endpoint description. That request goes to the test's scripted `HttpMessageHandler`. On .NET Framework the handler read the request body with the tokenless `ReadAsByteArrayAsync()`. For a `ByteArrayContent` this copies the bytes through `Stream.BeginWrite`, which runs on a thread-pool thread. On a starved runner the read can therefore stall indefinitely. Because it cannot be cancelled, it outlasts both the channel's own 6 s `GetEndpoints` timeout and the test's 10 s deadline. On net8+ the handler already passes the token. ## Proof - **Non-cancellable 12 s delay** injected at that point in the handler: reproduces the CI failure exactly. The channel sits in `TransportConnecting` for 10 s, then `TaskCanceledException` is thrown from `WaitForConnectedAsync`. - **Cancellable 12 s delay** at the same point: the channel's 6 s `GetEndpoints` timeout cuts it off, the channel opens and the test passes. - **Probe on net48:** `ReadAsByteArrayAsync()` on the request content never completes synchronously, while `ReadAsStreamAsync()` on a `ByteArrayContent` always does. The product's own response reading (`HttpResponseBodyReader`) already uses `ReadAsStreamAsync` and passes the token, so only the test handler is affected. ## Fix On .NET Framework the handler now reads the content stream, which `ByteArrayContent` creates synchronously, and copies it in memory. There is no thread-pool hop left in the handler. ## Testing `ManagedSessionResourceOwnershipTests` passed 15 of 15 runs on net48 and 15 of 15 on net10.0. Before the fix, the test also passed 150 repeats locally, including under full CPU load. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> |