Repository navigation
Fix Opc.Ua.Client audit findings (security fallback, reconnect, subscriptions, redundancy, browse, files, WebApi) - #4572
Conversation
…oints An empty page with a continuation point is legal (Part 4 5.9.2.1): all three browse paths now keep following it, give up after 10 empty rounds with BadNoData and release the point. BrowseStreamAsync releases every point of the current page on early exit and documents result correlation; the single node browse releases on any failure; MaxNodesPerBrowse > int.MaxValue no longer produces a negative batch size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…requested SelectEndpoint(useSecurity: true) returned the first endpoint of any mode when nothing matched, letting a rogue discovery answer downgrade to None; it now returns null and the GDS clients fail with BadSecurityPolicyRejected. DiscoverServersAsync skips null DiscoveryUrls instead of throwing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion An empty DataValue collided with the missing-slot sentinel and mispaired values with NodeIds. The node cache lookups no longer turn cancellation into "node does not exist". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on before token reuse A Warm/Hot ex-leader kept its leader session (subscriptions + shared token) and fought the new leader over the mirrored session; it is now disposed and replaced by a fresh standby. Token-reuse promotion closes the standby's own server session instead of orphaning it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A NodeClass hint that differs from the node's real class threw KeyNotFoundException out of the whole batch; an undefined NodeClass value failed every node. Both now yield a per-node error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A cancelled first StartTrackingAsync stopped the pump another caller had joined. The pump is now stopped only when the last waiter abandons the start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A pending ack exactly 2^31 away from the latest sequence number threw an OverflowException inside the ack lock, dropping the publish response and keeping the bad ack. Compare the wrap-aware distance without Math.Abs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… notification SaveValueInCache keeps the last notification whatever its kind, so a data change for an event item (or the reverse) made the hard cast throw InvalidCastException into application code. Use as-casts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The copy constructor dropped TriggeredItems, so a session recreated from a template without a successful transfer silently lost every SetTriggering link. Links reference client handles, which clones with copyClientHandle keep, so copy them (the server id of the triggering item is reset). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three keep-alive intervals above int.MaxValue wrapped negative, so every Publish was sent with a 1 s timeout; an interval capped at int.MaxValue made PublishingStopped always true. Compute in long/double instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…livery The trim loop called RemoveFirst on an empty cache when MaxMessageCount was negative, so every worker pass threw before any notification was delivered and unprocessed messages piled up. Stop trimming when empty. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…onger deadlocks PublishStatusChanged(Stopped) is raised on the publish timer callback, and Dispose waited for the timer's running callbacks, i.e. for itself. Mark the timer callback as a dispatch context so Dispose takes the non-blocking path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…resh The discovery reply is unauthenticated; retrying with Invalid/null policy let a misconfigured server or attacker downgrade the session to SecurityMode None. Fall back only to an endpoint with equal or stronger locally computed security. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A chunk longer than requested corrupted the value and looped until OOM or int overflow; reject it and stop before the next range overflows int. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… provided An absent (0) or > int.MaxValue server MaxByteStringLength made ReadByteStringInChunksAsync throw instead of using MessageContext.MaxByteStringLength. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…idation, worker flow L3-4: TransitionToFailover no longer overwrites a concurrent Closing state. L3-7: MaxTotalReconnectTime 0 means unlimited, other negatives fail at construction. L2-1: worker flow is scoped per state step so spawned background work stops passing as the worker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restore/Load keep the ServerId of the previous session, so when the subscription was created instead of transferred CreateAsync took those items as created and silently skipped them. Reset the server ids on create. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RemoveItem only queued created items, so an item removed while its CreateMonitoredItems call was in flight was created on the server and never deleted. After the create results are applied, queue such items for deletion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tics string table L3-2: when a later batch fails, release browse/history continuation points and delete monitored items created by completed batches (Part 4 7.9) before rethrowing. L3-3: rebase merged DiagnosticInfo string indexes even when missing diagnostics are padded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l hosts L8-1: Undefined (and bare Any) registrations default to Once as documented instead of Always. L8-2: re-adding an endpoint disposes the replaced manual host instead of leaking it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…elete unadoptable ones The restore-from-storage transfer compared GetMonitoredItems against every saved item, so one item that was never created failed the transfer, and the server subscription just moved to the session stayed alive as an orphan. Count only created items, and delete the server subscription if it cannot be adopted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… numbers The republish path passed an empty AvailableSequenceNumbers list, which the subscription stored, so other missing messages were given up as BadMessageNotAvailable and pending acks were pruned while the server still held them. Republished messages now keep the last Publish response's list and all pending acks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gain A Publish response racing its Republish (or a message arriving after its placeholder expired) was appended at the list tail and delivered a second time, and a copy of a processed entry was re-marked unprocessed. Ignore copies that are not newer than the last processed sequence number, never reprocess a processed entry, and reset the sequence state when a new server subscription is created. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…client made The unsolicited-transfer check looked at the old session's Reconnecting flag, which an explicit live-to-live transfer never sets, so the notification sent to the old session stopped the publish worker (ReportOnly) or recreated the subscription on the new session. The session now marks subscriptions while it transfers them, and notifications received during or before such a transfer are ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After a reconnect that transfers the subscription onto the same object, unacknowledged but already dispatched messages are still listed as available (Part 4 5.14.7.2). Acknowledge them instead of republishing and dispatching them a second time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A keep-alive carries the next sequence number (Part 4 5.14.1.1), so one more than one ahead of the last data message proves the messages in between were lost. Republish them right away instead of waiting for the next data message, which on quiet subscriptions may never come. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sync Client handles need not be unique on the server (Part 4 7.21); a lost create response that is retried leaves two server items with the same handle. Build the handle map before clearing the item table, keep the bound item and delete the extra server items instead of throwing and losing every item. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pass A transient per-item failure was retried immediately until the 6-attempt budget was exhausted within milliseconds, leaving the item permanently un-created. Stop the pass once only already-failed changes remain so the state manager backoff spaces the retries (also stops a hot loop on a delete that keeps failing). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A conforming server can revise publishing interval and keep-alive count so their product exceeds TimeSpan.MaxValue (Part 4 5.14.2.2); a NaN interval is non-conforming but possible. The throwing TimeSpan APIs aborted CreateSubscription/CreateMonitoredItems after the server succeeded and faulted every publish worker in a respawn loop. Use saturating conversions (L6-1 regression test lands with the L6-2 commit, same test hunk). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Master now contains PR #4516 (46fa7d3) at a later head than the squashed copy this branch was based on (46fc78c). The branch's client fixes (46fc78c..cb0764a) were replayed onto master with 46fc78c as the merge base, so only the two real overlaps needed resolving: - ManagedSession: master inlined the same lease-swap reseed as the L2-4 fix; kept the SeedChannelReconnectTracking helper. - FakeManagedSubscription: kept the OnIsCreationInProgress hook and master's new IsIntentionallyDeleted property. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Background work items can run concurrently, so the test could read the disposal task before the spawner item assigned it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved security, race-condition, file-readiness, continuation-point, and Web API timeout findings block approval.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
This PR addresses a broad audit of OPC UA client security, reconnect/recovery, subscriptions, browsing, redundancy, file transfer, Web API transport, and ComplexTypes.
Changes:
- Hardened security endpoint selection, credential handling, reconnects, and transport failures.
- Fixed subscription, session, browse, redundancy, file-system, and state-machine edge cases.
- Added extensive regression tests and documentation.
| File | Summary |
|---|---|
tests/Opc.Ua.Client.Tests/Utils/AsyncReaderWriterLockTests.cs |
Tests bounded writer draining. |
tests/Opc.Ua.Client.Tests/Subscription/MonitoredItemTests.cs |
Tests index-range normalization. |
tests/Opc.Ua.Client.Tests/Subscription/MonitoredItemManagerTests.cs |
Tests deferred retries. |
tests/Opc.Ua.Client.Tests/Subscription/Fakes/FakeMessageAckQueue.cs |
Adds session-availability test support. |
tests/Opc.Ua.Client.Tests/Subscription/Fakes/FakeManagedSubscription.cs |
Adds creation-state test support. |
tests/Opc.Ua.Client.Tests/Subscription/Classic/ClassicMonitoredItemCoverageTests.cs |
Tests triggering-link cloning. |
tests/Opc.Ua.Client.Tests/Streaming/StreamingSubscriptionTests.cs |
Tests disposal races. |
tests/Opc.Ua.Client.Tests/StateMachines/StateMachineTypeClientExtensionsTests.cs |
Tests state observation failures. |
tests/Opc.Ua.Client.Tests/StateMachines/FiniteStateMachineTypeClientExtensionsTests.cs |
Tests instance sub-state resolution. |
tests/Opc.Ua.Client.Tests/Session/UpdateIdentityAsyncTests.cs |
Tests ECC identity updates. |
tests/Opc.Ua.Client.Tests/Session/SessionReconnectHandlerTests.cs |
Tests secure fallback selection. |
tests/Opc.Ua.Client.Tests/Session/SessionExtensionsTests.cs |
Tests byte-string limit fallback. |
tests/Opc.Ua.Client.Tests/Session/SessionClientExtensionsTests.cs |
Tests oversized byte responses. |
tests/Opc.Ua.Client.Tests/Session/SessionClientBatchTests.cs |
Tests batch cleanup and diagnostics. |
tests/Opc.Ua.Client.Tests/Session/ServerRedundancyHandlerTests.cs |
Tests healthy-server failover decisions. |
tests/Opc.Ua.Client.Tests/Session/ProactiveRefreshTests.cs |
Tests identity refresh behavior. |
tests/Opc.Ua.Client.Tests/Session/ManagedSessionTests.cs |
Tests handler-triggered disposal. |
tests/Opc.Ua.Client.Tests/Session/ManagedSessionReconnectTests.cs |
Tests recovery concurrency. |
tests/Opc.Ua.Client.Tests/Session/ConnectionStateMachineTests.cs |
Tests reconnect budgets and close races. |
tests/Opc.Ua.Client.Tests/Session/ClientFailoverCoordinatorTests.cs |
Tests prepared subscription takeover. |
tests/Opc.Ua.Client.Tests/Session/ClassicSubscriptionEngineTests.cs |
Tests republish and sequence handling. |
tests/Opc.Ua.Client.Tests/NodeCache/NodeCacheTests.cs |
Tests cancellation and empty values. |
tests/Opc.Ua.Client.Tests/NodeCache/NodeCacheContextTests.cs |
Tests node-class mismatches. |
tests/Opc.Ua.Client.Tests/ModelChange/ModelChangeTrackerTests.cs |
Tests joined tracking starts. |
tests/Opc.Ua.Client.Tests/Historian/HistoryClientUnitTests.cs |
Tests null profile handling. |
tests/Opc.Ua.Client.Tests/FileSystem/UaPathTests.cs |
Tests escaped path names. |
tests/Opc.Ua.Client.Tests/FileSystem/TemporaryFileTransferClientTests.cs |
Tests file-generation state handling. |
tests/Opc.Ua.Client.Tests/FileSystem/FileSystemSessionHarness.cs |
Adds deletion simulation support. |
tests/Opc.Ua.Client.Tests/FileSystem/FileSystemClientPathResolutionTests.cs |
Tests path round trips. |
tests/Opc.Ua.Client.Tests/FileSystem/FileSystemClientCrudTests.cs |
Tests deletion and stale-cache behavior. |
tests/Opc.Ua.Client.Tests/ClientBuilder/ReverseConnectManagerTests.cs |
Tests reverse-connect strategies. |
tests/Opc.Ua.Client.Tests/ClientBuilder/OpcUaClientBuilderTests.cs |
Tests managed-session ownership. |
tests/Opc.Ua.Client.Tests/ClientBuilder/OpcUaClientBuilderExtensionsCoverageTests.cs |
Tests redundancy and discovery configuration. |
tests/Opc.Ua.Client.Tests/AuditRegressions/SessionCoreAuditRegressionTests.cs |
Adds session audit regressions. |
tests/Opc.Ua.Client.Tests/AuditRegressions/CoreClientUtilsAuditRegressionTests.cs |
Adds endpoint-selection regressions. |
tests/Opc.Ua.Client.Tests/AliasNames/AliasNameResolverTests.cs |
Tests alias refresh and merging. |
tests/Opc.Ua.Client.ComplexTypes.Tests/Types/EncoderTests.cs |
Tests union encoding and decoding. |
src/Opc.Ua.Gds.Client/ServerPushConfigurationClient.cs |
Rejects missing secure endpoints. |
src/Opc.Ua.Gds.Client/GlobalDiscoveryServerClient.cs |
Rejects missing secure endpoints. |
src/Opc.Ua.Core/Stack/Https/HttpsTransportChannel.cs |
Shares HTTP failure mapping. |
src/Opc.Ua.Client/WebApi/WebApiTransportChannel.cs |
Adds Web API timeout and failure mapping. |
src/Opc.Ua.Client/WebApi/WebApiClient.cs |
Blocks credentials over plaintext HTTP. |
src/Opc.Ua.Client/Utils/AsyncReaderWriterLock.cs |
Adds bounded writer draining. |
src/Opc.Ua.Client/Subscription/Subscription.cs |
Improves resets and saturation handling. |
src/Opc.Ua.Client/Subscription/Streaming/StreamingSubscription.cs |
Fixes disposal races. |
src/Opc.Ua.Client/Subscription/SaturatingTimeSpan.cs |
Adds safe duration conversions. |
src/Opc.Ua.Client/Subscription/MonitoredItemStateSnapshot.cs |
Normalizes empty index ranges. |
src/Opc.Ua.Client/Subscription/MonitoredItemManager.cs |
Defers retries and handles duplicate handles. |
src/Opc.Ua.Client/Subscription/MonitoredItem.cs |
Handles revised intervals and option comparisons. |
src/Opc.Ua.Client/Subscription/Classic/MonitoredItem.cs |
Preserves triggering links. |
src/Opc.Ua.Client/StateMachines/StateMachineTypeClientExtensions.cs |
Reports state failures accurately. |
src/Opc.Ua.Client/StateMachines/FiniteStateMachineTypeClientExtensions.cs |
Resolves instance sub-state machines. |
src/Opc.Ua.Client/Session/Subscription/SessionEngineContext.cs |
Updates background notification dispatch. |
src/Opc.Ua.Client/Session/Subscription/ClassicSubscriptionEngine.cs |
Fixes republish and sequence handling. |
src/Opc.Ua.Client/Session/SessionExtensions.cs |
Handles unknown byte-string limits. |
src/Opc.Ua.Client/Session/SessionClientExtensions.cs |
Validates oversized byte responses. |
src/Opc.Ua.Client/Session/Session.ChannelManager.cs |
Coordinates recovery locking. |
src/Opc.Ua.Client/Session/Redundancy/IServerRedundancyHandler.cs |
Prevents failover from healthy servers. |
src/Opc.Ua.Client/Session/Redundancy/ClientFailoverCoordinator.cs |
Transfers prepared subscriptions. |
src/Opc.Ua.Client/Session/Reconnect/SessionReconnectHandler.cs |
Prevents insecure reconnect fallback. |
src/Opc.Ua.Client/Session/Reconnect/ReconnectPolicyOptions.cs |
Documents reconnect-budget semantics. |
src/Opc.Ua.Client/Session/Reconnect/ReconnectPolicy.cs |
Documents reconnect-budget semantics. |
src/Opc.Ua.Client/Session/Reconnect/ConnectionStateMachine.cs |
Scopes worker flow and close transitions. |
src/Opc.Ua.Client/Roles/RoleManagementClient.cs |
Adds paged role browsing. |
src/Opc.Ua.Client/ReverseConnectManager.cs |
Fixes endpoint replacement and strategy defaults. |
src/Opc.Ua.Client/NodeCache/NodeCache.cs |
Propagates cancellation and preserves value slots. |
src/Opc.Ua.Client/ModelChange/ModelChangeTracker.cs |
Coordinates concurrent tracking starts. |
src/Opc.Ua.Client/Historian/HistoryClient.Extras.cs |
Skips null historical profiles. |
src/Opc.Ua.Client/Fluent/ManagedSessionBuilder.cs |
Adds redundancy options configuration. |
src/Opc.Ua.Client/FileSystem/UaPath.cs |
Escapes reserved path characters. |
src/Opc.Ua.Client/FileSystem/TemporaryFileTransferClient.cs |
Waits for generated files. |
src/Opc.Ua.Client/EventIds.cs |
Adds alias resolver event IDs. |
src/Opc.Ua.Client/CoreClientUtils.cs |
Hardens secure endpoint selection. |
src/Opc.Ua.Client/AliasNames/AliasNameResolver.cs |
Handles refresh failures and duplicate aliases. |
src/Opc.Ua.Client/AliasNames/AliasNameClientErrors.cs |
Maps unsupported alias methods. |
src/Opc.Ua.Client.ComplexTypes/Types/UnionComplexType.cs |
Validates union selectors and JSON fields. |
samples/Reference/ConsoleReferenceClient/UAClient.cs |
Reports endpoint-selection failures. |
docs/HighAvailability.md |
Documents prepared subscription takeover. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Tests used string.Contains(string, StringComparison) and Task.Wait(TimeSpan, CancellationToken), which net472/net48 lack. - WebApiClient now maps HTTP 429/503 to BadServerTooBusy (with the Retry-After hint) before EnsureSuccessStatusCode on every target framework, reusing HttpsTransportChannel.CreateServerTooBusyException. The channel relied on HttpRequestException.StatusCode, which only exists on .NET 5+, so the netstandard2.1 build reported BadUnknownResponse (review finding G15 / RE-1). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code coverage✅ Coverage gate passed.
Uncovered changed lines
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
… readiness - ModelChangeTracker: decide "last start waiter" and detach the generation under one lock, so a caller joining between the two can no longer have its generation cancelled (review G18). - WebApiClient: the public HttpClient constructor now rejects Bearer/Basic credentials over a non-https BaseAddress before installing them, and every send checks the effective base address. - TemporaryFileTransferClient: only ReadTransfer ends the wait for an asynchronously generated file; Idle means not started (Part 20 4.4.3). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
…ent-bugs-786838 # Conflicts: # src/Opc.Ua.Client.ComplexTypes/Types/UnionComplexType.cs Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Resolved the conflicts by merging current |
|
|
# Description
Makes every push to `master` and `release/*` report both required
checks, so a release branch can be cut from `master`.
Two things stopped `master` commits from getting a green `images
summary`, which the `Release` ruleset requires, including when a branch
is created:
1. **Cancellation.** `CI` (`buildandtest.yml`) and `Images CI`
(`docker-image.yml`) used one concurrency group per ref with
`cancel-in-progress: true`. Each merge to `master` cancels the
still-running checks of the commit before it. A queued run in the same
group is replaced as well ("Canceling since a higher priority waiting
request … exists"). `Images CI` takes about 100 minutes, and `master`
currently gets a merge about every 90 minutes:
| Commit | `build-and-test summary` | `images summary` |
| --- | --- | --- |
| `87c35b84b` (#4572) | success | failure: `pumpserver` / `refserver`
cancelled after ~100 min by the #4573 push |
| `20c2d77d5` (#4573) | success | failure: `refserver` / `mcpserver` /
`pumpserver` cancelled by the #4577 push |
The ruleset has no bypass actors, so `git push origin
20c2d77:refs/heads/release/2.0` is rejected with `Required status
check "images summary" is failing`.
2. **Push path filter.** `Images CI`'s push trigger had `paths-ignore:
docs/**, tests/**`. A documentation- or test-only push to `master` or
`release/*` never started the workflow, so it never reported `images
summary` and could never be a cut point.
## Changes
- **Concurrency (both workflows):** pushes to protected lines (`master`,
`main`, `master378`, `release/*`) get one concurrency group per commit
(`github.sha`) and never cancel, so every commit runs its checks to
completion. Pull requests and other branches are unchanged: they group
by PR number or ref and cancel superseded runs.
- The per-commit group is needed as well as `cancel-in-progress: false`.
With a shared group GitHub still replaces a *queued* run.
- **`Images CI` push trigger:** the `paths-ignore` is gone. The
`discover` job now decides relevance for pushes the same way it already
did for pull requests.
- It diffs `github.event.before..HEAD` and skips the image builds for
docs/tests-only changes, while `images summary` still reports.
- A new branch (all-zero `before`) or a `before` commit the clone
doesn't have builds everything, and a failed diff still fails the gate.
- `main` and `master378` are not added to the push trigger: `main`
doesn't exist, and `master378` runs its own copy of this workflow.
## Verification
- The `discover` script was extracted and run locally for push events:
| Push | `relevant_changes` |
| --- | --- |
| docs-only | `false` (images skipped, summary reports) |
| workflow change | `true` |
| new branch | `true` |
| unknown `before` | `true` |
- The CI-related `Opc.Ua.Tools.Tests` (`~Ci`, 53 tests, including the
fail-closed diff pin) pass on net10.0.
## Cost
Overlapping merges to `master` now run CI and image builds in parallel
instead of cancelling each other, which uses more runner minutes on busy
days. Image pushes from overlapping `master` runs can finish out of
order, so `latest-master` reflects whichever finishes last. Version tags
are per build and unaffected.
## Related Issues
- Blocks cutting `release/2.0` for `2.0.0-preview.6` (see #4574).
## 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>


Summary
Fixes the findings of a bug audit of
Opc.Ua.ClientandOpc.Ua.Client.ComplexTypes, done on master plus #4516 (now merged).The finding IDs (
L<area>-<n>for the audit,G<n>for the review) appear in the commit titles.Behaviour and user-facing changes
Security and endpoint selection
SessionReconnectHandlerno longer switches to whatever endpoint the server advertises, None included. It only picks an endpoint at least as strong as the configured one, judged by a locally computed security level. The server-advertisedSecurityLevelis not trusted. If nothing qualifies, reconnect fails with the original error (new log eventSessionReconnectHandler + 13) and keeps retrying with the original settings.CoreClientUtils.SelectEndpoint(useSecurity: true)never returns a SecurityMode None endpoint over opc.tcp (L9-9, High). It now returnsnullwhen no secure endpoint matches.https/opc.https, an HTTPS endpoint with SecurityMode None is still accepted as a last resort, because TLS protects it (Part 6 §7.4.1) (G17).Invalidno longer count as secure.GlobalDiscoveryServerClientandServerPushConfigurationClientnow throwBadSecurityPolicyRejectedwhen no secure endpoint matches, instead of passingnullon. The ConsoleReferenceClient sample reports this with a clear error.http://(L12-2).WebApiClient.Createand WebApi channel open/reconnect reject them withBadSecurityChecksFailed. The publicWebApiClient(HttpClient, …)constructor also rejects them when the client'sBaseAddressis not https, and every send checks the address it actually goes to (covers aBaseAddressset later).Session, keep-alive and identity
TimeoutHint(2 ×KeepAliveInterval), which is what Part 4 §7.32 says the client should do. It then fails withBadNoCommunication, so reconnect starts after about 2 ×KeepAliveIntervalinstead ofOperationTimeout(120 s by default).UpdateIdentityAsync,TransferSubscriptionsAsyncandReactivateSubscriptionsAsyncfail withBadSecureChannelClosed(retryable) instead of deadlocking. This covers the operation holding the reconnect lock and any queued behind it.SessionandManagedSessionClose/Dispose from a Notification handler no longer wait 30 s for their own background drain.ManagedSessionfrom a keep-alivePublishStatusChangedhandler no longer deadlocks.BadSecurityPolicyRejected.ManagedSession.ConfiguredEndpointfollows failover (L2-6). It now reports the inner session's endpoint, so it changes after a failover or network-path rotation.ManagedSession.UpdateSessionAsync(identity)stops the provider's refresh loop and drops the provider for good. If the update fails, the loop is restarted.KeepAliveInterval, 1 s)) only once the channel is unhealthy (keep-alive stopped, channel not Ready), so a dead channel no longer holds recovery forOperationTimeout.Reconnect state machine and reverse connect
MaxTotalReconnectTime = 0now means "no time limit" (L3-7). Reviewer decision needed. Before, 0 disabled reconnect entirely. Negative values other thanTimeout.InfiniteTimeSpannow throwArgumentOutOfRangeExceptionwhen the state machine is constructed; before, they crashed the worker. The XML docs ofReconnectPolicy/ReconnectPolicyOptionsare updated. If 0 should keep meaning "no reconnect", this is a one-line revert.UndefinedandAnyalone are removed after the first callback (L8-1). This matches the documentation; before, they behaved likeAlways.AddEndpointfor a URL that is already added disposes the replaced listener (L8-2).Service calls
Browse/BrowseNext/HistoryRead/CreateMonitoredItemsfails, the continuation points and monitored items created by earlier batches are released or deleted before the exception is rethrown. This adds one request.AddNodes/RegisterNodesare not undone.ReadBytesAsyncrejects oversized responses (L3-5). It throwsBadUnknownResponsewhen the server returns more bytes than requested, andBadEncodingLimitsExceededbeyondintrange.ReadByteStringInChunksAsynchandles an unknown server limit (L3-6). When the server'sMaxByteStringLengthis 0 (absent) or larger thanint, it falls back to the client'sMessageContext.MaxByteStringLength. It throws only when both are unknown.Browse and NodeCache
BadNoDataand releases the continuation point.BrowseAsync(NodeId)now throws instead of silently returning a truncated list.BrowseStreamAsyncyields aBadNoDataresult for that node.RoleManagementClient.ListRolesAsyncand the FileSystem enumerate/empty-check loops.BrowseAsync(NodeId)releases them on any exception, andBrowseStreamAsyncreleases all points of the current page when the consumer stops early.FindAsync,IsKnownAsync,FetchNodeAsync,FindReferenceTypeNameAsync, the reference-table fill and the type-hierarchy search now letOperationCanceledExceptionthrough. Before, a cancelled lookup looked like "node does not exist".BadNodeClassInvalidfor that node instead of throwingKeyNotFoundExceptionfor the whole batch.MaxNodesPerBrowse≥ 2^31 no longer breaks the managed browse (L9-2).ModelChangeTrackerno longer cancels a joined start (L9-11).BrowserContinuationPointExistsButBrowse(Browser + 0) was removed, andBrowserServerReturnedEmptyReferencesButnow includes the round count.Subscriptions (default engine)
SetTriggeringAsyncfrom a callback no longer hangs (L5-7, G21). While the subscription is deleted, recreated or disposed, it returnsBadOperationAbandoned. A cancelled reset no longer makes later calls fail.IndexRangeare treated as equal, and restored snapshots usenull(L5-6).StreamingSubscriptionthrowsObjectDisposedExceptionafter dispose (L6-6).Subscriptions (classic
Subscription)AvailableSequenceNumbers, so other missing messages are not given up while the server still holds them.Dispose()from a keep-alivePublishStatusChangedhandler can return while that callback is still finishing.ReactivateSubscriptionsAsyncnever delete it.Subscription.TransferAsyncnever deletes.CreateAsyncclears stale server ids (L7-6).Good_SubscriptionTransferredfor the client's own transfer is ignored (L7-7).MaxMessageCountis handled (L7-10).LastMessagetolerates a wrong notification type (L7-11).Subscription + 68..70.Redundancy
ManagedSession.AbandonServerSessionAsyncandSession.AbandonServerSession.RoleChanged(false)is now raised before the standby connect and no longer depends on it, soCurrentSessioncan briefly benull.RoleChangedhandlers are logged and do not stop the teardown.TransferIdset to the active client's id) are moved through the session's own transfer and keep publishing.docs/HighAvailability.mdis updated.ShouldFailoverthrough the interface (L4-6). Calling it throughIServerRedundancyHandlerno longer fails over away from a server that is still Healthy.Fluent / dependency injection
AddClientis reconnected after a caller disposes it, and is disposed with the container. After that the connect delegate throwsObjectDisposedException.ManagedSessionBuilder.WithServerRedundancyOptions(ServerRedundancyOptions)(L10-6). This is additive. TheSession.ServerRedundancyoptions are now actually applied.AddDiscoveryAndConnectrewrites the advertised address (L10-7). It replaces the host and port the server advertises with those of the discovery URL (Part 4 §5.6.2.1, NAT/containers), and prefers endpoints with the same scheme.State machines and role management
GetSubStateMachineAsyncreturns the instance's own sub-state machine (L10-1). Before, it returned the type's placeholder.WaitForStateAsyncreports failures with OPC UA status codes (L10-2). It throwsBadNotFoundwhenCurrentState/Idcannot be resolved, andBadInvalidStatewhen observation ends without reaching the state. Before, both surfaced asOperationCanceledException.RoleManagementClient.ListRolesAsyncpages correctly (L10-3). It follows continuation points, releases them on any failure, and throws on a Bad browse status instead of returning an empty list.File system, temporary files and alias names
:and/with&, and&itself as&&before those characters or at the end of a name.FormatandParseround-trip all names. Typed input ending in&/now reads the slash as part of the name.GetInfoAsyncretries once from the server.ExistsAsyncandGetFileAsyncreport "not found" instead of throwing.GenerateFileForReadAsyncwaits for the file (L11-2). It waits until the file-generation state machine reaches ReadTransfer (Idlecounts as not started yet), and throwsIOExceptionif it reports Error. The wait is bounded only by the caller's token.BadMethodInvalidmaps toNotSupportedExceptionfor all alias calls, soUseVerbosefalls back toFindAlias.AliasNameResolver = 560.ServerProfileArrayentries no longer crashGetConformanceInfoAsync.WebApi transport
ServiceResultException(L12-1).BadRequestTimeout.BadServerTooBusywith the Retry-After hint, on every target framework.WebApiClientchecks the status itself (reusingHttpsTransportChannel.CreateServerTooBusyException), so callers ofWebApiClientalso get aServiceResultExceptioninstead of anHttpRequestExceptionfor these two statuses (G15).BadNotConnected.OperationTimeoutapplies to each request. WhenRequestTimeoutis unset, HttpClient's 100 s default no longer cuts off long Publish requests.OperationTimeout, and cancelling the open now throws.HttpsTransportChannel.MapRequestFailurewent from private to internal so the WebApi channel can reuse it.ComplexTypes
BadDecodingErrorwhen decoded andBadEncodingErrorwhen encoded (Part 6 §5.2.8).null.Not fixed / partial
AddNodes/RegisterNodesbatches are not rolled back.ECDHPolicyUrion ActivateSession (SessionSecurityPolicyHelper), which breaks Part 6 §6.8.2. As a result, the L1-4 two-step ECC switch ends inBadSecurityPolicyRejectedagainst our own server.Testing
All suites on net10.0, on the branch before the master merge. Every suite had 0 failures:
After merging current master (net10.0), every suite had 0 failures except one, a flaky new test fixed in 9f52952:
The touched test projects also build on net48 with 0 warnings.
🤖 Generated with Claude Code