mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-21 09:06:37 +08:00
master378
2236 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 romanett
|
359abe68be |
[master378] Complete each incoming request exactly once on server stop (#4729) (#4738)
## Proposed changes Fixes #4729 on master378 (1.5.378.x), the branch the issue was reported against. The master counterpart is #4736. If a server is stopped while requests are queued, a request could be completed twice. The second completion landed on a `ServiceResponsePooledValueTaskSource` that had already gone back to the process-wide pool and been rented by another request. That request, possibly on another server in the same process, then received a foreign response, which showed up as `InvalidCastException` or `BadSecurityChecksFailed`. ### Changes - **Leased completion (root defense, same as #4736):** each rental of the pooled source gets a lease number. `EndpointIncomingRequest` captures it at construction and completes through `TrySetResult(lease, response)`, a single CAS on the pending lease. Only the first completion of the current rental is accepted, and a late completion can never reach the next owner. - **`ServerBase.RequestQueue`:** - `Dispose` drains the queue with `TryDequeue` instead of completing a `ToList()` snapshot. Each request is now removed by exactly one party, either a worker or `Dispose`. This replaces the `m_queue.Clear()` that only compiled under `NETSTANDARD2_1_OR_GREATER`. - The stop flag is now an `int`. `Dispose` sets it with `Interlocked.Exchange`, which is a full fence, and is now idempotent. - `ScheduleIncomingRequest` re-checks the stop flag after enqueuing and drains the queue instead of letting `ObjectDisposedException` from `Release()` escape. Previously that exception made `ProcessAsync` complete the already-queued request a second time. - A worker that dequeues a request after stop completes it with `BadServerHalted` instead of running it. Workers also treat a disposed semaphore as shutdown. - Completions from the queue go through a helper that catches and logs exceptions. ## Tests - `RequestQueueShutdownTests.DisposeCompletesQueuedRequestsOnceAsync`: fails on the current master378 (a request is completed twice) and passes with the fix. - `RequestQueueShutdownTests.ScheduleAfterDisposeCompletesOnce`. - `RequestQueueShutdownTests.ConcurrentScheduleAndDisposeCompletesEachRequestOnceAsync`: a stress guard. It did not reproduce the old bug locally. - `ServiceResponsePooledValueTaskSourceTests`: a duplicate completion is ignored, and a stale completion after the source is recycled does not complete the next rental. - Local net10.0 runs: `Opc.Ua.Core.Tests` Stack.* 1496 passed, 0 failed; `Opc.Ua.Server.Tests` 513 passed, 0 failed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Suciu Mircea Adrian
|
6ea334a17c |
[master378] [CTT] Fix application certificate renewal to use configured trust stores (#4678)
Ignore supplied issuers for the default application group and validate replacements using installed CA certificates. Add regression tests. # Description Certificate renewal could fail when the client omitted the issuer certificate, even though that CA was already installed on the server. Use the server’s configured trust stores and ignore supplied issuers for the default application group. Clients must install CA trust material before renewal; supplied issuers are no longer imported. Existing validation policy and ApplyChanges behavior are preserved. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [X] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [X] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] 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. |
||
|
romanett
|
30d7a31c1d |
[master378] Secure channel: never answer with an expired token; accept a just-expired one (#4701)
Backport of the secure channel fix from #4698 to `master378`. ## Problem (OPC 10000-4 §5.6.2.1) 1. **Server:** after a token renew, the server keeps securing messages with the old token until the client sends something with the new one. That activation only happened on *receive*. A client that sends nothing, for example one whose Publish requests are all outstanding, kept getting responses secured with the old token after it expired. 2. **Client:** the client rejected a response secured with a just-expired token right away (`BadTcpSecureChannelUnknown`) and shut the channel down. The spec says to accept such messages for up to 25 % of the token lifetime. Together these can drop the subscription channel, time out the session and abandon the subscription. Whether it happens depends on timing. ## Fix - `UaSCBinaryChannel.ActivateRenewedTokenIfDue()` activates a pending renewed token once the current one passes `TokenActivationPeriod` (95 %). The receive path now uses it, and `TcpServerChannel.SendResponse` calls it before securing a response. - `ReadSymmetricMessage` accepts responses (not requests) secured with an expired token for `TcpMessageLimits.TokenExpiryGracePeriod` (25 %) of its lifetime. ### Differences from #4698 - Ported to this branch's `HiResClock`-based `ChannelToken` (`Expired` / `ActivationRequired`). There is no `TimeProvider` here, so the new overload is `IsExpired(double gracePeriod)`. - The `FakeTimeProvider`-based `TcpReconnectKeyAgreementTests` fixture does not exist on this branch. It is replaced by `ChannelTokenExpiryTests`, which checks the grace-period and activation thresholds directly. - The CI changes (soak/stress labels, `assert-tests-ran.ps1`) are not backported. `stability-test.yml` on this branch still targets `Opc.Ua.Client.Tests`, which is correct here. ## Test plan - [x] `Opc.Ua.Core.Tests` `Stack.Transport` (net10.0): 58/58 passed, including the 4 new tests - [x] `Opc.Ua.Core.Tests` builds for net48 with 0 warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
32099910dd |
[master378] Align session token, client channel policy check and identity token errors with the spec (#4665)
## Proposed changes Three spec alignment fixes for the 1.5.378 line. - **Session AuthenticationToken** (`SessionManager`): CreateSession now always assigns a random 32 byte opaque AuthenticationToken. Previously a numeric, incrementing token was used for sessions created on a secured endpoint. OPC 10000-6 7.4.1 requires a random token of at least 32 bytes for HTTPS, and a single format keeps all transports consistent. - **OpenSecureChannel response policy** (`UaSCBinaryClientChannel`, `UaSCBinaryChannel.Asymmetric`): a client channel opened with a SecurityPolicy other than None now requires the OpenSecureChannel response to use the same SecurityPolicy and SecurityMode and to carry the server certificate. Previously the response could switch the channel to SecurityPolicy None (OPC 10000-4 5.6.2.1, OPC 10000-6 6.7.2.3). Channels configured for None are unchanged. - **Identity token decryption errors** (`Session`, `RsaUtils`): every failure while decrypting a user identity token is now reported as `Bad_IdentityTokenInvalid` without an inner result, as OPC 10000-4 7.40.2.1 specifies; the cause is logged on the server instead. An invalid length prefix in RSA-decrypted data now fails the same way as other decryption errors instead of `Bad_EndOfStream`. ## Behaviour changes - AuthenticationTokens on secured endpoints are ByteString (opaque) NodeIds instead of numeric NodeIds. - ActivateSession with an undecryptable identity token returns `Bad_IdentityTokenInvalid` in all cases, also when the client requests inner diagnostics. ## Tests - `ClientOpenSecureChannelDowngradeTests` (Core): secured client rejects a None OpenSecureChannel response; None client still accepts it. - `RsaUtilsDecryptTests` (Core): round trip; invalid length prefixes fail with the same exception type as invalid padding. - `SessionSecurityRegressionTests` (Server): opaque 32 byte tokens on a shared secured channel id; uniform ActivateSession status for identity token decryption failures. Local results (net10.0): Core 7938 passed / 0 failed, Server 511 passed / 0 failed, Client 1063 passed / 3 failed. The three Client failures (`TransferSubscriptionNativeAsync` x2, `ShortHaulStabilityTestAsync`) pass when run in isolation on both this branch and unmodified master378. The new tests also pass on net48 (Release). ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
36acf1bb79 |
[master378] Fix GDS client deadlock after a failed keep-alive (#4623)
## Proposed changes Minimal backport of the `GlobalDiscoveryServerClient` keep-alive part of #4158 to `master378`. On a bad keep-alive, `Session_KeepAliveAsync` took `m_lock` and called `Session.Dispose()` on the keep-alive worker. `Session.Dispose()` waits for that worker to stop, so the client deadlocked while holding `m_lock`, and every later call (for example `FindApplicationAsync`) hung. The handler now returns right away and runs the cleanup with `Task.Run`, so the keep-alive worker can stop before the session is disposed. The cleanup skips its work if the client was disposed in the meantime (it handles `ObjectDisposedException` on the lock and checks `m_disposed`). `BackgroundTaskScope` from #4158 is not on 1.5.378, so this uses a plain `Task.Run`. No public API changes. ## Related Issues - Fixes #4621 ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) ## Checklist - [x] Existing GDS client tests pass locally (`Opc.Ua.Gds.Tests` `ClientTest`, net10.0: 102/102) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Suciu Mircea Adrian
|
3b3305355b |
[master378] Fix queued data delivery after durable subscription transfer (#4622)
Force a fresh queue check after transfer so the first Publish returns saved data instead of an empty keep-alive. Reset the subscription’s expiration counter after transfer. Add tests for queued data delivery, the timing bug, and an empty keep-alive after all queued values have been delivered. # Description A durable subscription could return an empty keep-alive on the first Publish after transfer, even when values had accumulated while the client was disconnected. The server could reuse its previous keep-alive state before checking the queues. This change forces a fresh queue check after transfer and resets the subscription's expiration counter. Queued values are delivered first; normal keep-alives resume once the queues are drained. Tests cover the timing bug, data delivery after reconnecting with SendInitialValues=false, and a keep-alive after draining all queued writes, including across multiple Publish responses. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] 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. |
||
|
romanett
|
594eb7a245 |
[master378] Bound nested array preallocation, XML depth and inbound PubSub metadata (#4603)
Minimal backports to the 1.5.378 line of three findings from the codec audit behind #4599 / #4602 (B1-1, X1-1, P1-1). All three can be reached from untrusted input. The changes are kept small, and the performance of legitimate messages is unchanged. ## Changes **B1-1: nested array preallocation (`BinaryDecoder`)** Variant, DataValue, ExtensionObject and encodeable arrays were allocated at the full length prefix before any element was read. Arrays nested in the first element of their parent all see the same remaining message, so a chain of them multiplied the allocation: about 1 KB of input in `RequestHeader.AdditionalHeader` preallocated about 200 MB before authentication. - These arrays now use the #4602 budget. An array is allocated at full length only while all such arrays still being read take at most 8 bytes per remaining message byte. Otherwise it starts at 16 KB and grows as elements are read. - The reservation is released in a `finally`. - Leaf arrays (numbers, strings, NodeIds, …) are unchanged and stay bounded by MaxArrayLength. **X1-1: XML element depth (`XmlDecoder`, `BinaryDecoder`, `JsonDecoder`)** The depth of XmlElement values and XML ExtensionObject bodies was unbounded, and the `InnerXml` setter and `CloneNode(true)` recurse once per level. A known-type XML body of about 200 KB inside a binary message terminated the process with a stack overflow. - Before any network XML is loaded into a DOM, it is now checked with a streaming reader. XML nested deeper than `MaxEncodingNestingLevels` (default 200, the same limit master uses) is rejected with `BadEncodingLimitsExceeded`. - Master's node-by-node copier is not ported. **P1-1: inbound DataSetMetaData (`UaPubSubConnection`)** Received metadata is not authenticated, yet it replaced the metadata of every reader with a matching DataSetWriterId, whichever publisher sent it. - It now only updates readers whose PublisherId filter accepts the sender, using the same rule as data messages. A reader without a PublisherId still accepts any publisher. - Master's registry cap is not needed, because 378 has no registry. ## Verification (net10.0 unless noted) | Check | Result | |---|---| | Nested Variant-array chain, 995 bytes | 199.2 MB → 3.3 MB allocated | | Known-type XML body 30 000 levels deep inside binary | process crash → `BadEncodingLimitsExceeded` | | `Opc.Ua.Types.Tests` (incl. new `DecoderNestingLimitTests`) | 327 / 327; new tests also 9 / 9 on net48 | | `Opc.Ua.Encoders.Fuzz.Tests` (crash/timeout/slow corpus) | 150 / 150 | | `Opc.Ua.Core.Tests` (Encoders + Types) | 6 327 passed, 0 failed | | `Opc.Ua.PubSub.Tests` (without broker-dependent MQTT/UDP) + new `MetaDataPublisherFilterTests` | 617 + 4 passed, 13 skipped | | Types / Core / PubSub build, all TFMs | 0 warnings | **Performance vs. db39629d2:** - Test cases: decoding `Variant[100000]`, `Variant(Variant[])`, `DataValue[10000]`, `ReadValueId[20000]` and `ExtensionObject[20000]`. - Allocations are byte-identical to the baseline. - Median times are within run-to-run noise over two interleaved rounds. - The only new cost is one extra streaming parse for XmlElement values. ## Notes for review - If a deployment raises `MaxEncodingNestingLevels` well above the default, the allowed XML depth rises with it. Master behaves the same way. - No public API changes. The XML depth helper is internal to `Opc.Ua.Types`, and `JsonDecoder` in `Opc.Ua.Core` has a private copy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> |
||
|
Suciu Mircea Adrian
|
db39629d2a |
[CTT][master378] Fix ServerConfiguration URI values for CTT push-management checks (#4592)
Populate ApplicationUri and ProductUri from application configuration, guarding absent optional nodes. Add push tests verifying both values match the advertised server identity. # Description Addresses the reported CTT complaint for Push Model for Global Certificate and TrustList Management, where these ServerConfiguration properties returned null: - ApplicationUri (ns=0;i=25706) - ProductUri (ns=0;i=25725) Initializes both properties from the application configuration, with guards for absent optional nodes. Validation: regression tests verify Good status and nonempty values matching the advertised server identity. All five variants pass; the full push fixture reports 116 passed, 9 expected skips, and no failures on .NET 10. CTT itself has not been rerun. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [X ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [X] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [X] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
Suciu Mircea Adrian
|
077d67f5ae |
Master378 fix incomplete message buffering (#4550)
# Description
Backport the essential incomplete-message memory protection from
|
||
|
Suciu Mircea Adrian
|
3af78d7711 |
Remove unused fields from ReferenceNodeManager (#4472)
# Description Remove unused fields from ReferenceNodeManager |
||
|
Suciu Mircea Adrian
|
dfdb9bb384 |
Fix signed builds for Quickstarts.Servers (#4466)
# Description Test assembly permission moved from AssemblyInfo.cs to the project file, matching the other projects and allowing the required public key to be added automatically. |
||
|
Suciu Mircea Adrian
|
c896c673ae |
Update Microsoft.SourceLink.GitHub and Microsoft.SourceLink.AzureRepos.Git from 10.0.102 to 10.0.111 (#4465)
Update Microsoft.SourceLink.GitHub and Microsoft.SourceLink.AzureRepos.Git from 10.0.102 to 10.0.111 |
||
|
Suciu Mircea Adrian
|
dcfd4df45f |
Master378 backport ctt changes (#4326)
# Description Backport CTT related changes from master into master378. |
||
|
romanett
|
5c1037c274 |
[master378] Send the filtered retain trailing event with a client specific Retain = false (#4454)
# Description Backport of #4453 to the 1.5.378 line. When `SupportsFilteredRetain` is true and a condition drops out of a client's where clause, the server sends one trailing event to that client. Before this change the trailing event carried the condition's real `Retain` value, typically `true`, so a spec-following client kept an alarm on its display that it should have dropped. OPC 10000-9, 5.5.2 (Figure 11) and the "Retain sent" column of Table B.3 require that event to be sent with a client specific `Retain = false`. The value is per client and per filter, so it is applied inside the monitored item: * `CanSendFilteredAlarm` gains an `out bool overrideRetain` parameter that is `true` only for the trailing out-of-scope event. The protected signature is changed in place rather than overloaded. * For that event the event fields are read through a new internal `FilteredRetainTarget` decorator. It delegates everything to the original `IFilterTarget` and returns `false` for a `Retain` value clause the target actually resolves. A clause the type check rejects stays null so the field list keeps its shape. * The shared `InstanceStateSnapshot` that `ReportEvent` fans out to every monitored item is never touched, so other subscriptions whose filter the condition still passes keep receiving the server's real value. The queued `EventFieldList` keeps the original instance as its `Handle`, so reference-based duplicate detection and node manager handle lookups are unaffected. Differences from the master change: this branch has neither the #4377 filtered-retain rework nor `docs/AlarmsAndConditions.md`, so the decorator is written against the `object`-based `IFilterTarget` of this line, the existing tracking logic is left as it is, and there is no documentation change. The reflection-based test helper reads the new out parameter back from the argument array. Tests added in `FilterRetainTests`: * the step-5 event from the issue queued through two monitored items sharing one snapshot: the out-of-scope client receives `Retain = false`, the still-passing client receives `true`, and the snapshot itself still reports `true`; * the override applies to the single trailing event only and the server value returns once the condition re-enters scope; * an unresolvable `Retain` clause stays null; * the existing 16-step Table B.3 test now also asserts the "Retain sent" column for the trailing rows. ## Related Issues - Fixes #4449 for 1.5.378 - Backport of #4453 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
Suciu Mircea Adrian
|
0744066357 |
[master378] Fix Publish with maximum TimeoutHint (#4373)
# Description Fixes server side handling of PublishRequests with maximum TimeoutHint on master378 branch. ## Related Issues - Fixes master378 coresponding 4371 issue ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] 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. |
||
|
romanett
|
6ad0d95b26 |
Fix reverse connect hold time when several Servers share one listener (#4341)
# Description Fixes the reverse connect problem reported in #3985: a Client which uses one reverse connect listener port for several Servers connects to the first Server, and every following `WaitForConnectionAsync` times out. A separate listener port per Server was the only workaround. ## Root cause Confirming the analysis in [mrsuciu's comment on #3985](https://github.com/OPCFoundation/UA-.NETStandard/issues/3985): - Servers which send `ReverseHello` before the application registered a waiting connection for them are held in `OnConnectionWaitingAsync` for `HoldTime`. - All held connections share a single `CancellationTokenSource`, so `RegisterWaitingConnection` wakes up **every** held connection, not only the one it was registered for. - The hold loop was `while (!matched) { ... break; }` with an unconditional `break`, so each woken connection got exactly one re-match attempt. The connections which did not match the new registration fell out of the loop unaccepted, although most of their own hold time was left. - An unaccepted connection is force faulted by the listener (`TcpReverseConnectChannel`, "The reverse connection was rejected by the client"), so that Server is dropped and only returns on its next `ReverseHello` interval, long after the Client timed out waiting for it. Separate ports avoid the shared wakeup path, which is exactly why that workaround succeeds. ## Changes - `ReverseConnectManager.OnConnectionWaitingAsync` re-arms the hold after a wakeup: it re-matches, and if the registration which caused the wakeup was for another Server the connection keeps waiting for the remainder of its own hold time. The loop is still bounded by `HoldTime`, so a Server which is never registered for is rejected exactly as before. - A shutdown flag set by `StopService`/`Dispose` (cleared by `StartService`) releases held connections immediately during teardown, instead of parking them until the hold time expires. `Dispose` now also cancels the token source before disposing it, so a held callback can no longer read a disposed token. - `Docs/ReverseConnect.md`: new "Sharing a listener across multiple Servers" section documenting that the listener port staying in `LISTENING` is expected, that one shared `ReverseConnectManager` should serve all Servers on the same Client Url, and the `HoldTime`/`WaitTimeout` semantics. No public API, signature or serialization change. ## Validation - New `ReverseConnectManagerUnitTests` (3 tests): the two Server regression, hold time expiry still rejecting, and dispose releasing a held connection. They drive the callback through an internal test hook, so no listener or socket is involved. Verified that the regression test fails against the previous behaviour with "The reverse connection of the second server was released before its hold time expired." - New tests pass on net10.0 and net48. - Full reverse connect suite (`FullyQualifiedName~ReverseConnect`, including the existing `ReverseConnectTest` server fixture): 33/33 pass on net10.0. ## Related Issues - Relates to #3985. - `master` carries the identical defect: the same `break` survived #4009 and #4059 and is still present in `src/Opc.Ua.Client/ReverseConnectManager.cs`. The equivalent fix for `master` is #4342 (it uses `TimeProvider` and breaks on a pending callback drain instead of a new flag), so the issue is intentionally not auto closed here. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
romanett
|
2ae373f8d3 |
Fix ServerTimestamp update on read of stored/static nodes (#4257) (#4260)
## Summary Backports the fix for ServerTimestamp updating on Read operations for stored/static nodes to master378 (1.5.378), addressing issue #4257 (part 2). ## Details - In CustomNodeManager and CoreNodeManager, ensure ServerTimestamp is stamped with the read time for stored/static nodes whose SourceTimestamp is older than eadTime (e.g. after a Write). - Retain matching SourceTimestamp and ServerTimestamp for dynamically produced values generated during Read (such as ServerStatus children). - Adds unit test ReadStaticVariableStampsFreshServerTimestampAfterWriteAsync in ReferenceServerTest.cs to verify that after writing to a static variable, subsequent reads return ServerTimestamp stamped with the current read time. Fixes #4257 (ServerTimestamp behavior) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
romanett
|
a240bf8fe3 |
Default TransportQuotas in ApplicationConfiguration.ValidateAsync to prevent server-start NullReferenceException (backport to master378) (#4264)
Backport of https://github.com/OPCFoundation/UA-.NETStandard/commit/9883c41e48992a7cbc7347f9b2f1e50c8a3c1df6 (#4055) from `master` to `master378`. ## Description `ValidateAsync(ApplicationType.Server)` accepted a null `TransportQuotas` without error, but the transport/secure-channel layers assume it is non-null, so `ApplicationInstance.StartAsync` later threw a `NullReferenceException`. The fluent builder path (`ApplicationInstance.Build`) already always initializes `TransportQuotas = new TransportQuotas()`; manually-assembled configurations could leave it null. Changes: - **`ApplicationConfiguration.ValidateAsync`**: default `TransportQuotas` when null, aligning the manual-construction path with the builder path. ```csharp // Transport/secure-channel layers rely on this being non-null during bring-up. TransportQuotas ??= new TransportQuotas(); ``` `TransportQuotas`'s default constructor yields complete, valid limits (`DefaultEncodingLimits` / `TcpMessageLimits`), so minimal server configs now start with sensible defaults instead of crashing. - **Tests** (`ApplicationConfigurationTests`, new file on this branch): `ValidateAsyncDefaultsTransportQuotasWhenNull`, `ValidateAsyncKeepsExplicitTransportQuotas`, and `ValidateAsyncDefaultsTransportQuotasWhenNullForClient`. ## Notes - Ported to `master378`. The test fixture did not exist on this branch, so it was added. - Built `Opc.Ua.Core.Tests` (net8.0) with 0 warnings; the 3 new tests pass. ## Checklist - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
romanett
|
d97bb4566e |
Set ContinuationPoint to null when browse is finished (port of #4057) (#4266)
Ports the ContinuationPoint fix from master commit
|
||
|
romanett
|
a2782191b1 |
fix: scope Cancel request matching to the current session (#4023) (#4267)
Backport of
|
||
|
romanett
|
10b948db8d |
fix: don't return continuation points on BadNoContinuationPoints (#4022) [backport to master378] (#4268)
### Summary
Backport of commit
|
||
|
romanett
|
f6549e2612 |
Fix unordered server endpoint validation (#4029) [backport to master378] (#4269)
Backport of OPCFoundation/UA-.NETStandard@8f4087605a from `master` to `master378`. ### Summary Makes `Session.ValidateServerEndpoints()` compare `ServerEndpoints` and `UserIdentityTokens` as unordered sets and adds regression tests verifying `OpenAsync()` no longer rejects legal servers that return the same endpoint data in a different order. ### Problem Per OPC UA Part 4, the client validates the endpoint set returned by `CreateSessionResponse.ServerEndpoints` against the set observed during discovery. The spec describes these as a *set* of endpoint descriptions filtered by transport profile, not an ordered array. Previously the validation compared `m_discoveryServerEndpoints[ii]` against `serverEndpoints[ii]` by index, and `UserIdentityTokens[jj]` by index. A server returning the same legal endpoints/token policies in a different order could be wrongly rejected with `BadSecurityChecksFailed`. ### Changes - Replace index-based `ServerEndpoints` validation with unordered matching on the same endpoint fields already validated by the client. - Compare `UserIdentityTokens` as an unordered multiset. - Extend the client session test scaffolding so discovery endpoints can be supplied explicitly. - Add regression tests for reordered `ServerEndpoints` and reordered `UserIdentityTokens`. ### Notes Adapted to the `master378` API surface (`EndpointDescriptionCollection` / `UserTokenPolicyCollection` / `StringCollection` / `byte[]` server nonce) and the `Libraries/`/`Tests/` layout. Client tests build and the new tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
romanett
|
adfe814024 |
fix(server): enforce browse continuation point limit at capacity (#4036) (#4270)
Backports the browse continuation point limit fix from master (commit
|
||
|
romanett
|
4ea1e25060 |
Treat zero MaxNotificationsPerPublish as unlimited (OPC 10000-4) (#4263)
## Proposed changes Backport commit https://github.com/OPCFoundation/UA-.NETStandard/commit/a5dd6eae7cc1ab074d62d723194285952560555f (#4047) to `master378`. OPC UA defines zero as no notification limit. The message builder compared the queued count directly with zero, so an effective zero limit emitted no event or data-change notifications and left the queues pending. Separately, the revision logic treated a configured Server maximum of zero as a numeric cap, revising a non-zero Client request to zero and silently removing the Client's requested limit. ### Fix - Treat an effective zero limit as unlimited while draining event and data-change queues. - When the Client requests zero, use the Server's configured maximum, which may itself remain zero/unlimited. - When the Server maximum is zero, preserve a non-zero Client request, including `uint.MaxValue`. - When the Server maximum is finite, cap zero, `uint.MaxValue`, and larger finite Client requests to that maximum. - Cover the same revision matrix through both CreateSubscription and ModifySubscription. ## Related Issues - Backport of #4047 (commit `a5dd6eae7cc1ab074d62d723194285952560555f`) ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) - [ ] Enhancement (non-breaking change which adds functionality) - [x] Test enhancement (non-breaking change to increase test coverage) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages) - [ ] Documentation Update (if none of the other choices apply) ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf). - [x] I ran tests locally with my changes, all passed. - [ ] I fixed all failing tests in the CI pipelines. - [ ] I fixed all introduced issues with CodeQL and LGTM. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added necessary documentation (if appropriate). - [ ] Any dependent changes have been merged and published in downstream modules. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
KarenKrill
|
db256c6a81 |
Fixed timer leaks in ChannelAsyncOperation.EndAsync (#3669) (#4166)
Fixed timer leaks in ChannelAsyncOperation.EndAsync ## Proposed changes - Added linked cancellation token source for delay task (runtimes <= .NET 6.0) - Added delay task cancellation (runtimes <= .NET 6.0) ## Related Issues - Fixes #3668 ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) - [ ] Enhancement (non-breaking change which adds functionality) - [ ] Test enhancement (non-breaking change to increase test coverage) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages) - [ ] Documentation Update (if none of the other choices apply) ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf). - [ ] I ran tests locally with my changes, all passed. - [ ] I fixed all failing tests in the CI pipelines. - [ ] I fixed all introduced issues with CodeQL and LGTM. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added necessary documentation (if appropriate). - [ ] Any dependent changes have been merged and published in downstream modules. ## Further comments Fix already accepted in master branch, please add it to latest 1.5.378 version --------- Co-authored-by: Suciu Mircea Adrian <Mircea-Adrian.Suciu@softing.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
Copilot
|
940a391664 |
Add documentation for custom NodeSet2 authoring workflow (#4170)
## Proposed changes
This change adds a focused guide for users building their own NodeSet2
models with the .NET Standard stack. It documents the intended workflow
discussed in the issue: author in ModelDesign, generate artifacts with
ModelCompiler, and create runtime instances in `CreateAddressSpace`
instead of hand-maintaining generated NodeSet2 content.
- **New workflow guide**
- Added `Docs/CustomNodeSet2Workflow.md`
- Documents the recommended split between:
- `ModelDesign` as the source of truth
- ModelCompiler outputs (`NodeSet2`, CSV, generated classes/constants,
predefined nodes)
- runtime instantiation in server code
- **Runtime guidance clarified**
- Explains when to use `CreateAddressSpace` vs.
`AddBehaviourToPredefinedNode`
- Calls out the common case of selecting one machine variant at runtime
from several modeled types
- Describes why generated instance children receive distinct NodeIds and
why manually reusing explicit NodeIds causes confusion
- **Modeling guidance clarified**
- Recommends making inherited optional children mandatory in the derived
type when the server always exposes them
- Positions generated `NodeSet2` as an output artifact, not the primary
hand-edited source
Example from the documented approach:
```xml
<opc:ObjectType SymbolicName="GearMachineType" BaseType="MachineTool:MachineToolType">
<opc:Children>
<opc:Property SymbolicName="DI:Model"
DataType="ua:LocalizedText"
ModellingRule="Mandatory" />
</opc:Children>
</opc:ObjectType>
```
## Related Issues
## Types of changes
What types of changes does your code introduce?
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR._
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] Enhancement (non-breaking change which adds functionality)
- [ ] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [x] Documentation Update (if none of the other choices apply)
## Checklist
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR. If you're unsure about any of them, don't hesitate to
ask. We're here to help! This is simply a reminder of what we are going
to look for before merging your code._
- [ ] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [ ] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines.
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.
## Further comments
The guide is anchored to existing quickstart patterns already present in
the repository, so it explains the current intended workflow without
introducing new tooling or changing runtime behavior.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes #4169
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
|
||
|
Marc Schier
|
d2fa85c989 |
Fix BadContinuationPointInvalid caused by empty continuation point in Browser.BrowseAsync (#4168)
Re-submission of #4167 against `master378`. # Description #4167 was merged directly into `release/1.5.378`. Maintenance fixes for the 1.5.378 lineage have to land on `master378` first, so the commit was removed from `release/1.5.378` again and the fix is submitted here instead. Servers 1.6.0 and higher return a **zero length** continuation point in the `Browse` response to indicate that no further references are available. `Browser.BrowseAsync` only checked the continuation point for `null`, so it issued a `BrowseNext` call with an empty continuation point, which the server rejects with `BadContinuationPointInvalid`. The fix follows the continuation point only if it actually contains data. This is consistent with the already existing `continuationPoint?.Length > 0` check in the `OperationCanceledException` filter of the same method. Credit for the original fix goes to @KarenKrill (#4167). ## Related Issues - Fixes #3698 - Supersedes #4167 ## 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 the affected tests locally. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ## Further comments New unit tests in `Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs` pin the behaviour for `null`, empty and non empty continuation points. Verified that the new tests fail without the one line change and pass with it: `dotnet test Tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj -f net10.0 --filter "FullyQualifiedName~BrowserUnitTests"` -> `Failed: 0, Passed: 3` The issue does not exist on `master` (2.0), where the `ByteString` refactoring already handles this. Co-authored-by: KarenKrill <74286712+KarenKrill@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 75db5878-10f9-4099-993a-5eb969fa080e |
||
|
Copilot
|
6739b8c7e0 |
Preserve publish queue timestamp ordering (#4084)
## Proposed changes The master378 publish timer reset timestamps for already-ready subscriptions, breaking oldest-first ordering among equal-priority subscriptions. - Restore the ready-subscription guard. - Recheck readiness under the queue lock to prevent concurrent timestamp resets. - Replace invalid requeue coverage with a deterministic ordering regression test. ## Related Issues ## Types of changes What types of changes does your code introduce? _Put an `x` in the boxes that apply. You can also fill these out after creating the PR._ - [x] Bugfix (non-breaking change which fixes an issue) - [ ] Enhancement (non-breaking change which adds functionality) - [x] Test enhancement (non-breaking change to increase test coverage) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages) - [ ] Documentation Update (if none of the other choices apply) ## Checklist _Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._ - [ ] I have read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf). - [x] I ran tests locally with my changes, all passed. - [ ] I fixed all failing tests in the CI pipelines. - [ ] I fixed all introduced issues with CodeQL and LGTM. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added necessary documentation (if appropriate). - [ ] Any dependent changes have been merged and published in downstream modules. ## Further comments Retains the single-lock lost-wakeup correction from the preceding change while preserving subscription scheduling timestamps. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #3997 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 892a02b2-efa4-4a3b-bc9a-5e42103f4c26 |
||
|
Copilot
|
23063afacb |
Obsolete legacy TraceConfiguration in favor of ITelemetryContext (#4090)
## Proposed changes
`ApplicationConfiguration.TraceConfiguration` still existed as a live
configuration surface after `ITelemetryContext` was introduced, which
made the migration path unclear. This change makes that legacy surface
explicitly discoverable as transitional API and points consumers to the
telemetry-based replacement.
- **Public API guidance**
- Marks `ApplicationConfiguration.TraceConfiguration` as obsolete with a
message directing consumers to `ITelemetryContext` and `ILogger`-based
diagnostics.
- Keeps the property functional so existing configuration loading and
legacy trace application continue to work during migration.
- **Internal compatibility**
- Adds targeted `CS0618` suppressions at the remaining internal legacy
call sites that intentionally preserve `TraceConfiguration` behavior.
- Avoids broad warning suppression; only the compatibility path is
annotated.
- **Coverage**
- Adds a focused reflection-based test that verifies the property is
marked obsolete and that the migration message stays stable.
```csharp
[Obsolete("Use ITelemetryContext and ILogger-based diagnostics instead.")]
public TraceConfiguration TraceConfiguration { get; set; }
```
## Related Issues
## Types of changes
What types of changes does your code introduce?
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR._
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] Enhancement (non-breaking change which adds functionality)
- [x] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)
## Checklist
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR. If you're unsure about any of them, don't hesitate to
ask. We're here to help! This is simply a reminder of what we are going
to look for before merging your code._
- [ ] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines.
- [x] I fixed all introduced issues with CodeQL and LGTM.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.
## Further comments
This is intentionally narrow in scope: it clarifies the migration path
without removing the legacy trace pipeline yet. The change is limited to
surfacing the deprecation at the public configuration boundary and
isolating the remaining intentional legacy usages.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes #4056
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
|
||
|
Marc Schier
|
7e8f9e528a |
Fix publish queue lost wake-up (#4017)
# Description Fixes a server-side lost wake-up in `SessionPublishQueue` that can permanently stop monitored-item notifications while the Session and SecureChannel remain healthy. In 1.5.378, `PublishAsync` checked for a ready Subscription under `m_subscriptionPublishLock` and then queued the Publish request under `m_lock`. A Subscription could become ready between those operations, be marked `ReadyToPublish` because no request was queued yet, and then leave the newly queued request stranded. Later timer ticks skipped assignment because the Subscription was already marked ready. This change backports the relevant `SessionPublishQueue` correction from #3611 and incorporates the review follow-up: - Makes the ready-Subscription check and Publish-request enqueue atomic under `m_lock`. - Synchronizes `PublishCompleted`, `Requeue`, and timer-driven assignment with the same lock. - Removes `m_subscriptionPublishLock`. - Invokes `SessionClosed` callbacks outside the queue lock. - Retries timer-driven assignment for already-ready, non-publishing Subscriptions instead of skipping them. - Adds deterministic regression tests for the lost-wakeup and requeue paths. ## Related Issues - Fixes #3997 - Backports the relevant queue fix from #3611 ## Testing - The focused `SessionPublishQueueRaceTests` fixture passes on net472, net48, net8.0, net9.0, and net10.0. - Earlier branch validation ran the full `UA.slnx` on net10.0 and net48 with zero failures. - Three pre-existing Quickstarts CA1823 warnings remain unchanged. |
||
|
Suciu Mircea Adrian
|
52294d279a |
Revert "Prune extra decoded children that should stay on the type defnition only. (#3843)" (#3976)
This reverts commit
|
||
|
Marc Schier
|
33bd8faf63 |
Fix macOS CI: avoid empty-KeyUsage test cert (#3953)
## Summary Follow-up to #3948 (merged) that fixes a **macOS-only** CI failure introduced by the issuer/CA KeyUsage tests. On the macOS Core test job, three tests failed with: ``` Interop+AppleCrypto+AppleCommonCryptoCryptographicException : Unknown format in import. at System.Security.Cryptography.X509Certificates.X509Pal.AppleX509Pal.GetCertContentType(...) at System.Security.Cryptography.X509Certificates.X509CertificateLoader.LoadCertificate(...) ``` The affected tests built a test CA with an **empty** KeyUsage extension (`new X509KeyUsageExtension(X509KeyUsageFlags.None, true)` → a zero-bit `BIT STRING`). Windows (CryptoAPI) and Linux (OpenSSL) load such a certificate, but macOS (AppleCrypto) rejects it as an unknown format. The production code is unaffected — this is purely a test-certificate artifact. ## Changes - Removed the degenerate empty-KeyUsage test certificate. The **truly-absent** KeyUsage case (built via `CertificateRequest` so no KeyUsage extension is emitted) already covers the `GetKeyUsage() == None` code path. - The reject/suppress integration tests now use a present-but-insufficient `DigitalSignature` KeyUsage (a valid, macOS-loadable certificate that is still missing `keyCertSign`/`cRLSign`). ## Testing - `CertificateValidator` IssuerKeyUsage tests pass on **Windows** and **Linux** (net10.0). The change directly removes the macOS root cause (the empty-KeyUsage extension); all remaining test certificates use standard formats that macOS already loads (the compliant-CA test passed on macOS). Relates to #3944 · follow-up to #3948 |
||
|
Marc Schier
|
21eccddb1c |
Validate issuer/CA certificate KeyUsage (keyCertSign, cRLSign) [master378 backport] (#3948)
## Summary **Backport of the #3944 fix to the `master378` (1.5.378) maintenance branch.** See the 2.0 PR #3947 for the primary change; this PR applies the equivalent fix to the 1.5.378 code. The stack did not verify that **CA/issuer certificates** in a chain assert the KeyUsage required for a CA. Per **OPC 10000-6 §6.2.4, Table 52**, a CA certificate must carry a KeyUsage extension asserting `keyCertSign` and `cRLSign`, and **OPC 10000-4 §6.1.3, Table 100 ("Certificate Usage")** requires validators to check issuer usage and report `Bad_CertificateIssuerUseNotAllowed`. The validator only checked the **leaf** certificate's KeyUsage and delegated issuer/CA usage to .NET `X509Chain`, which (RFC 5280 §4.2.1.3) does **not** reject a CA whose KeyUsage extension is *absent* — so a CA with no KeyUsage was accepted while strict third-party stacks reject the chain with `BadCertificateInvalid`. ## Changes - `X509Utils.HasRequiredIssuerKeyUsage(X509Certificate2)` — returns whether both `keyCertSign` and `cRLSign` are asserted (absent KeyUsage → `None` → fails). - `CertificateValidator.InternalValidateAsync` — after the leaf KeyUsage check, verifies every issuer/CA certificate in the chain and appends a **suppressible** `Bad_CertificateIssuerUseNotAllowed` when the required bits are missing. - **Two coupled defects fixed in `CheckChainStatus`**: 1. A `FormatException` from a named-placeholder format string (`{Status}: {Information}` → positional `{0}: {1}`) that crashed validation on any `NotValidForUsage` chain status. 2. An inverted `isIssuer` status-code ternary (issuer↔leaf codes swapped, contrary to Part 4 Table 100). - `Docs/Certificates.md` — new "CA (issuer) KeyUsage validation" subsection. Every new check/helper carries an in-code citation to the governing spec sections. ## Testing - New tests in `CertificateValidatorTest.cs` (matching the branch's existing style: `X509Certificate2`, classic asserts, throwing contract, event-based approver): a deterministic helper unit test plus reject / suppress / accept integration tests. - Full `CertificateValidator` fixture green on **net8.0** and **net48**, no regressions. ## Compatibility - This is long-standing behaviour on both `master` and `master378` — not a 2.0 regression. No public API change; the new error is suppressible. Relates to #3944 · 2.0 PR: #3947 |
||
|
Copilot
|
6a304ef8e2 |
Fix GDS Push UpdateCertificate ignoring configured SecurityConfiguration policy (master378) (#3936)
## Proposed changes Backport of the #3931 fix to master378. On this branch, server-side GDS Push `UpdateCertificate` validated the new application certificate with a throwaway `CertificateValidator` that was never seeded with the server's `SecurityConfiguration`. It therefore enforced hard-coded defaults (`MinimumCertificateKeySize = 2048`, `RejectSHA1SignedCertificates = true`) regardless of operator configuration — e.g. a server configured for a 1024-bit minimum still rejected a re-issued 1024-bit certificate with `BadCertificatePolicyCheckFailed`. Regression originated in #3795 (integrity check switched from a lenient `X509Chain` check to the full validator). The master branch reworked this area entirely, so the fix is re-implemented from scratch here, adapted to master378's `X509Certificate2` types. - **`ConfigurationNodeManager.UpdateCertificateAsync`**: replaced the throwaway `CertificateValidator` with a lenient `X509Chain` integrity check that verifies only chain integrity — valid signatures and a complete issuer chain — using the caller-supplied issuers as the trust anchor (`AllowUnknownCertificateAuthority`, revocation `NoCheck`, downloads disabled). Policy (min key size, SHA-1, revocation) is intentionally **not** applied here; it remains governed by the server's `SecurityConfiguration` on its normal communication channels, so configured values are honored in both stricter and more-lenient directions. Chain status handling: `NoError`/`UntrustedRoot` pass; `NotSignatureValid`/`PartialChain`/`NotValidForUsage`/`InvalidBasicConstraints` fail with `BadSecurityChecksFailed`; and the chain element count must match the supplied issuer count to reject incomplete chains. |
||
|
Copilot
|
5924fe6db3 |
Synchronize ConditionState branch collection access (#3895)
# Description `ConditionState.m_branches` (a plain `Dictionary`) was read and written from multiple threads without synchronization. A state change on a background thread could mutate the dictionary while readers (`GetRetainState`/`UpdateRetainState`, `ConditionRefresh`, `GetBranch`) enumerated it, yielding torn entries and native access violations (observed as `IsBranch` on a null `this`). This ports the master378 fix to `master`, where the type lives at `Stack/Opc.Ua.Core.Types/State/ConditionState.cs` and uses `ByteString`/nullable refs. - **Lock all branch access**: added a private `m_branchesLock` guarding every read/write of `m_branches` (`CreateBranch`, `GetBranch`, `ReplaceBranchEvent`, `RemoveBranchEvent`, `ClearBranches`, `GetBranchCount`). - **Snapshot enumeration**: `GetBranches()` now returns a copy taken under the lock, so internal readers and `AlarmHolder.GetBranchesForConditionRefresh` iterate a stable collection while other threads mutate the live one. Public signature is unchanged. - **Test**: added `ConcurrentBranchAccessIsThreadSafe`, stressing concurrent create/clear writers against enumerate/count readers. `lock` is used rather than `SemaphoreSlim` because all affected methods are synchronous. Each `ConditionState` holds its own lock, so the recursive `UpdateRetainState` traversal across branches neither contends nor deadlocks. `ConcurrentDictionary` was rejected: it would change the public return type and still leave compound read-modify-write sequences (e.g. `ReplaceBranchEvent`) unprotected. ## Related Issues ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Suciu Mircea Adrian <Mircea-Adrian.Suciu@softing.com> |
||
|
Marc Schier
|
35708e81d3 |
[Server] Fix #3896: send full certificate chain when CA is in the issuer store (1.5.x) (#3898)
# Description Fixes #3896 on the `1.5.x` maintenance line. A server configured with `SecurityConfiguration.SendCertificateChain = true`, whose application instance certificate is issued by a CA placed in the **issuer store** (`TrustedIssuerCertificates`), sent only the **leaf** certificate instead of the full chain. Clients that rely on receiving the chain then failed to connect. ## Root cause Introduced in `1.5.375` by commit `f001eabcc` ("Add support for ECC profiles", #2398), which moved chain loading into `CertificateTypesProvider.LoadCertificateChainAsync`: ```csharp if (await m_certificateValidator.GetIssuersAsync(certificate, issuers).ConfigureAwait(false)) { for (int i = 0; i < issuers.Count; i++) { certificateChain.Add(issuers[i].Certificate); } } ``` `GetIssuersAsync` returns `isTrusted` — `true` only when the issuer is found in the **trusted** store — but populates the `issuers` out-list regardless (trusted → issuer → untrusted). When the CA lives in the **issuer store** (the spec-recommended setup), the call returns `false`, so the already-resolved issuers were discarded and the chain stayed leaf-only. This is a regression from `1.5.374`, where the issuers were appended **unconditionally**. ## Fix Append the resolved issuers unconditionally; the boolean trust result is intentionally ignored — a server must send its chain even when the issuing CA is in the issuer store rather than the trusted store. ```csharp _ = await m_certificateValidator.GetIssuersAsync(certificate, issuers).ConfigureAwait(false); for (int i = 0; i < issuers.Count; i++) { certificateChain.Add(issuers[i].Certificate); } ``` The companion fix for the 2.0 line (`master`) is in a separate PR. ## Tests New `CertificateTypesProviderTests`: - `LoadCertificateChainResolvesIssuerFromIssuerStoreAsync`: the CA is placed in the issuer store only — **not** the trusted store — and the resolved chain is asserted to be `leaf + CA` (and the raw blob `leaf || CA` byte-for-byte). Confirmed this test **fails** with the original gate and **passes** with the fix. - `LoadCertificateChainFallsBackToLeafWhenIssuerMissingAsync`: when the CA is in neither store, the chain is leaf-only without throwing. Verified locally on **net48** and **net10.0** (new tests + existing `VerifyIssuerChain*` validator tests pass). Build is clean. ## Related Issues Fixes #3896. ## 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 (no public API change; behavioural fix only). - [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. (Ran the certificate suites on net48 + net10.0; full-solution run pending CI.) - [ ] 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: agent <agent@local> |
||
|
romanett
|
fa9364cd5e |
[Server] Fix NRE in ActivateSession if Session times out before activation completes (#3844)
## Proposed changes Fixes a NullReferenceException (NRE) in `StandardServer.ActivateSessionAsync` that could occur when a session times out between the completion of `SessionManager.ActivateSessionAsync` and the subsequent `GetSession` call. A null-guard (`?? throw new ServiceResultException(StatusCodes.BadSessionIdInvalid)`) is added to handle this race condition safely. ## Related Issues ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) - [ ] Enhancement (non-breaking change which adds functionality) - [ ] Test enhancement (non-breaking change to increase test coverage) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages) - [ ] Documentation Update (if none of the other choices apply) ## Checklist - [ ] I have read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf). - [ ] I ran tests locally with my changes, all passed. - [ ] I fixed all failing tests in the CI pipelines. - [ ] I fixed all introduced issues with CodeQL and LGTM. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added necessary documentation (if appropriate). - [ ] Any dependent changes have been merged and published in downstream modules. ## Further comments The fix is a minimal, targeted change in `StandardServer.cs`. Earlier attempts to also modify `SessionManager.ValidateRequest` and `Session.ValidateRequest` were reverted because removing the `ActivateSession` bypass from `SessionManager.ValidateRequest` caused the secure channel ID check in `Session.ValidateRequest` to run for reconnect-on-alternate-channel scenarios — a valid OPC UA pattern — resulting in `BadSecureChannelIdInvalid` errors in the Client test suite (`ReconnectJWTAsync`, `ReconnectSessionOnAlternateChannelAsync`, etc.). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
Suciu Mircea Adrian
|
00519fa6dd |
Prune extra decoded children that should stay on the type definition only. (#3843)
Keep the generated children that belong on instances, and prune the extra decoded children that should stay on the type definition only, such as Optional/Mandatory placeholders and ones that do not have Mandatory/Optional ModellingRules. # Description Currently child nodes that are type definition detailes such as ones that do not have Mandatory/Optional ModellingRules are instantiated in the runtime instance of such type definitions. This PR proposes to correct this behaviour during NodeState creation as an alternative from it being fixed by the ModelCompiler. ## Related Issues - Fixes #3770 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] 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. |
||
|
Suciu Mircea Adrian
|
c73dafe2d4 |
[master378] Use request.SessionId instead of request.Session.Id in the ReportAuditCancelEvent call from CancelRequests. (#3853)
# Description Use request.SessionId instead of request.Session.Id in the ReportAuditCancelEvent call from CancelRequests to avoid accessing Session in case ActivateSession. Cancel service is returning BadUnexpectedError because audit reporting crashes on request.Session.Id in case the ActivateSession service call is in-flight and its cancelation is requested becasue Session is null. ## Related Issues - Fixes issue 3850 on branch master378 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] 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. |
||
|
Markus Wallén
|
af064312be |
Fix Race condition in OnPublishComplete (#3837)
# Description
The catch block in OnPublishComplete reads m_subscriptions.Count and
m_subscriptions.Any(s => s.Created) without holding m_lock, while all
other accesses (AddSubscription, RemoveSubscriptionAsync,
RemoveTransferredSubscription) correctly lock before modifying the list.
This causes an InvalidOperationException ("Collection was modified;
enumeration operation may not execute") when a publish response errors
concurrently with a subscription being removed during reconnect.
Snapshot both values under m_lock, consistent with all other call sites.
## Related Issues
- Fixes #3835
## Checklist
_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._
- [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.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
|
||
|
Copilot
|
d1812a0cc7 | Fix memory leak: dispose linked CancellationTokenSource in ChannelAsyncOperation.EndAsync (#3812) | ||
|
opcfoundation-org
|
d621313b64 |
Hot fix 1.05.07 2026 05 01 (#3773)
* Update NodeSet for 1.05.07-2026-05-01. * Update NuGets. --------- Co-authored-by: Randy Armstrong <randy-armstrong@users.noreply.github.com> |
||
|
Suciu Mircea Adrian
|
9d43d121f1 |
Remove OptionalPlaceHolders on NodeState Instances (#3573)
* Test that OrderedListState does not instantiate OrderedObjectPlaceholder * Add global NodeState placeholder scan with assembly context * Prune optional placeholder children during instance creation while leaving type-definition trees untouched. |
||
|
Suciu Mircea Adrian
|
fe12ef6b0a |
Merge pull request #3742 from mrsuciu/roman-port-publishing-cert-status-master378
Port two targeted fixes from master to master378: Filter certificate validation StatusCodes and Fix 'moreNotifications' handling |
||
|
mrsuciu
|
d6a92f3827 | Fix subscription tests diagnostics setup order | ||
|
Roman Ettlinger
|
aa29518293 | fix subscription publish bug wrongly reporting moreNotifications as false | ||
|
Roman Ettlinger
|
0380603d49 | filter CertifiicateStatusCodes before returing to client | ||
|
Anatoly Kochnev
|
46825b7579 |
[Client] Fixed excessive tasks spawning during session connection loss (cherry pick PR #3707) (#3738)
Added checks for Session.KeepAliveStopped before trying to execute Publish for subscriptions.
Cherry-pick from PR #3707 (SHA
|
||
|
Marc Schier
|
1f4cf50f26 | Add opc ua mit license text to nugets instead of license expression (378) (#3725) | ||
|
Suciu Mircea Adrian
|
b1338301cc |
Merge pull request #3715 from mrsuciu/fixMissingOperandCrash
Fix for crash when processing invalid eventFilter: WhereClause does not contain operand on master378 |