GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-28 14:09:56 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,369 Commits 75 Branches 105 Tags
fuzzing
Commit Graph
3 Commits
Author SHA1 Message Date
Marc Schierandagent a81d981755 dotnet format sweep (whitespace, IDE, RCS) + code-style enforcer agent fix (#3865) 
# Description

Pure code-style sweep across the repo — no behavioral changes. Drives
`dotnet format` info-level diagnostics down on a large slice of the
codebase (Stack, Libraries, Tests, Tools).

Six commits, applied bottom-up in safety order so each batch leaves the
build green:

1. **`style: fix whitespace and final-newline formatting (WHITESPACE,
FINALNEWLINE)`**
   197 hits across ~30 files; pure mechanical cleanup.

2. **`style: apply dotnet format style fixes (IDE rules)`**
IDE-rule batch via `dotnet format style --severity info` —
`var`/explicit type, simplify names, collection expressions, expression
bodies, unused usings, etc.

3. **`style: apply dotnet format analyzers fixes (RCS blank-line
formatting)`**
   Roslynator formatting (RCS0xxx) blank-line and punctuation rules.

4. **`style: convert comments to doc comments and inline local vars
(RCS1181, RCS1124)`**
24 files. RCS1181 promotes `// summary`-style comments above
public/internal members to `/// <summary>` doc comments; RCS1124 inlines
locals used exactly once. Generated `Tools/` files were reverted to keep
the source-generator outputs deterministic.

5. **`docs(agent): correct --diagnostics flag form and verify Roslynator
4.x code fixers`**
Updates `.github/agents/opc-ua-codestyle-enforcer.agent.md` with two
corrections discovered while running this sweep:
- `dotnet format --diagnostics "RULE1 RULE2"` (single quoted,
space-separated) silently fixes nothing on `dotnet format` 10.x — it
parses as one unknown ID and exits 0. The correct form is `--diagnostics
RULE1 --diagnostics RULE2 …`, repeated per rule.
- Roslynator 4.15.0 **does** ship code fixers for
RCS1078/1118/1124/1140/1142/1181/1221/1260 (the previous "no auto-fixer"
list was outdated). RCS1140/1142 default to `Hidden` so need `--severity
info` to surface and fire; RCS1078 requires
`roslynator_empty_string_style = field` (already set in
`.editorconfig`). Only NUnit4002/2046/2010 genuinely lack auto-fixers in
this stack.

6. **`style: format new code from master merge (dotnet format sweep)`**
After merging master into the branch (commit `e3ebc8742`), re-ran the
same three-phase format sweep on the 151 .cs files brought in by the
merge, scoped per project via repeated `--include`. 81 files changed;
same rule families as commits 1–4.
- Hand-fixed `Tests/Opc.Ua.MigrationAnalyzer.Tests/AnalyzerHarness.cs`
for `RCS1174` (redundant `async`/`await` removal) because the auto-fixer
didn't apply for that specific case.
   - Reverted files that the formatter would break:
- `Libraries/Opc.Ua.Server/Server/ServerInternalData.cs` — multi-TFM
analysis disagreement injected `<<<<<<<` / `=======` / `>>>>>>>`
markers.
-
`Tools/Opc.Ua.MigrationAnalyzer.Core/{Core/Stack/Client/ChannelBase.cs,
Core/Stack/Types/IUserIdentityTokenHandler.cs,
Client/Session/Session.cs}` — IDE0005 removed `using
Opc.Ua.Security.Certificates` that IS used (the files are 1.5.378
reference fixtures for the analyzer).
- `Tests/Opc.Ua.Gds.Tests/PushTest.cs` — IDE0002 over-simplified
`OpcUa.ObjectTypeIds` to `ObjectTypeIds`, which then resolves to a
conflicting type.

### What this PR does **not** touch

- No behavioral or semantic logic changes.
- No `.editorconfig` rule promotions (rules stay at their current
severities; this is a backlog-burndown pass, not a rule tightening).
- Generated source under `Tools/` is intentionally excluded.
- Higher-risk rule families (`RCS1090`/`CA2007` `ConfigureAwait`,
`RCS1249` null-forgiving cleanup, `IDE0270`/`IDE0059`) are left for
follow-up sweeps because they need per-TFM build verification or
semantic review.

## Related Issues

_No tracking issue — this is a non-functional style sweep and the
agent-doc correction is a follow-up to the same work. Happy to open one
if the maintainers prefer._

## 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. _(N/A — style-only, no
behavioral change)_
- [x] I have added all necessary documentation. _(agent doc updated with
the two corrections)_
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings. _(verified clean on `net10.0` and `net48`; only
pre-existing `NETSDK1005`/`NETSDK1207` failures on Tools/AOT projects)_
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. _(build
verified on net10 and net48 per batch; full test suite not yet run
locally — relying on 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>
 2026-06-10 08:35:44 +02:00
Marc Schier e7d9a153b9 [Test] Unit tests for ManagedSession revalidation loop (follow-up to #3849) (#3864) 
# Description

Follow-up unit tests for the debounced server-certificate revalidation
loop added in PR #3849 (now merged as `15eb55c46`). Codecov flagged
`ManagedSession.CertificateChanges.cs` as the lowest-coverage file in
that PR (34% patch coverage) — the loop body that drives the debounce /
at-most-one-in-flight / re-arm semantics is the heart of the trust/CRL
revalidation behaviour and was previously untested.

## Changes

*
**`Libraries/Opc.Ua.Client/Session/ManagedSession.CertificateChanges.cs`**
— extract `RunRevalidationLoopAsync` into a pure `internal static`
overload that takes a `ChannelReader<int>`, `TimeProvider`, debounce
window, validate callback, logger, and cancellation token. The instance
overload now forwards to it. This gives unit tests a seam without
standing up a connected `ManagedSession` (whose private constructor +
`CreateAsync` triggers a real connect).

*
**`Tests/Opc.Ua.Client.Tests/Session/ManagedSessionRevalidationLoopTests.cs`
(new)** — 5 unit tests covering the loop body in isolation:
- `BurstOfSignalsCollapsesToSingleValidationAsync` — 50 signals →
exactly 1 validation (bounded-1 channel + DropWrite).
- `SignalDuringInflightValidationReArmsExactlyOnceAsync` — re-arm
semantics during in-flight validation.
- `NoSignalMeansNoValidationAsync` — idle loop never invokes the
validator.
- `CancellationDuringDebounceAbortsPendingValidationAsync` —
cancellation cleanly aborts a pending validation.
- `ValidationExceptionsAreSwallowedAndLoopContinuesAsync` — transient
validation failures don't break the loop.

Uses real `TimeProvider.System` with a 50 ms debounce + 250 ms settle
window so each test runs in well under a second. A `FakeTimeProvider`
seam was considered but rejected — the loop's `ReadAsync → Delay →
drain` sequence is racy to drive with a fake clock because the timer is
not created until after the first signal has been consumed.

## Related Issues

- Follow-up to #3562 / #3160 (both fixed in #3849, merged).

## 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. _(None needed — pure
test addition + non-public testing seam.)_
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran focused tests locally using the **UA.slnx** projects against
.net **10**, and all 5 new tests passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
 2026-06-10 08:35:08 +02:00
Marc Schier 15eb55c46e Cert rotation: force renegotiate (#3562) + auto-detect on client (#3160) (#3849) 
# Description

Implements the OPC UA Part 12 §7.10.9 contract on the server (fixes
#3562) AND the client-side counterpart for immediate detection of
own-cert / trust-list / CRL changes (fixes #3160).

## Server side (#3562)

Per Part 12 §7.10.9 the server _shall_ "force existing SecureChannels
affected by the changes to renegotiate" once the `ApplyChanges` method
response has been delivered. Previously the server reloaded the
certificate but did not cut affected channels; clients kept using the
old cert until natural rekey, and the code carried an explicit `TODO`
next to a 1-second `Task.Delay` hack in
`ConfigurationNodeManager.ApplyChanges`.

Key server changes:

* **New optional capability interface**
`Opc.Ua.ITransportListenerCertificateRotation` (single method
`CloseChannelsForCertificate(Certificate)`) — implemented by
`TcpTransportListener` and `HttpsTransportListener`. Custom transport
listeners can opt in without breaking `ITransportListener`.
* **TCP path**: new internal
`TcpListenerChannel.TryCloseForCertificateRotation(oldThumbprint, out
globalChannelId)` matches the channel's negotiated `ServerCertificate`
by thumbprint, sends `BadCertificateInvalid`, and calls
`ChannelFaulted()` to actually close the socket.
`TcpTransportListener.CloseChannelsForCertificate` snapshots
`m_channels` outside `m_lock` to avoid lock inversion, invokes the
per-channel helper, returns cut global channel ids. **The listener
socket stays bound** — clients can reconnect immediately.
* **HTTPS path**: HTTPS owns the cert at the Kestrel/HTTP.sys listener
level, so a `Stop()`+`Start()` cycle is the only practical
force-renegotiate. Moved out of `CertificateUpdate` (which now does only
the soft fan-out) into `CloseChannelsForCertificate` so the staging
event no longer disrupts in-flight requests prematurely.
* **`ConfigurationNodeManager.ApplyChanges`** rewritten:
- Captures the per-group pre-transaction certificate (preserved across
multi-stage transactions per Part 12 §7.10.2) and the staged
`UpdateCertificateData`.
- Schedules a deferred apply: waits for the response to flush, re-syncs
the registry via `CertificateManager.UpdateAsync`, then calls
`CloseChannelsForCertificate` on every listener that implements the
capability.
- **Configurable grace period**: new
`IConfigurationNodeManager.ApplyChangesGracePeriod` (default 250 ms,
negative clamped to zero). Hosts running over higher-latency
administrative links can raise it.
- Exposes
`IConfigurationNodeManager.DrainPendingApplyChangesAsync(CancellationToken)`
for tests/hosts to deterministically await rotation.
  - Removes the explicit 1-second `Task.Delay` TODO hack.
* **`Opc.Ua.Server.ITransportListenerRegistryProvider`** — narrow opt-in
interface on `ServerInternalData` exposing the live transport listener
list. Lets `ConfigurationNodeManager` drive the fan-out without
expanding `IServerInternal` (binary compat for mocked test fixtures).
* **Sessions are NOT closed**. Per spec, the cut is on the SecureChannel
only; existing `Session.IsSecureChannelValid` + `Session.Activate`
rebind handles transfer over the new channel.

## Client side (#3160)

The client SDK previously had no automatic detection of changes to its
own certificate, trust list, or CRL — users had to call
`Session.ReloadInstanceCertificateAsync` manually and trigger a
reconnect. The spec-mandated `ApplicationCertificateUpdated`,
`TrustListUpdated`, and `CrlUpdated` enum values existed but only the
first was being emitted by `CertificateManager`.

Key client/shared changes:

* **`CertificateManager` now emits `TrustListUpdated` and `CrlUpdated`**
from `WriteTrustListAsync` (per mask) and from
`TrustListTransaction.CommitAsync` (after the atomic apply). The new
internal `NotifyTrustListChanged` also invalidates cached
peer/user/HTTPS validation cores so subsequent validations pick up the
fresh trust state.
* **`ManagedSession.AutoReconnectOnCertificateChange`** opt-in property
(default `false` for backwards compat). When `true`, `ManagedSession`
subscribes to `CertificateManager.CertificateChanges` on construction
and, on the three relevant kinds:
- Calls `Session.ReloadInstanceCertificateAsync` on
`ApplicationCertificateUpdated` so the next ActivateSession is signed
with the rotated client cert.
- Triggers a reconnect via the state machine so the new state is
honoured within milliseconds rather than at the next
`SecurityTokenLifetime` rekey.
* **`ManagedSession.ApplicationCertificateChanged`** event surfaces
every observed change regardless of the auto-reconnect policy for
diagnostics or custom rotation logic.
* Subscription is disposed by the existing `Dispose` / `DisposeAsync`
paths.

```csharp
using var managed = await ManagedSession.CreateAsync(
    configuration, endpoint, sessionFactory);
managed.AutoReconnectOnCertificateChange = true;

managed.ApplicationCertificateChanged += (sender, e) =>
{
    Console.WriteLine($"observed {e.Kind} on {e.TrustList}");
};
```

## Tests

| Suite | net10.0 | net48 |
|---|---|---|
| `TransportListenerCertificateRotationTests` (new, 6 tests) | ✅ | ✅ |
| `CertificateManagerTests` (4 new event tests) | ✅ 22/22 | ✅ 4/4 |
| `PushCertManagement` | 50/53 ✅ | 50/53 ✅ |
| `GDS PushTest` (focused subset) | 15/15 ✅ | 16/16 ✅ |
| `Opc.Ua.Server.Tests` (focused subset) | 38/38 ✅ | 38/38 ✅ |
| `GDS PushTest` (full) | 313/338 ✅ | — |

Skips are pre-existing (cert-type mismatches, optional methods) — no
regressions.

New integration test
`PushTest.ApplyChangesForcesChannelRenegotiateAsync` positively asserts
the stale channel fails with a recognized channel-invalidation status
after `ApplyChanges`, and that a fresh reconnect succeeds with the
rotated cert.

## Documentation

- `Docs/CertificateManager.md` — new sections:
- "Server-Side Certificate Rotation via Push (OPC UA Part 12 §7.10.9)" —
`ITransportListenerCertificateRotation` contract + custom listener
example.
- "Client-Side Auto-Detection of Certificate Changes (issue #3160)" —
`AutoReconnectOnCertificateChange` + `ApplicationCertificateChanged`
event + the three covered scenarios.

## Related Issues

- Fixes #3562
- Fixes #3160

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran focused tests locally using the **UA.slnx** projects against
.net **framework 4.8** and .net **10**, and all passed. _(Full UA.slnx
test run delegated to CI.)_
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
 2026-06-08 12:19:49 +02:00