mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-28 05:57:47 +08:00
aas
14
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
33ed3028e6 |
Fix macOS test host crash caused by indefinite keepAlive worker await in session teardown (#4319)
`test-macOS-latest-History` CI job crashes after all 293 tests pass. All
tests complete successfully but the test host exits with code 1 ("Test
host process crashed") ~35 seconds into teardown.
**Root cause chain:**
- `Session.StopKeepAliveTimerAsync` unconditionally `await`s
`keepAliveWorker`, which is blocked in `ReadAsync`. On macOS under CI
load, socket reads do not honor `CancellationToken`, causing session
teardown to hang indefinitely.
- After 15 s, `TestFixture.OneTimeTearDown`'s watchdog fires, sets
`s_skipRemainingTeardowns = true`, and bails out—without calling
`ServerFixture.StopAsync()`, leaving certificate managers un-disposed.
- Certificate leak assertions fire in `GlobalTeardown`, triggering
`ArmProcessExitWatchdog` → `Environment.Exit(0)` on a background thread
severs the VSTest pipe → "Test host process crashed", exit code 1.
## Changes
**`src/Opc.Ua.Client/Session/Session.cs`**
- `StopKeepAliveTimerAsync`: replace unconditional `await
keepAliveWorker` with a 5-second bounded `Task.WhenAny`. If the worker
doesn't stop in time, log a warning and abandon it. Well-behaved
transports cancel immediately; the bound guards against unresponsive
macOS sockets.
- Added `s_keepAliveStopTimeout` static field (5 s) and
`KeepAliveWorkerDidNotStopWithinTimeout` source-generated log message
(`Session+69`).
```csharp
// Before
await keepAliveWorker.ConfigureAwait(false);
// After
var completed = await Task.WhenAny(keepAliveWorker,
Task.Delay(s_keepAliveStopTimeout)).ConfigureAwait(false);
if (completed != keepAliveWorker)
{
SessionLog.KeepAliveWorkerDidNotStopWithinTimeout(m_logger, s_keepAliveStopTimeout);
}
```
**`tests/Opc.Ua.Client.TestFramework/TestFixture.cs`**
- `OneTimeTearDown`: both the early-skip and post-session-close skip
paths now call `ServerFixture.StopAsync()` before nulling the reference.
`StopAsync` fast-paths when `s_skipRemainingTeardowns` is set—disposes
certificate managers without blocking on server shutdown—breaking the
leak→crash cascade.
## 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.
- [ ] 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.
- [ ] 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: romanett <romanett98@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
||
|
Marc Schier
|
06ae192084 |
Fix CI failures tracked by #4299 (#4308)
## Summary - stabilize the History, authentication lockout, Robotics, redundancy, and DI tests reported in #4301-#4305 - fix the closed-transport buffered-write completion deadlock that hung `ConnectMultipleSessionsAsync` - add macOS crash diagnostics for #4306 and retain evidence for root-cause analysis - run macOS tests and NativeAOT in GitHub Actions pull requests, using supported Intel and ARM64 runners, while keeping every Azure macOS definition disabled by default - move all remaining source-level `InternalsVisibleTo` declarations into project files so signed OpenUSD builds receive the repository public key ## Local validation - MCP PubSub: 29 passed - History focused tests: 3 passed - Sessions lockout: 3 passed - Sessions multiple-session regression: 3 passed - deterministic binary client channel tests: 22 passed - Robotics runtime: 34 passed - Redundancy focused tests: 4 passed - OpenUSD net10.0: 914 passed - affected OpenUSD projects build on net10.0 and net48 Refs #4299, #4300, #4306 Fixes #4301 Fixes #4302 Fixes #4303 Fixes #4304 Fixes #4305 --------- Copilot-Session: 7cb486de-8b4c-43c5-a900-e7d2e0ad04a8 |
||
|
|
aad8350bc1 |
Pluggable symmetric cryptography, an awaitable asymmetric path, registrable security policies, and PubSub crypto offboarding (#4206, #4207, #4208, #4210) (#4232)
# Description Closes the three crypto-offboarding follow-ups split out of #4192. Each was recorded there as knowingly *not* done, with the reason and what would justify revisiting it. Those conditions are met, and where one of them turned out to be unachievable that is written down rather than worked around. The safety property throughout: **every seam is opt-in and null-checked, and in the default software configuration the added code is observably inert.** No interface dispatch on the per-message path, and the asynchronous paths complete synchronously — so channel locking order, buffer ownership and error sequencing are unchanged for a deployment that configures nothing. ## #4207 — symmetric, key-derivation and RNG provider seams `ISymmetricCryptoProvider`, `IKeyDerivationProvider` and `ISecureRandomSource`, added on the terms #4207 set: only once a concrete requirement arrived (a validated module that must perform *every* operation, not just the asymmetric ones), behind a fast path, with `SymmetricChannelCryptoBenchmarks` as the gate. They are **facets discovered by type test**, not members of `ICryptoProvider`, so providers written against the shipped interface still compile. `PlatformCryptoProvider` implements all three, which answers the "an interface nothing implements" objection recorded in the issue. `CryptoProviderFacets` returns `null` both when no registry is configured *and* when resolution lands on the platform, because the platform facets are the inline code. Resolution happens once, in `CalculateSymmetricKeySizes`, and is held for the life of the channel. `CryptoCompliance.GetUnservedOperationPurposes` closes the failure the seam would otherwise introduce: a provider bound to `ChannelSymmetric` without implementing the facet is silently replaced by the platform, so a deployment could believe its validated module performed the per-message cryptography while the platform did. Under `FipsOnly` that now refuses to start. ## #4208 — the asymmetric path is awaitable `RSA` and `ECDsa` are .NET's contracts and cannot be replaced without making every ready-made hardware and cloud implementation unusable. Instead an implementation may **also** declare `IAsyncRsaKey` / `IAsyncEcdsaKey`, which the stack finds by type test. Now asynchronous: the secure channel open and renew path, user identity token signing and decryption, and session activation. A software key declares neither facet, so those paths complete synchronously and nothing about their ordering changes. `ReadAsymmetricMessage` is refactored into shared helpers rather than duplicated — adding an `await` was not worth two hundred cloned lines of signature, padding and endpoint-selection logic. `AsymmetricMessage` / `AsymmetricWriteResult` exist because an asynchronous method cannot have `out` parameters. The channel no longer serialises its state on a monitor. A monitor cannot be held across an `await`, and a plain `SemaphoreSlim` was not an option at the time: the code relied on re-entrancy in eight places (`HandleIncomingMessage` holds the lock and calls `ForceChannelFault`, which takes it again; `ForceReconnect` and `Shutdown` do the same). `ChannelGate` replaces it at 32 sites and `DataLock` is `[Obsolete]`. That reliance has since been removed as well — see [Removing the gate's re-entrancy](#removing-the-gates-re-entrancy) below. ### Defects this surfaced, all fixed and regression-tested **Detached work started inline.** Writes were started with `_ = WriteBuffersAsync(...)`. An async method runs its prologue on the caller's stack, so `LeaveInheritedContext` — which exists to stop *detached* work inheriting the right to re-enter the gate — was stripping the **caller's own** entitlement; when the send then completed synchronously, the completion blocked on a gate that very thread was holding. The client sent its OpenSecureChannel request and never processed the response. Writes are now queued. **A synchronous gate handle held across an `await`.** `Enter()` records the acquiring thread so an inline completion callback can re-enter — `ChannelAsyncOperation` invokes its callback both inline and detached. That record is only sound while the holder is synchronously on that thread; the token renewal path held it across an await, so a reused pool thread was recognised as the holder. **`SignAsync` rejected the policy that signs nothing.** It validated the certificate before the algorithm, so `SecurityPolicies.None` — which carries no certificate — threw where the synchronous `Sign` returns `null`. **`EnterAsync` lost the holder when contended** *(found in review)*. The `AsyncLocal` recording ownership was written inside the awaiting method, and .NET restores the caller's execution context when an async method completes, so the write was discarded. The caller returned owning the semaphore while `IsHeldByCurrentContext` reported `false` — and any nested `Enter()` then blocked forever on a gate its own context held. Reachable remotely: `HandleIncomingMessageAsync` → error path → `ForceChannelFault`. The holder is now published from the caller's frame before the wait, and only its depth is raised in the continuation. **Queued writes lost their order** *(found in review)*. Sequence numbers are assigned under the gate, but independently queued work items reach the transport's FIFO send lock in arbitrary order, so a peer could reject a chunk with `BadSequenceNumberInvalid`. Writes are now appended to a chain: still off the caller's stack, but a write cannot start before its predecessor finishes. ### Four more this surfaced in CI, on paths only the integration suites reach Queuing the writes to fix the deadlock above turned out to have moved three implicit guarantees, and the gate had one more liveness bug. Each was reproduced locally against a clean baseline before it was changed. **A faulting channel's error message never reached the peer.** A listener channel sends a UASC `Error` describing why it faulted and closes the transport as its next statement. That worked only because the write started on the caller's stack, so the bytes reached the socket first; queued, the close discarded them and the client saw `BadConnectionClosed` instead of the certificate status it was told to expect. Nine `SecurityCertValidationTests` cases assert exactly that status. The terminal error is written inline again, on a path that touches the gate only to disclaim what it inherited and reports nothing — a channel that is already faulted is owed nothing beyond its buffer. **The certificate of a rejected sender leaked.** An asynchronous method cannot have `out` parameters, so the asymmetric read returns a record instead. That silently dropped an ownership guarantee: an `out` parameter reaches the caller even when the method goes on to throw, so the caller's `catch` disposed the sender certificate and reported it to the audit. A return value does not — nine leaked certificates, one per rejection case, which the leak detector fails the run on. The parsed certificate is now handed to the caller before anything that can reject the message, restoring both the disposal and the audit. **The gate could strand its own permit.** It released only when the holder's depth returned to zero, but that depth lives in an `AsyncLocal` and is therefore shared with every context forked from the owner's — a timer callback whose timer was created inside the region, for instance. Such a context entering the gate raised the shared depth, so the owner's own exit saw a non-zero depth and returned without releasing, and nothing afterwards could: the nested exit is not the owner. Every later entry on that channel blocked forever. `Opc.Ua.Features.Tests` hung after its tests had finished, with a receive loop waiting on the gate and the server unable to stop. The owner now releases when it leaves; depth still gates re-entrancy, but liveness no longer depends on a count another context can move. **Write completion throttled the writer.** Reporting a completed write sat inside the ordering chain, and the client channel implements that callback by entering the gate — so every write queued behind a gate the receive loop contends for on each incoming chunk. A session with fifty subscriptions publishing continuously had its publish requests throttled enough that the client kept topping them up past its own limit, which `Subscriptions.Classic` asserts against. Completion is now reported off the chain, which covers only what ordering actually requires. ### Still synchronous, and why Certificate, certificate request and revocation list signing cannot be made asynchronous at all: `X509SignatureGenerator.SignData` is invoked by .NET's own builders. Service faults and the `Reconnect` override are reached from synchronous call sites (`SendServiceFault` alone has eight) and are left as they are behind scoped, commented suppressions. All three are documented rather than left to be rediscovered. ## #4210 — PubSub This one produced a finding rather than the API the issue asked for, and the finding is the point: > **With a standard Security Key Service the key necessarily exists in process memory.** `GetSecurityKeys` (Part 14 §8.3.2) returns raw key bytes over the wire, so the "key never leaves the device" property #4192 achieved for client and server **cannot** be achieved for PubSub through the SKS pull profile. That is a property of the specification, not of this stack. A wrapped-key envelope would change what is on the wire and break interoperability with third-party key services and publishers, so it was rejected rather than built. What is achievable is delivered: the per-message AES-CTR and HMAC route through `ISymmetricCryptoProvider`, so a validated module performs them. `IPubSubSecurityPolicy` is **not** modified — new members would break every implementation of it; the provider is supplied through the policy constructor. `PlatformSymmetricCryptoProvider` gained AES counter mode, which the seam should have covered anyway since the algorithm enum names it. Key lifetime is also tightened: unpacking an SKS response copied key material into intermediates that `ByteString.Create` then copied again, leaving plain keys in the heap for the lifetime of the process. Those are now zeroed. ### Two wiring gaps this depended on, both found in review The registrations above were **inert**, so this section originally claimed more than the code did: - `CryptoProviderConfiguration` was applied nowhere outside tests, so `AddCryptoProvider(configure)` never reached the registry in a real host. The registry now applies every registered configuration when it is first resolved — which also affects #4207's DI story. - `PubSubSecurityWrapperResolver` selected `PubSubAes256CtrPolicy.Instance`, the provider-less static singleton, ignoring the provider-backed bundles registered for it. A `FipsOnly` deployment therefore certified while the platform did the work. The resolver now selects from the registered policies; the container and `PubSubApplicationBuilder` both supply theirs. A third, smaller one: `CryptoProviderBuilder.Use` published a **policy-scoped** random source as the process-wide nonce source, so `.For(purpose, policyUri).Use(hsm)` redirected nonce generation for every other policy too. Only an unscoped binding does that now. ## #4206 — a provider can contribute a security policy Added in review: the policy set was fixed at compile time, so a provider could contribute key custody but not a *profile*. It is now table-driven and open, and the blockers #4206 listed are gone. ```csharp using IDisposable registration = SecurityPolicies.Register( new SecurityPolicyInfo("urn:vendor:SecurityPolicy#CustomProfile", "CustomProfile") { PlatformSupport = () => true, SupportedCertificateTypes = [ObjectTypeIds.RsaSha256ApplicationCertificateType], IsDefault = true }); services.AddOpcUa() .AddCryptoProvider(crypto => crypto .For(CryptoPurpose.ChannelSymmetric, "urn:vendor:SecurityPolicy#CustomProfile").Use(module)) .AddSecurityPolicy(customPolicy); ``` - **The reflection is gone.** Four lookup tables were built by reflecting over the fields of `SecurityPolicies` and `SecurityPolicyInfo`. They are one explicit table now, which removes the **last reflection in `Security/Constants`** — so this is a trimming and Native AOT improvement as much as a feature. - **`SecurityPolicyInfo` is constructible from another assembly** and carries the metadata the helpers need, so `IsPlatformSupportedName`, `GetDefaultUris`, `MapSecurityPolicyToCertificateTypes` and `GetCurveFromCertificateTypeId` read that table instead of each being hand-written. Adding a policy is one entry rather than five edits. - **The dead `#if CURVE25519` conditional is deleted** from `Security/Constants`. - Registration is copy-on-write, so the read path stays lock-free. Shadowing a built-in policy requires `replaceExisting: true` and is reversible by disposing the registration. `RegisterLightsUpCurvePoliciesFromOutsideCore` is the acceptance test the issue asked for: it registers `ECC_curve25519` and `ECC_curve448` from the test project and asserts they become discoverable through the public API. **Registering a policy makes it advertised and resolvable; it does not supply the cryptography behind it.** For those two curve profiles the in-tree key agreement is still behind a compile symbol no project defines plus a BouncyCastle dependency, so a deployment lighting them up provides the operations through a provider. That division is the point: the policy set says *what* is offered, the provider says *who* performs it. ## Removing the gate's re-entrancy Folded in from #4242, which was merged into this branch. `ChannelGate` was re-entrant, and that forced the design behind most of the defects above. A holder that awaits releases its thread, so ownership could not be tracked by thread identity — it had to live in an `AsyncLocal<Holder?>`. A logical context is *inherited*, so anything started while the gate was held inherited the right to re-enter and ran inside the guarded region alongside whatever started it, unless it opted out by hand via `LeaveInheritedContext()` at each of eight sites, enforced by review alone. Five of the defects listed above trace to that mechanism, including both CI hangs. So the *requirement* was removed rather than the mechanism, and the `AsyncLocal` then deleted itself. Every path that re-entered now calls a lock-free `Core` method (`ForceChannelFaultCore`, `ShutdownCore`, `ForceReconnectCore`, `CompleteConnectCore`), and three regions stopped running foreign code under the lock: the `RequestReceived` dispatch is deferred until the gate is released, `OnTransportError` no longer wraps the `HandleSocketError` virtual, and the callback setters and `EndpointDescription` publish a single reference with a volatile write instead of locking — the last is load-bearing, because `AsyncResultBase.OperationCompleted` invokes the reverse-hello callback inline while the gate is held. `ChannelGate` is now a `SemaphoreSlim(1,1)` behind a `Releaser`; `Holder`, the thread-identity fallback, `LeaveInheritedContext`, `IsHeldByCurrentContext` and all eight disclaimer calls are gone, with the call shape at all 32 sites unchanged. `SaveIntermediateChunk`, `GetSavedChunks` and `DoMessageLimitsExceeded` gained a `gateHeld` argument, because the client reaches them both with and without the gate and must pick the matching teardown — without it an oversized `OpenSecureChannel` response deadlocked the receive loop against itself. The remaining paths were not found by reading the code, since that is what missed them originally. A temporary strict mode made every re-entrant acquisition throw with its stack, the suite was run under it until clean, and it was deleted with the re-entrancy it measured. It caught two sites review had missed: the `EndpointDescription` **property getter** taking the gate, and the `RequestReceived` dispatch to application code happening inside the guarded region. Two deliberate residuals are documented for reviewers: `Dispose` no longer takes the gate (unavoidable — it is reached from `ChannelFaulted` with the gate held, where the old re-entrant acquisition excluded nothing anyway), and the deferred request dispatch builds `SecureChannelContext` just after gate release rather than under it. ## Testing New: the three seams and their platform implementations, facet discovery and resolution precedence, `FipsOnly` refusing a provider that cannot serve what it was bound to, AES counter mode (including a non-block-aligned round trip and a check that the counter carries, since one that did not would silently reuse key stream), `ChannelGate` (exclusion, re-entrancy across an await, re-entrancy on a reused thread, and the deadlocks above), the asynchronous channel paths, and the PubSub policy provider. Each review finding got a test that fails without its fix: - `ContendedEntryStillRecordsTheHolderAsync` — established contention deterministically, then re-entered; failed on the pre-fix gate. Removed with the re-entrancy it covered; `ChannelGateTests` now asserts the non-re-entrant contract instead. - `SecureClientAndTcpListenerKeepChunkOrderUnderConcurrentRequestsAsync` — 60 concurrent multi-chunk `SignAndEncrypt` requests over a real loopback listener; failed 3 runs out of 3 with the previous dispatch, passes 3 out of 3 with the chain. - `DependencyInjectionRoutesPubSubCryptoThroughTheRegisteredProviderAsync` — builds a container, registers a counting provider, wraps a message and asserts the provider actually performed the encryption and the signature. This is the assertion the PubSub claim above now rests on. - `PolicyScopedRandomBindingDoesNotEscapeItsPolicy`. Two further assertions are the ones that would catch a silent defect: a provider and the platform must produce **byte-identical** output — a validated module that disagreed could not interoperate — and with a software key the returned `ValueTask` must report `IsCompletedSuccessfully`, which is what makes "ordering is unchanged" checkable rather than merely asserted. `SymmetricChannelCryptoBenchmarks` gains a provider-registered variant beside the existing baseline, so the cost of the indirection is measured rather than assumed. Verified locally, every suite at or above its recorded baseline: `dotnet build UA.slnx -c Release` **0 warnings, 0 errors on all six target frameworks**; Core **4413** (net10.0); Core.Security **617**, with its certificate-leak detector clean; PubSub **1350** plus PubSub.Diagnostics **54** and PubSub.Udp **233**; Features **156**; Subscriptions.Classic **32**; Server integration **4053**; Client integration **2123**; MigrationAnalyzer **151** and MigrationAnalyzer.Core **13**. Zero failures in each. Core moved from 4388 to 4381 when the gate change landed — eight re-entrancy tests were removed along with the behaviour they covered, and one deadlock regression test added — and has since grown to 4413 with the registry isolation, provider-output and security-review tests. `ChannelGateTests` was rewritten around the new contract — exclusion across both entry modes, an uncontended asynchronous entry not suspending, re-entry blocking by contract, and the property that used to be the hazard: work started while the gate is held is excluded without having to opt out. Every CI failure this PR produced was reproduced locally, diagnosed against a clean `origin/master` baseline in a second worktree, and fixed at the source. Two hangs were found from `--blame-hang-timeout` dumps read with `dotnet-dump`. One failure that looked like a candidate was confirmed **not** to be: an intermittent `Opc.Ua.Aot.Tests` connect failure, pre-existing on `origin/master` (5 of 8 baseline runs failed, versus 3 of 14 here). ### The publish request overshoot `Subscriptions.Classic.PublishRequestCountAsync` failed on the Windows net10.0 leg with **207** outstanding publish requests against a cap of `max(maxServerPublishRequest, subscriptions)` = 50. It failed in 2 of 6 pipeline builds on this branch, never on `master` in 13 sampled builds, and never locally in 12 attempts including under deliberate CPU contention. The mechanism is a genuine race. `ClassicSubscriptionEngine.QueueBeginPublish` was a **check-then-act**: it read `GoodPublishRequestCount`, compared it against `GetDesiredPublishRequestCount(false)`, and issued a publish if it was below. It runs on every publish completion, so *n* completions that overlap each read the same value, each conclude they are below the cap, and each issue — overshooting by up to *n* per round. The read also *lags*: a request only becomes visible to `GoodPublishRequestCount` once `AsyncRequestStarted` has recorded it, which happens after the request has already been sent. That code is pre-existing and was untouched by this PR. What this PR changed is how hard the race is hit: the server used to hold the channel gate across request processing, which serialised every request on a channel, so publish completions arrived one at a time. Dispatching `RequestReceived` outside the gate — an intended improvement, since a channel should not serialise on application request-processing time — lets them complete concurrently. Rather than change the publish pipeline blind, the race was first made reproducible. `ConcurrentPublishReEvaluationDoesNotExceedDesiredRequestCount` drives 64 barrier-released completions against a mocked context whose count lags the send exactly the way the session's does. It issued **64 requests where 5 were desired** — the same defect the CI leg hit, now deterministic and running in milliseconds. The engine now tracks the requests it has in flight and reserves a slot with a compare-exchange before sending, so the decision and the send are one atomic step. The reservation is released when the request completes, when the send is declined, or when it throws. The repro test then issues at most the desired count. `StartPublishing` is deliberately left **uncapped**. It is the recovery valve that refills a pipeline whose requests are outstanding but no longer expected to return, so gating it on the in-flight count would remove the only path out of a stalled pipeline. An earlier revision of this fix did gate it, and that is the one change here that could not be justified on the evidence — it was reverted. The assertion was **not** loosened, retried away, or marked flaky: it caught a real throttling regression earlier in this PR and is worth keeping sharp. Validated: the repro test red before the fix and green after; `Subscriptions.Classic` 32/32 **five** consecutive runs on net10.0 and once on net48; `Opc.Ua.Client.Tests` 2124 on net10.0 across four consecutive runs and 2127 on net48; `Opc.Ua.Subscriptions.Tests` 604 passed with this fix in place. ### The SSH.NET advisory that turned CI red repo-wide Partway through this work every build job began failing with `NU1903: Package 'SSH.NET' 2025.1.0 has a known high severity vulnerability` ([GHSA-q939-rpr3-3284](https://github.com/advisories/GHSA-q939-rpr3-3284), `<= 2025.1.0` affected, first patched `2026.0.0`). Nothing in the dependency graph changed — the advisory was published. Build 17026 failed on it where build 17000, two hours earlier on an identical graph, had passed every build job. SSH.NET is not referenced anywhere in the repository; it arrives transitively through `Testcontainers.Kafka`. `master` landed the remediation independently while this was being investigated (a central transitive pin to `2026.0.0`, which `CentralPackageTransitivePinningEnabled` exists for). This branch briefly carried its own identical pin, which made the merge commit hold **two** `PackageVersion` entries for SSH.NET and fail restore with `NU1506`. The duplicate is removed and `master` is merged, so the pin now comes from one place only. The audit was **not** suppressed: `NuGetAudit` stays enabled and `NU1903` stays an error. Verified after the merge — `dotnet restore UA.slnx` reports neither NU1903 nor NU1506, and `dotnet build UA.slnx -c Release` is **0 warnings, 0 errors** with the audit live. The remaining red check in build 17026 was not from this change either. `aot-ubuntu-latest` is the intermittent `Opc.Ua.Aot.Tests` failure already noted above — `ReconnectSessionAsync` timed out at 30s with 135 of 136 passing. It remains below the `origin/master` baseline rate (4 of 15 sampled here against 5 of 8 on the baseline), and the managed reconnect paths are green throughout: `Opc.Ua.Client.Tests` 2124 across four consecutive runs and `Opc.Ua.Subscriptions.Tests` 604. ## Documentation `docs/CryptoProvider.md` gains sections on substituting the symmetric primitives, using a key served over a network, and PubSub — including what cannot be claimed and why, the scope rule for a random-source binding, and how to supply a provider-backed policy outside the container. `docs/MigrationGuide.md` covers the `[Obsolete]` channel virtuals and `DataLock`, and the non-re-entrant gate's contract for channel subclasses. `docs/WhatsNewIn2.0.md` is updated. `plans/cryptooffboard.md` is deleted now that the work it planned has landed. ## Note for reviewers This touches the secure channel handshake, which is the most security-critical code in the stack. Nine distinct concurrency and lifetime defects were found and fixed in it during this work — three during development, two in review, four from CI — so `ChannelGate`, the write path and the certificate ownership across the new suspension points deserve the closest reading. Five of those nine traced to the gate's `AsyncLocal`, which is why the re-entrancy was subsequently removed and the `AsyncLocal` deleted. The wrapper/`Core` split and the two documented residuals in that section are the parts most worth a second reading. The #4206 work removes the last reflection from `Security/Constants` and rewrites the policy lookup tables, so `SecurityConfiguration.SupportedSecurityPolicies` and the endpoint set it feeds are worth a second look: an earlier revision of it changed that set and broke six server tests, which is now covered by `SecurityConfigurationBuildsExpectedSupportedPolicySet`. ## Related Issues - Fixes #4206 - Fixes #4207 - Fixes #4208 - Fixes #4210 - Follow-up #4250 — this PR makes the security policy set injectable, but nothing in the stack resolves `ISecurityPolicyRegistry` from DI yet, so every production path still lands on the `Default` fallback. #4250 tracks threading it through `Session` and `UaSCBinaryChannel`, which is what makes the injection points already added here load-bearing. ## 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. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Copilot-Session: 7a68ca72-83e8-496c-b2f5-3df4c718320b Copilot-Session: 3f5ab76a-c0ba-4f47-87ac-54d1f4182c6b |
||
|
|
6e0ed89d3f |
Make UANodeSetSerializers.g.cs generation deterministic (#4252)
## Problem
Regenerating `src/Opc.Ua.Types/Schema/UANodeSetSerializers.g.cs` via
`UANodeSetSerializers.ps1` produced ~144 lines of no-op churn on every
run. The sgen tool (Microsoft.XmlSerializer.Generator) emits the Reader
`idNN_Name` field declarations and the matching `InitIDs()` initializers
in a hash-seed-dependent order: the assigned id numbers are stable, but
their emission order shuffles between runs. This made an otherwise no-op
regeneration produce a large, noisy diff that was painful to review.
## Fix
- Added a canonicalization pass in `UANodeSetSerializers.ps1` that sorts
each of the two affected contiguous blocks (the `string idNN_Name;`
field declarations and the `idNN_Name = Reader.NameTable.Add(...)`
initializers) by numeric id ascending, so re-running on unchanged input
yields byte-identical output.
- The name regex was widened to `@"[^"]*"` so entries with an empty name
(e.g. `id3_Item`) or URI-style names do not split the block into
independently-sorted runs (which would keep it unstable).
- Regenerated the committed `.g.cs` once through the improved script;
the resulting diff is sort-only.
## Also fixed: tool download
Replaced `Expand-Archive` (which rejects the `.nupkg` extension on
Windows PowerShell) with
`System.IO.Compression.ZipFile.ExtractToDirectory`, so a clean run with
an empty tool cache works end-to-end on both Windows PowerShell 5.1 and
PowerShell 7+.
## Validation
- From an empty tool cache, the full download + extract + generate path
succeeds.
- A second run is a byte-identical no-op (`git diff --exit-code` = 0).
- `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f
net10.0` builds with 0 warnings and 0 errors.
- All AOT post-processing is preserved: custom header, `#if
NET5_0_OR_GREATER` wrapper, module IL2026/IL3050 suppressions,
`[RequiresUnreferencedCode]`/`[RequiresDynamicCode]` on the
InitCallbacks() overrides (Writer = RequiresUnreferencedCode only,
Reader = both), pragmas, UTF-8 BOM, and CRLF line endings.
No hand-edits to `.g.cs`; the fix lives entirely in the script so future
regenerations stay stable.
## Also: Load UserAccessLevel from NodeSet2 model
Bundled here because it regenerates `UANodeSetSerializers.g.cs`, which
this PR owns.
- `UANodeSet.cs`: adds the `{Member}Specified` XmlSerializer convention
to `UAVariable.UserAccessLevel` (new `UserAccessLevelSpecified`
companion; removed the `[DefaultValue((uint)1)]`) so an
explicitly-present `UserAccessLevel` attribute is distinguishable from
the schema default of 1.
- `UANodeSetSerializers.g.cs`: regenerated - the writer now emits
`UserAccessLevel` when `UserAccessLevelSpecified`, and the reader sets
`UserAccessLevelSpecified = true`. Regenerating produced only these two
behavioral changes plus removal of pre-existing empty `if
(o.@MemberSpecified) { }` no-op blocks (see below); no `idNN` re-sort,
so determinism held.
- `UANodeSetHelpers.cs`: the runtime importer honors an
explicitly-specified `UserAccessLevel` instead of always deriving it
from `AccessLevel`.
- Source generator (`ModelDesign.cs` `RawUserAccessLevel`,
`ModelDesignExtensions.GetUserAccessLevelAsCode`,
`NodeSetToModelDesign.cs`): emit the verbatim `UserAccessLevel` when the
NodeSet sets it, otherwise derive from `AccessLevel` (matching the
runtime importer).
### Generator normalization for `{Member}Specified`
sgen emits an extra empty `if (o.@MemberSpecified) { }` no-op block near
`WriteEndElement` for every member that uses the `{Member}Specified`
convention. Adding the companion to `UserAccessLevel` would have
introduced one such block, and the committed file already contained
several for other members (`LastModified`, `AccessRestrictions`,
`PublicationDate`). `UANodeSetSerializers.ps1` now strips these empty
guard blocks during post-processing, so the generated file stays free of
dead code and a re-run of the generator remains a byte-identical no-op.
The real, non-empty guard around the `WriteAttribute` call is preserved.
The ReferenceServer sample application of this feature is intentionally
excluded and lands with #4220.
### Validation (UserAccessLevel)
- `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f
net10.0` and `dotnet build
tools\Opc.Ua.SourceGeneration.Core\Opc.Ua.SourceGeneration.Core.csproj
-c Release`: both 0 warnings / 0 errors.
- `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests -c Release -f
net8.0 --filter
"FullyQualifiedName~NodeSetToModelDesign|FullyQualifiedName~UserAccessLevel"`:
16 passed, 0 failed.
- Re-running the generator remains a byte-identical no-op.
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
|
||
|
Marc SchierandCopilot
|
fac30c45e6 |
Add MinSupportedSamplingInterval configuration and honor it when revising sampling intervals (#4243)
# Description PR #4194 removed the unused `ServerConfiguration.MinMetadataSamplingInterval` and left nothing in its place. As a result there was no way to populate `Server.ServerCapabilities.MinSupportedSampleRate` (`i=2272`) — `ServerInternalData.CreateServerObjectAsync` hardcoded it to `0`. Separately, the only clamp applied when revising a monitored item's sampling interval was the per-node `BaseVariableState.MinimumSamplingInterval`, which defaults to `MinimumSamplingIntervals.Continuous` (`0`), so reference-server nodes accepted any requested interval (e.g. 10 ms) unrevised — exactly what was reported. This adds a configuration entry **in the exact schema slot #4194 vacated** and enforces it as a lower bound. ## Configuration - **`ServerConfiguration.MinSupportedSamplingInterval`** — `double` (milliseconds), default `0`, `[DataTypeField(Order = 17)]`, i.e. between `MaxNotificationsPerPublish` (16) and `AvailableSamplingRates` (18). - `ApplicationConfiguration.xsd` — `<xs:element name="MinSupportedSamplingInterval" type="xs:double" minOccurs="0" />` in the same position the removed element occupied. - Fluent API **`SetMinSupportedSamplingInterval(double)`** on `IApplicationConfigurationBuilderServerOptions`, in the place `SetMinMetadataSamplingInterval` was removed from. - The element is added to the six config XML files #4194 touched (reference / CTT / AOT / GDS use `0`; `test-server-full.xml` uses a distinctive non-zero value for the round-trip test). The address-space Property keeps its OPC UA spec name, so `Server.ServerCapabilities.MinSupportedSampleRate` and `VariableIds.Server_ServerCapabilities_MinSupportedSampleRate` are unchanged. ```xml <MaxNotificationsPerPublish>1000</MaxNotificationsPerPublish> <MinSupportedSamplingInterval>2000</MinSupportedSamplingInterval> <AvailableSamplingRates> ``` ```csharp application.Build(applicationUri, productUri) .AsServer([endpointUrl]) .SetMinSupportedSamplingInterval(2000); ``` ## Behavior - `ServerInternalData` now publishes the configured value to `Server.ServerCapabilities.MinSupportedSampleRate`. - New `SubscriptionManager.CalculateRevisedSamplingInterval(...)` (two overloads, alongside the existing `CalculateRevisedQueueSize`) centralizes the revision rule and replaces the logic that was duplicated across four call sites — the create and modify paths of `AsyncCustomNodeManager` and `CustomNodeManager` — which now expose a `MinSupportedSamplingInterval` property seeded from the configuration (mirroring how `MaxQueueSize` is initialized). The rule: 1. A requested interval below zero resolves to the *default sampling interval*: the **publishing interval of the subscription** on create, and the item's **current sampling interval** on modify. The modify case preserves 1.5.378 behavior, so a `ModifyMonitoredItems` call that leaves the sampling interval unspecified does not silently retune the item. 2. A node declaring `MinimumSamplingIntervals.Continuous` (`0`) for the `Value` Attribute reports **by exception** and is **not** bound by the server-wide minimum — the requested interval is returned unchanged. 3. Every other item is raised to `max(nodeMinimumSamplingInterval, MinSupportedSamplingInterval)`. Attributes other than `Value`, nodes declaring `MinimumSamplingIntervals.Indeterminate` (`-1`), and non-Variable nodes are bound only by `MinSupportedSamplingInterval`. 4. `double.MaxValue` is capped to one year (pre-existing behavior, now in one place). Event monitored items do not sample and are unaffected. **Backward compatible:** with the default of `0` the computation is identical to the previous code for every input, including `Continuous`, `Indeterminate`, positive node minimums, non-`Value` attributes and `NaN`. No existing configuration or deployment changes behavior. Node managers that implement `INodeManager` directly can call the new public helper to apply the same rule. ## Related Issues - Fixes #4191 The issue thread also asked whether `Subscription.MaxNotificationsPerPublish` is revised by the server. It is (`SubscriptionManager.CalculateMaxNotificationsPerPublish`), but it was untested and is not observable from the `CreateSubscription` response, which carries no revised field for that parameter. Four tests were added that assert it through `SubscriptionDiagnostics.MaxNotificationsPerPublish` on a privileged session, covering create and modify, the above-limit, below-limit and `0` (server default) cases. ## Tests - **`SamplingIntervalRevisionTests`** (17 unit tests, `Opc.Ua.Server.Tests`) — the full revision matrix: negative request resolving to the default interval, the `Continuous` bypass, `Indeterminate`, node minimum above *and* below the floor, a floor of `0` being a no-op, non-`Value` attributes, non-Variable and null nodes, and the `double.MaxValue` cap. - **`MinSupportedSamplingIntervalTests`** (10 integration tests, `Opc.Ua.Subscriptions.Tests`) — against a reference server configured with a 500 ms floor: the capabilities node reflects the configuration; a node declaring 100 ms is lifted to 500 ms; `Scalar_Static_Arrays2D_LocalizedText` (1000 ms) wins over the floor; `CurrentTime` (250 ms) is lifted; a `Continuous` node keeps its 10 ms; a non-`Value` attribute is floored; `ModifyMonitoredItems` behaves the same in both directions; event items are unaffected. - Restored the configuration builder and round-trip test coverage that #4194 deleted. - `TestFixture` gained a `protected virtual void ConfigureServer(ApplicationConfiguration)` hook so a derived fixture can vary the server configuration before start. Default implementation is a no-op, so no existing fixture changes behavior. ## Documentation - `docs/Subscriptions.md` — new **Sampling interval revision** section with the input table, the four-step rule (including both negative-interval fallbacks), XML and fluent configuration examples, and guidance for custom node managers. - `docs/migrate/2.0.x/configuration.md` — new section documenting that `MinMetadataSamplingInterval` was removed and `MinSupportedSamplingInterval` added. Verified against `master378`: the old setting defaulted to `1000` but was never read, and `MinSupportedSampleRate` was hardcoded to `0` with no server-wide floor. The guide therefore stresses that these are **two different settings, not a rename** — consumers should drop the old element (a default of `0` reproduces 1.5.378 exactly) and only opt into the new one deliberately, since reusing the old value would silently introduce a sampling floor. ## Validation - `dotnet build UA.slnx` — **0 warnings, 0 errors** across all TFMs. - .NET 10: `Opc.Ua.Server.Tests` 4091 passed, `Opc.Ua.Subscriptions.Tests` 619 passed (full suite including the new fixture), `Opc.Ua.InformationModel.Tests` 1063 passed, `Opc.Ua.Configuration.Tests` 221 passed, `Opc.Ua.Gds.Tests` 119 passed. - .NET Framework 4.8: new unit tests (200 incl. sampling groups), new integration tests (17) and configuration round-trip (135) all passed. - `ConsoleReferenceServer` starts successfully with the updated configuration file. ## 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. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79f8090c-b2a3-409c-a5bf-56b6d2a35e70 |
||
|
Marc SchierandCopilot
|
5bc8a9d705 |
Reduce ISession: type the continuation points, delete the obsolete validate overload (#4199)
## Stacked on #4197 Base is `marcschier/server-bind-phase`. The full stack is **#4188 → #4193 → #4197 → this**. GitHub retargets each base automatically as the stack merges. **Review the earlier three first.** **#4217 has been merged into this branch** (squash `53dd4b618`), so the last remaining item from the issue — `ISession.SessionDiagnostics` — is now part of this PR. Closes #4187. Refs #4203 — **does not close it.** #4217 swapped `OnUpdateDiagnostics` onto `m_diagnosticsLock`, but it still calls `Variant.FromStructure(ServerDiagnostics)` with the default `copy: false`, which wraps the *live* structure in the `ExtensionObject`. The caller reads its fields after the lock is released, so the lock currently excludes nothing. #4222 adds `copy: true` and is what actually closes #4203. ## The finding that changed the approach `ISession` has **exactly one production adapter**: `src/Opc.Ua.Server/Session/Session.cs`. The other `ISession` implementations in the tree are the unrelated **client** interface (`src/Opc.Ua.Client/Session/Session.cs`, `RedundantClientSession`) and a migration-analyzer test stub. One adapter means the seam is hypothetical; two means it is real. The issue proposed splitting `ISession` into five concern-interfaces — that would turn one hypothetical seam into five. The interface earns its keep for testability and because node managers receive it through `OperationContext`, but that argues for making it **smaller**, not for multiplying it. **So: reduce, don't split.** Reasoning posted to the issue as [comment 5226712936](https://github.com/OPCFoundation/UA-.NETStandard/issues/4187#issuecomment-5226712936) so the five-way framing does not get re-proposed later. ## Measurement 38 members, 368 references. External production consumers are **4 files using 5 members**: | Consumer | Members used | |---|---| | `Opc.Ua.Redundancy.Server/Sessions/DistributedSessionManager.cs` | `Id`, `InitializeAsync`, `Dispose` | | `Opc.Ua.Di.Server/Locking/DefaultLockService.cs` | `Id` | | `samples/ConsoleReferenceServer/UAServer.cs` | `Id`, `Identity`, `ReadDiagnostics` | | `samples/Quickstarts.Servers/.../DataChangeMonitoredItem.cs` | type reference only | Every production caller of the five continuation-point members is inside `Opc.Ua.Server` (`MasterNodeManager`, `NodeManagerLifecycle`, `HistorianDispatcher`). **`ISession` is 38 → 32 members.** ## Commits ### `882b2937` Type the history continuation point `ISession` leaked `object` twice, against the repository rule: ```csharp void SaveHistoryContinuationPoint(Guid id, object continuationPoint); object? RestoreHistoryContinuationPoint(ByteString continuationPoint); ``` The erasure was **not** hiding a genuinely opaque plugin token. `HistorianDispatcher` is the only production caller, it always passes a `HistorianContinuationState`, and it casts straight back to that type on restore. The `object` was a round trip through the type system that bought nothing. `HistorianContinuationState` is `internal`, so it cannot appear on a public interface. `IHistoryContinuationPoint` names the concept instead: it carries an `Id` and extends `IDisposable`. Two latent bugs fell out of typing it: - The session disposed only those points that **happened** to implement `IDisposable` and **silently leaked the rest**. Every point is now disposable by construction, and three `as IDisposable` probes are gone. - Carrying the `Id` on the point removes the separate `Guid` parameter, which could disagree with the payload it was filed under. The mock stores in the historian tests had the same latent inconsistency. ### `0d020b77` Delete the obsolete synchronous `ValidateBeforeActivate` **Zero production callers** — the nine references were all tests written for it. `[Obsolete]` since 1.5.378, which the repository rules allow removing in master. It is also the divergent-signature problem the runtime analysis flagged: the async form returns the validated handler and policy as a tuple, while the sync one took the same two values as trailing `out` parameters, so a caller had to learn two shapes for one operation. The overload could not verify a user token requiring decryption, so on a secure endpoint it failed closed and told the caller to use the async path anyway. The one test asserting that fail-closed behaviour goes with the method it described; the path it pointed at is already covered by `ActivationValidationDecodesBinaryEncodedUserIdentityToken`. The other eight tests are **converted, not deleted** — the channel, signature, security-policy and token-policy validation they cover is still asserted, now against the path the server actually uses. ### `821e8302` Route the continuation points behind one member Five `ISession` members were one-line pass-throughs to `SessionContinuationPoints`, a 419-line module that already owns the behaviour and already has its own `IContinuationPointStore` seam. The interface restated the module's surface without adding anything. ```csharp // was session.SaveContinuationPoint(cp); session.RestoreHistoryContinuationPoint(bytes); // now session.ContinuationPoints.SaveBrowse(cp); session.ContinuationPoints.RestoreHistory(bytes); ``` The holder is `internal sealed`, so `ISessionContinuationPoints` is what makes it addressable. **This is composition, not the service locator `IServerContext` refused to be.** A subsystem with an independent lifetime should be injected; continuation points belong to the session and cannot exist without it. `InvalidateContinuationPoints` called `RemoveBrowseForManager` and `RemoveHistoryForManager` in sequence, so the pair becomes `RemoveForManager` — the caller's intent is that a node manager is going away, not that two collections need visiting. ## Tests Three harnesses had hand-rolled a `Dictionary<Guid, object>` to stand in for the holder. They now use the **real** one, so the dispatcher and `BrowseNext` meet the eviction and disposal rules a live session actually applies. **One hazard worth recording for future interface work.** A Moq member that is never stubbed returns `null`. Where the old code called a method straight on `ISession`, that `null` was harmless. Reaching *through* a property means an unstubbed `ContinuationPoints` null-derefs instead. Three tests failed exactly that way; they are wired to a real holder, not restubbed. | Suite | net10.0 | net48 | |---|---|---| | `Opc.Ua.Server.Tests` | 4032 passed, 1 failed | builds clean | | Touched areas (continuation points, historian, master node manager, session) | 898 passed | — | The single failure is `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a local certificate-store problem verified to reproduce unchanged at HEAD with these changes stashed. Not caused by this PR. `Opc.Ua.Redundancy.Server`, `Opc.Ua.Di.Server`, `Quickstarts.Servers` and `ConsoleReferenceServer` all build clean. ## Docs `docs/MigrationGuide.md` gains a "Removed members on ISession" section covering the `ValidateBeforeActivate` removal with the async replacement, and the `IHistoryContinuationPoint` change with before/after. ## The SessionDiagnostics removal (folded in from #4217) `ISession.SessionDiagnostics` handed out the whole mutable `SessionDiagnosticsDataType` — the structure the session's diagnostics lock protects — so a caller could read a field while the owner was writing it. Every server-side reader wanted exactly one value out of it, so the interface now exposes those two values (`SessionName`, `ClientApplicationUri`) directly and the structure stays behind the lock. That work landed here as #4217 rather than as a separate PR, and it also fixes #4203: `OnUpdateDiagnostics` now takes `m_diagnosticsLock` instead of locking the diagnostics payload it was handed. ## Reviewer questions - `ISessionContinuationPoints` exposes `MaxBrowse` as read-only while the concrete holder keeps a setter (the test framework adjusts it to exercise limits). Should the setter be on the interface, or is test-only mutation better left to the concrete type? - `RemoveForManager` collapses two removal methods. The concrete `RemoveBrowseForManager`/`RemoveHistoryForManager` remain public on the holder — worth making them private now that nothing outside calls them individually? --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041 |
||
|
Marc SchierandCopilot
|
e73e71184d |
[WotCon] Add OPC UA WoT Connectivity 1.1 with the Types, source generation, Server and xRegistry work it builds on (#4128)
## Summary Adds OPC UA WoT Connectivity 1.1 to the stack, together with the Types, source generation, Server and xRegistry work it is built on. This branch is the merge point for the fourteen-PR stack that replaced integration PR #4093; every constituent PR was reviewed and approved separately and is listed below. 414 files changed, +108,936 / -3,199. Roughly 46k added lines of product code, 57k of tests, plus samples, tools and documentation. ## Types - Complete `ExtensionObject` raw-body decoding for binary, XML and JSON bodies by resolving the concrete type through the message context's encodeable factory, and simplify the `Variant` structure helpers by delegating to it (#4128). - Add `IAtomicFileReplace` as an optional `IFileSystem` capability so existing external implementations keep working, with atomic publish for `LocalFileSystem` via `File.Replace`/`File.Move` and for `VirtualFileSystem` by re-keying the in-memory entry (#4128). - Add lossless conversion between WoT documents and NodeSet2 (`Opc.Ua.Wot.WotNodeSetConverter`). A byte-exact `uav:nodeSet` envelope is preserved when requested, the structured `uav:nodes` projection is used when the readable vocabulary is incomplete, and NodeSet2 is otherwise synthesized from readable WoT terms. Unmapped JSON members survive a round trip as pointer-addressed residue (#4131). ## Source generation - Improve the NodeSet to ModelDesign conversion and the generated node state (#4132). - Generate OPC UA models directly from WoT Thing Description files (#4134). ## Server - Add live NodeManager shadow and immediate reload. A reload materializes a new generation beside the active one and switches atomically; the superseded generation is retired gracefully so existing MonitoredItems keep being served until they drain, or immediately when the caller asks for it. Includes the request admission and drain machinery that lets an orderly shutdown wait for admitted requests instead of tearing down underneath them (#4147). - Make subscription transfer between sessions transactional, so a failed transfer leaves neither session holding a partially moved subscription (#4150). ## xRegistry - Allow clients to use explicit registry roots rather than assuming a well-known location (#4130). ## WoT Connectivity - Add the WoT Connectivity 1.1 information model: a registry-first revision layered on the abstract xRegistry base model, incorporating the complete published OPC 10100-1 v1.02 surface as deprecated nodes in the same namespace so existing 1.02 clients keep working (#4139). - Add the protocol binding abstractions and planners that compile WoT forms into executable plans (#4140). - Add the HTTP, Modbus, OPC UA and MQTT binding executors (#4144). - Add the registry and its stores, including document versioning, validation, dependency resolution and refresh (#4146). - Add the registry client (#4142). - Materialize WoT Thing Descriptions into the server address space, deriving types from Thing Models and instances from Thing Descriptions (#4154). - Restore the WoT asset registry bridge so the deprecated 1.02 asset surface is backed by the registry (#4171). - Add the WoT Connectivity samples and complete the documentation (#4156), and restore the remaining coverage and docs (#4172). ## New projects `src/Opc.Ua.WotCon.Bindings`, `src/Opc.Ua.WotCon.Bindings.Mqtt`, `tests/Opc.Ua.WotCon.Bindings.Tests`, `tests/Opc.Ua.WotCon.Samples.Tests`, and the `samples/WotCon` AggregationServer, AggregationClient and FlatTagServer samples. ## Constituent pull requests Merged into this branch top-down, each independently approved: | PR | Title | | --- | --- | | #4128 | [Types] Complete ExtensionObject decoding and add an atomic file replace capability | | #4131 | [Types] Add lossless conversion between WoT documents and NodeSet2 | | #4132 | [SourceGeneration] Improve NodeSet to ModelDesign conversion and generated node state | | #4134 | [SourceGeneration] Generate OPC UA models from WoT Thing Description files | | #4147 | [Server] Add live NodeManager shadow and immediate reload | | #4150 | [Server] Make subscription transfer between sessions transactional | | #4130 | [XRegistry] Allow clients to use explicit registry roots | | #4139 | [WotCon] Add the WoT Connectivity 1.1 information model | | #4140 | [WotCon] Add the WoT protocol binding abstractions and planners | | #4144 | [WotCon] Add the HTTP, Modbus, OPC UA and MQTT binding executors | | #4146 | [WotCon] Add the WoT Connectivity registry and its stores | | #4142 | [WotCon] Add the WoT Connectivity registry client | | #4154 | [WotCon] Materialize WoT Thing Descriptions into the server address space | | #4156 | [WotCon] Add the WoT Connectivity samples and complete the documentation | | #4171 | Restore WoT asset registry bridge | | #4172 | Restore WoT coverage and docs | Supersedes #4093. ## Follow-up The WoT Connectivity and WoT Binding drafts have moved to 1.1-draft2 since this work was authored. Aligning with that revision - the removed group vocabulary, the new projection/View construct, the remaining model vocabulary terms and the two new portable-identity validation rules - is tracked separately and will follow in its own pull request. ## Validation Built for every target framework with zero warnings and zero errors, and validated at the tip of the stack with `Opc.Ua.WotCon.Tests` at 999 passed / 0 failed and `Opc.Ua.Server.Tests` at 4024 passed / 0 failed. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8 |
||
|
Marc Schier
|
fde50c5700 |
Enforce ActivateSession transfer and reconnect security (OPC 10000-4 §5.7.3.1) (#4050)
# Description This draft isolates the Session/ActivateSession, reconnect, and distributed Session security work split from #4021. ### Failure - `ActivateSession` could transfer a Session to a new SecureChannel without consistently enforcing the original client certificate, MessageSecurityMode, SecurityPolicy, and exact OPC `ClientUserId` semantics. - Missing or legacy distributed Session security state did not provide enough information to validate transfer safely. - Malformed or reused nonces, concurrent activation, and cancellation could leave activation state inconsistent. - Client reconnect could treat recoverable Session-loss/security responses as fatal, reuse a Session after client-certificate rotation, or report a managed channel ready before Session recreation completed. ### Fix - Serialize activation, consume server nonces once, validate application signatures and user tokens asynchronously, and enforce transfer continuity against the original channel and authenticated identity. - Persist versioned transfer-security state for distributed Sessions and fail closed when certificate, identity, activation, or nonce state is absent or invalid. - Validate server signatures/nonces on the client, recreate Sessions for certificate rotation and recoverable activation failures, and await participant recreation before transitioning the managed channel to `Ready`. - Make the restart recovery integration test use bounded reconnect cycles instead of a fixed startup delay. ### Specification - OPC 10000-4 §5.7.2.2 — CreateSession signatures and nonces. - OPC 10000-4 §5.7.3.1 — ActivateSession identity continuity and SecureChannel transfer. - OPC 10000-4 §6.1.8 — application nonce generation, validation, and reuse requirements. ### Validation - `git diff --check` - Release `net10.0`: `Opc.Ua.Redundancy.Server.Tests` 567/567 plus the targeted server Session, Subscription transfer, ClientUserId, client reconnect and certificate rotation suites pass. - Release `net48`: the same redundancy suite (567/567) and the targeted Session/Subscription suites pass. - Full `Opc.Ua.Server.Tests` run on `net10.0`: 3633/3634. The single failure, `ServerFluentApiHostingTests.ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, reproduces identically on the unmodified commit and is a local certificate-store path issue rather than a regression. These were targeted validation runs; no long-soak result is claimed. |
||
|
Copilot
|
a03bca97cb |
Remove TraceConfiguration and related APIs (#4092)
# Description
This change removes the remaining `TraceConfiguration`-driven
apply/setter API path and aligns configuration startup with the 2.0
telemetry model (`ITelemetryContext`/`ILoggerFactory`). It also updates
migration docs to make the removal explicit and actionable.
- **Core API removals**
- Deleted `TraceConfiguration.ApplySettings()` implementation file.
- Removed runtime apply calls from configuration/server startup paths.
- Removed fluent builder trace setters:
- `SetOutputFilePath(...)`
- `SetDeleteOnLoad(...)`
- `SetTraceMasks(...)`
- Simplified builder interface inheritance accordingly.
- **Call-site and test fixture updates**
- Removed usage of deleted trace fluent APIs in affected test
infrastructure and fixtures.
- Updated GDS log readers to tolerate missing `TraceConfiguration`
output path safely.
- **Migration guidance**
- Updated migration docs to explicitly state removal of
`TraceConfiguration` apply APIs and direct users to telemetry-based
logging configuration.
```csharp
// Removed API pattern
app.Build(uri, product)
.AsClient()
.SetOutputFilePath("trace.log")
.SetTraceMasks(Utils.TraceMasks.Error);
// New direction
// Configure logging via ITelemetryContext / ILoggerFactory at composition/startup.
```
## Related Issues
- OPCFoundation/UA-.NETStandard#4091
## 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.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes #4091
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
|
||
|
Marc Schier
|
2a4bc0fe24 |
Make reverse connect lifecycle fully asynchronous (#4059)
## Summary - add cancellation-aware asynchronous start, lazy initialization, stop, reload, and disposal for `ReverseConnectManager` - make listener replacement transactional with explicit ownership, rollback/restoration, stale watcher protection, and deterministic concurrent start/stop/dispose behavior - add injectable async configuration providers, remove the former protected configuration hooks, and retain obsolete synchronous lifecycle compatibility wrappers - integrate eager Generic Host startup plus non-host lazy startup, including custom application-configuration providers - serialize `ReverseConnectHost` lifecycle and extend raw TCP, Kestrel TCP, WSS, hosting, concurrency, rollback, and NativeAOT coverage - document the async lifecycle, hosting behavior, cancellation guarantees, provider model, and migration path ## Validation - `Opc.Ua.Client` Release build across all supported target frameworks - reverse-connect client and core tests on net10.0 and net48 - raw TCP, Kestrel TCP, and WSS integration tests on net10.0; raw TCP on net48 - NativeAOT publish and 115-test executable run Closes #4013. Related: #3985, #4009. |
||
|
MrAlaskan
|
8f4087605a |
Fix unordered server endpoint validation (#4029)
### Summary This PR makes `Session.ValidateServerEndpoints()` compare `ServerEndpoints` and `UserIdentityTokens` as unordered sets and adds regression tests that verify `OpenAsync()` no longer rejects legal servers that return the same endpoint data in a different order. ### Problem According to OPC UA Part 4, the client validates the endpoint set returned by `CreateSessionResponse.ServerEndpoints` against the endpoint set observed during discovery. The specification describes these values as a set of endpoint descriptions filtered by the relevant transport profile, not as an ordered array that must preserve the exact discovery-time enumeration order. Previously, `Session.ValidateServerEndpoints()` first checked the endpoint count and then compared `m_discoveryServerEndpoints[ii]` against `serverEndpoints[ii]` by index. It also compared `UserIdentityTokens[jj]` by index within each endpoint. As a result, a server that returned the same legal endpoints in a different order, or returned the same token policies in a different order within an endpoint, could be rejected with `BadSecurityChecksFailed` even though the endpoint data itself had not changed. ### Changes - Replace index-based `ServerEndpoints` validation with unordered matching based on the same endpoint fields already validated by the client. - Compare `UserIdentityTokens` as an unordered multiset instead of requiring the server to preserve the original token enumeration order. - Extend the client session test scaffolding so discovery endpoints can be supplied explicitly. - Add regression tests that verify `OpenAsync()` accepts reordered `ServerEndpoints` and reordered `UserIdentityTokens`. |
||
|
Marc Schier
|
41a648c9db |
Fail fast on reverse connect listener startup (#4009)
# Description Fail reverse-connect listener startup immediately instead of logging bind failures and surfacing a later connection timeout. - Validate and stage configured and programmatic listener endpoints before replacing active configuration. - Open all listeners atomically and report endpoint-specific failures with `BadTcpEndpointUrlInvalid`, the transport-specific status, or `BadNoCommunication`. - Roll back listeners opened by a failed attempt without reacquiring the manager lock, including forced listener disposal when normal close fails. - Preserve meaningful `ServiceResultException` status codes through both `StartService` overloads. - Dispose replaced configuration watchers and reject callbacks from stale watcher generations. - Keep the intended shared-listener behavior: one long-lived `ReverseConnectManager` can accept connections from multiple Servers on the same Client URL. - Expand `Docs/ReverseConnect.md` with listener lifetime, shared-manager usage, diagnostics, and specification references. ## Validation - `ReverseConnectManagerTests`: net10.0 and net48. - `ReverseConnectHostTests`: net10.0 and net48. - Ordered reverse-connect listener reuse integration tests: net10.0. ## Related Issues - Relates to #3985. ## 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. |
||
|
Marc Schier
|
8538f531e7 | Update paths for the consolidated repository layout | ||
|
Marc Schier
|
7cec2d614d | Move repository files to the consolidated layout |