GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-18 22:55:08 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,814 Commits 105 Branches 105 Tags
master
Commit Graph

5 Commits

Author SHA1 Message Date
romanett 9b034f8291 Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) 
# Description

Nullable reference types are now enabled once, in `common.props`, and
every project in the repository builds warning-clean with them.

**Configuration**
- `<Nullable>enable</Nullable>` lives in `common.props`. All per-project
`<Nullable>` settings are removed: 178 project files (enable,
annotations, and the `disable` in the three Aot test projects).
- 419 per-file `#nullable enable`/`restore` directives and the 4
`#nullable disable` directives are removed.
- Kept on purpose:
- The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type
Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`,
`PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the
project setting in generated code, so the directive is required there.
- `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against
the 1.5.378 packages and does not import the repository props.
- `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now
describe the central setting.

**Migration** (the 43 unset and 13 annotations-only projects, roughly
10k diagnostics)
- **Production code: annotated at the declarations.** Covers
`tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the
LDS sample.
- Members, parameters and returns are nullable where null actually
flows.
- Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`,
`IsMethodTypeDesign`).
- `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take
`params object?[]`. `Template.AddReplacement` takes `string?` (rendering
already skips null replacements).
- `IRegisteredServerStore.RegisterAsync` returns
`Task<RegistrationEntry?>`: it returns null when a registration is
removed.
- Dereferences backed by an invariant (for example, the validated model)
use `!`, so runtime behaviour is unchanged.
- **Behavioural changes in production code** (small, deliberate):
- `NodeSetToModelDesign` throws `InvalidOperationException` when the
file is not a NodeSet. Before, it threw a NullReferenceException on the
next line.
- `ModelDesignValidator` starts its lookup tables empty instead of null.
A redundant `if (m_nodes == null) return;` guard is gone.
- **Test code: minimal annotations.**
  - `?` on values that can be null, `!` where the test asserts non-null.
  - `= null!` for fixture members assigned in SetUp.
- `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the
`IEncodeable` constraint.
- A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`)
folded into the adjacent `IsNull`/`Count` checks.

**Notes for reviewers**
- The diff is large (~1,500 files) but almost entirely mechanical: `?`,
`!` and directive removal. The non-annotation changes are listed above.
- Known follow-up, not done here:
`IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay
non-nullable even though the validator returns null before it loads a
dictionary. Making them nullable would ripple through every generator
call site.

## Related Issues

- Fixes #4439

## 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.
- [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.

The "no new warnings" box covers net10.0 and net48: full non-incremental
builds of `UA.slnx`, `tools/SourceGeneration.slnx`,
`tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0
warnings and 0 errors under the repository's warnings-as-errors
settings. net9.0 and net8.0 also had no nullable diagnostics, but they
were not rebuilt after the final fixes. Tests have not been run locally
yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 08:48:13 +02:00
romanett a0d569d756 Fix Server and GDS audit findings, review findings and spec-conformance gaps (#4571) 
## Summary

Fixes the findings of a bug audit of `Opc.Ua.Server`,
`Opc.Ua.Gds.Server` and `Opc.Ua.Gds.Client`. Rebased on master after
#4516.

What went in:
- **Audit:** 93 findings, each checked against the OPC UA specification.
- **Code review of those fixes:** 35 further findings.
- **Spec grounding of every fix commit:** 18 conformance gaps, now
closed.

Commits are one per finding, named `Fix <ID>`, `Fix review <ID>` or `Fix
spec gap <name>`. The IDs refer to the audit report (S = Server areas,
G1 = GDS server, G2 = GDS client).

Validation:
- Each finding was traced in code and checked against OPC 10000 (parts
3, 4, 5, 9, 11, 12, 13, 18, 20).
- Every fix commit was checked against the spec text again afterwards: 0
non-conformant, remaining gaps fixed.
- Regression tests were added for most fixes.
- Suites run on net10.0 before the rebase: Server, GDS, History,
Configuration, Features, Core.Security, DI, Redundancy, LDS,
InformationModel, PubSub.Server, Client, Sessions and the three
Subscriptions suites, all green. The same suites on the rebased branch
are still running; results will be posted as a comment.

## Behaviour and user-facing changes

### Secure defaults
- **`Historize()` / `HistorianBuilder`** grant HistoryRead only. History
writes need an explicit `historyAccessLevel: HistoryRead |
HistoryWrite`.
- **File-system mount:**
- Anonymous sessions can no longer create, delete, move or open files
for writing (`Bad_UserAccessDenied`).
  - UserWritable and UserExecutable now reflect the user.
- `AllowAnonymousWrite` restores the old behaviour; the ReferenceServer
sample turns it on.
- **Default PKI root** for the hosted server, hosted GDS, hosted LDS and
`OpcUaApplicationConfigurationProvider`:
- Moved from the shared temp directory to `{LocalApplicationData}/OPC
Foundation/{ApplicationName}/pki`, and startup fails if no per-user
directory exists.
- When certificate stores of an earlier version still exist under temp,
a warning names both paths. Existing deployments that relied on the
default get a new application certificate (and the hosted GDS a new CA)
unless `PkiRoot` is set to the old path.
- **GDS:**
  - `GdsServerOptions.AutoApprove` defaults to `false`.
- The default KeyCredential store grants no roles unless
`AuthorizeRoles` allows them.
- Certificate requests (StartSigningRequest, StartNewKeyPairRequest,
FinishRequest) need an encrypted channel.
- **Hosted server anonymous logins:**
- Anonymous is rejected (`Bad_IdentityTokenRejected`) when the default
authenticator options disable it.
- A custom `AddIdentityAuthenticator<T>()` on its own no longer disables
anonymous.
- With anonymous disabled and no configured token policies, the endpoint
advertises the authenticators' token types instead of Anonymous.
- **Runtime-added roles:** their properties and methods need
SecurityAdmin over an encrypted channel, like the well-known roles.
- **Disabled users:** users disabled through Part 18 UserManagement can
no longer log in through `RegisterDefaultAuthenticators(IUserDatabase)`.

### GDS server (Part 12)
- **Certificate requests:**
- FinishRequest / ReadRequest with another application's RequestId
returns `Bad_InvalidArgument`.
  - Every Start*Request returns a new RequestId.
- A CSR without the ApplicationUri is rejected
(`Bad_CertificateUriInvalid`).
- An abstract or unsupported certificate type is rejected at Start
(`Bad_InvalidArgument`).
- **Private-key password:** no longer persisted in the JSON applications
database. A password saved by an earlier version is read into memory
once and dropped on the next save.
- **CheckRevocationStatus:** validates offline against the GDS's own
trust lists and CRLs. It returns Good for good GDS-issued certificates
and `Bad_CertificateInvalid` for input that is not a certificate.
- **Certificate-group trust lists:**
- Writes need CertificateAuthorityAdmin; SecurityAdmin and application
admins are read-only.
  - Writes are validated with the Part 4 certificate validation.
- **AuthorizationService:**
- The legacy RequestAccessToken applies `AccessControl` and issues the
token for the session identity.
- StartRequestToken passes the session identity to every identity-aware
provider.
  - Unknown ResourceIds return `Bad_NotFound`.
- Expired token requests and refresh tokens are pruned, with at most 10
000 pending requests (`Bad_TooManyOperations`).
- **KeyCredential:** requests with PublicKey + SecurityPolicyUri are
rejected with `Bad_SecurityPolicyRejected` (secret encryption is not
implemented); a PublicKey without SecurityPolicyUri is rejected with
`Bad_InvalidArgument`.
- **Audit events:** access-token audit events no longer contain the
password or token data.
- **RegisterTickets / UnregisterTickets:** need the RegistrarAdmin role
when called from a session.
- **Managed application nodes:** moved from namespace 0 to their own
namespace (`ManagedApplicationsNamespaceUri`); the NodeId is the
ApplicationUri, and the mandatory child properties exist.
- **Hosted GDS:**
- New `GdsServerOptions.CertificateGroups`
(`GdsCertificateGroupOptions`), bindable from configuration.
  - The mandatory DefaultApplicationGroup is always present.
  - The default CA subject uses the resolved application name.
- **JSON applications database:**
- A corrupt file throws instead of being silently replaced by an empty
database.
- Saves are atomic and write in place on bind mounts; on net8+ builds
they also follow symlinks and keep the Unix file mode.
  - Issued certificates and trust-list links survive a reload.

### GDS client
- **Trust-list download:**
  - Reads until an empty chunk (Part 20 short reads).
- A size-limit rejection of the final end-of-file read, as sent by
released servers, counts as end of file.
- **Default certificate group:** a null group means
DefaultApplicationGroup, and vendor groups in other namespaces are no
longer mistaken for well-known groups.
- **Status codes and errors:**
  - Missing directory proxies report `Bad_ServerNotConnected`.
- Short or wrongly typed method outputs throw `BadUnexpectedError` /
`BadTypeMismatch`.
- **GdsAccessTokenProvider:** no longer caches a failed client task, and
one caller's cancellation no longer fails other callers.

### Trust lists, push and ConfigurationFile (Part 12 / Part 20)
- **AddCertificate:**
  - `IsTrustedCertificate=false` returns `Bad_CertificateInvalid`.
- Added certificates are validated (Part 4), and a certificate revoked
by a current CRL returns `Bad_CertificateRevoked`.
- **CloseAndUpdate:** validates the uploaded list against its own
certificates and CRLs, not the stores it replaces. Validation failures
are no longer recorded in the server's Rejected store.
- **RemoveCertificate:** removing a trusted or issuer CA that other
certificates still need returns `Bad_CertificateChainIncomplete`.
- **Handles:**
  - Several read handles may be open at once.
- Any open while a write handle exists returns `Bad_NotReadable` /
`Bad_NotWritable`.
  - A write open while any handle is open returns `Bad_NotWritable`.
  - Handles of closed sessions are released.
  - Idle handles close after ActivityTimeout (default 60 s).
- **Status codes:**
- Read clamps to the remaining bytes and no longer enforces the size
limit.
  - Oversized Write / AddCertificate returns `Bad_RequestTooLarge`.
  - Unsupported open modes return `Bad_NotSupported`.
- **ConfigurationFile.CloseAndUpdate:**
  - Empty Targets return `Bad_InvalidArgument`.
- The ConfigurationUpdatedAuditEvent is raised with the configuration
owner as source.
- With the new `IApplicationConfigurationFileTargetProvider`, providers
receive the Targets, a bad target returns Uncertain with per-target
results, and session-interrupting updates are applied after
RestartDelayTime.
- **Default namespace permissions:** namespaces registered after the
first permission lookup now get their NamespaceMetadata default
RolePermissions and AccessRestrictions (no stale negative cache).

### Sessions, identity and roles (Part 18)
- **Session timeouts:** a failure closing one timed-out session no
longer stops the session-timeout monitor.
- **ChangePassword:** an empty old password returns
`Bad_IdentityTokenInvalid`.
- **Removing or disabling a user:** closes only that user's
UserName-token sessions.
- **Role configuration changes:** roles granted by an `ImpersonateUser`
mapping are kept.
- **X509Subject rules:** a certificate subject value containing a quote
never matches an X509Subject rule.
- **`ImpersonateUser` callback:** no longer runs under the global
session lock.
- **Session diagnostics:** LocaleIds are kept as the client sent them,
while null LocaleIds are ignored for translation.

### Subscriptions and monitored items (Part 4)
- **Acknowledgements:** a failing acknowledgement returns a per-entry
Bad status instead of failing the whole Publish.
- **`DiscardedMessageCount`:** counts messages dropped from the
retransmission queue and unsent overflow messages.
- **Durable subscriptions:** transferring a restored anonymous
subscription works (no NullReferenceException).
- **Transferred subscriptions:** sampling and permission checks use the
item's current owner session.
- **Access denied:** a monitored item whose user may no longer read the
attribute gets a `Bad_UserAccessDenied` notification instead of going
silent. Non-Value attributes need Browse permission (RolePermissions
needs ReadRolePermissions), and User* attributes are evaluated per
subscriber.
- **ModifyMonitoredItems:** a SamplingInterval of -1 uses the
subscription's publishing interval.
- **AggregateFilter priming:** needs HistoryRead / ReadHistory
permission and is bounded (100 000 values, bounded page size).
- **Resource leak:** event monitored items no longer leak a consumer
task per create/delete cycle.

### Node management services (Part 3 / Part 4)
- **Reference types:** AddNodes, AddReferences, DeleteReferences and
Browse reject ids that are not ReferenceTypes with
`Bad_ReferenceTypeIdInvalid`. AddNodes, AddReferences and
DeleteReferences also reject abstract ReferenceTypes; Browse still
accepts them (e.g. `HierarchicalReferences` with includeSubtypes).
- **Concurrent AddNodes:** duplicate BrowseNames under one parent get
`Bad_BrowseNameDuplicated`.
- **TranslateBrowsePathsToNodeIds:**
- Requires Browse permission on intermediate nodes and final targets
(RolePermissions and AccessRestrictions).
  - A long RelativePath no longer rejects the whole request.
- **SemanticChangeEvent:**
- Raised once per actual change of a semantic property, whether or not
it is monitored.
  - Also raised for properties with the SemanticChange access-level bit.
  - Reports the owning node's TypeDefinition as AffectedType.
- **Localized diagnostics:** operation-level text is translated when
OperationLocalizedText is requested.

### History and aggregates (Part 11 / Part 13)
- **DeleteRawModified:** an unspecified or reversed range returns
`Bad_InvalidTimestampArgument`, and StartTime == EndTime deletes the
value at StartTime for every provider.
- **ReadAtTime fallback:** reads only the values around each requested
time. A time whose bound is not found within the search limit returns
`Bad_BoundNotSupported` instead of failing the request.
- **ReadProcessed:** a ProcessingInterval shorter than one tick returns
`Bad_AggregateInvalidInputs`.
- **Delta / DeltaBounds:** widen to the next larger signed type when the
result does not fit the source type (e.g. a decreasing unsigned counter,
or an Int16 overflow).
- **Range / Range2:** return the source data type.

### Alarms and fluent API (Part 9)
- **Suppression:** an alarm stays suppressed while any suppression group
is active, and re-evaluation without a state change does not move
TransitionTime.
- **`MonitorVariable(variable)`:** sets SourceNode, SourceName and
InputNode to the variable and makes it a ConditionSource (HasCondition
to the alarm, HasEventSource from the object).
- **Fluent input/output arguments:** InputArguments / OutputArguments
added through the fluent API to a registered method are readable.
- **Root notifiers:** a failed root-notifier registration no longer
leaves an orphaned event worker.
- **State-machine handlers:** exceptions are logged at Error.

### File system (Part 20)
- **Delete / MoveOrCopy:**
  - Only act on direct children of the directory.
- A NodeId whose kind does not match the entry returns `Bad_NotFound` (a
file NodeId can no longer delete a directory).
- Copying a directory into its own subtree returns
`Bad_InvalidArgument`.
- **Locking:**
- An open file, or a directory containing one, cannot be deleted or
moved (`Bad_InvalidState`).
  - The check is atomic with opens.
- While a Delete or Move runs, read opens get `Bad_NotReadable` and
write opens `Bad_InvalidState`.
- **Open:**
- Reserved mode bits or EraseExisting without Write return
`Bad_InvalidArgument`.
  - Read+Write returns `Bad_NotSupported`.
  - Read while a writer is open returns `Bad_NotReadable`.
- **Client `FileSystemErrors`:** maps `Bad_NotReadable` to
`IOException`.
- **RuntimeNodeSet:** well-formed NodeSets with a stray `Model` element,
or two models in one file where one requires the other, now load.

### Server core, hosting and lifecycle
- **`SessionEstablishmentQueueLimit` > 0:**
CreateSession/ActivateSession now queue for a permit. The permit is
acquired before the request is registered, and a waiting handshake
releases its worker through the request-parking policy.
- **Node manager lifecycle:**
  - Add, reload and remove no longer wait for parked Publish requests.
- A Migrate reload racing with CreateMonitoredItems no longer blocks
later lifecycle operations.
  - Late items on retired or removed managers get `Bad_NodeIdUnknown`.
- **HistoryUpdate:** a batch mixing data and event details is bounded by
both limits.
- **Server stop/start:** stopping the server disposes the configuration
watcher and certificate subscription; starting recreates them.
- **Monitored-item queue factory:** a factory supplied by the caller or
the DI container is no longer disposed by the server.
- **Audit events:** each certificate validation error raises its own
AuditCertificate event. A failed OpenSecureChannel without a service
result is audited as failed.

## New public API (additive)
- `IQueuedSessionEstablishmentLimiter`
- `StandardServer.OwnsMonitoredItemQueueFactory` (protected virtual)
- `DefaultPkiRoot.Get` (Opc.Ua.Configuration)
- `TrustList.SetCertificateValidation`
- `NamespaceMetadataRegistry.Invalidate`
- `IApplicationConfigurationFileTargetProvider` and
`ApplicationConfigurationUpdatePlan`
- `GdsServerOptions.CertificateGroups` / `GdsCertificateGroupOptions`
- `InMemoryKeyCredentialRequestStore.AuthorizeRoles`
-
`DefaultManagedApplicationsNodeManager.ManagedApplicationsNamespaceUri`
- `AuthorizationServiceManager.RequestAccessTokenAsync(...,
IUserIdentity?, ...)`
- `AllowAnonymousWrite` on the file-system node manager and its factory
- `IFileSystemHost.TryBeginMutation` / `EndMutation`, replacing
`HasOpenHandles`

## Not fixed / known limitations
- **KeyCredential:** secret encryption with the client's PublicKey
(requests needing it are rejected).
- **CSR SAN restriction:** restricting subject / SAN values copied from
a CSR (CA policy).
- **Ticket signatures:** ticket signature verification (needs a
trusted-signer configuration).
- **FileType Read+Write open:** needs a read/write stream contract in
`IFileSystemProvider`.
- **Custom `IUserDatabase` implementations:** the disabled-user check
still scans all users per login. The built-in stores do a single lookup.
- **64-bit Delta precision:** limited by the calculator's double
arithmetic.

## Tests with changed expectations
Existing tests were updated where they asserted the old behaviour:
- File-system status codes
- Trust-list handle eviction
- `DiscardedMessageCount`
- Localized diagnostics bits
- Delta on unsigned sources
- SourceNode of fluent alarms
- ReferenceServer transfer permission test (now expects
`Bad_UserAccessDenied` notifications)
- `UserDeactivationShutdownReviewTests` (the revoked session is a
UserName login)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-09-29 07:26:05 +02:00
Marc Schier fa8d45f2cf Fix server/core validation, lifecycle and protocol regressions (#4477) 
# Description

**Server/core audit fixes with regression tests. Latest review-fix
validation, merge validation and earlier acceptance results are
distinguished below.**

This addresses a 95-finding server/core report. Every finding was
reviewed with
the OPC UA specification knowledge base or, where the standard does not
prescribe a C# implementation, the corresponding ownership/configuration
contract. Incorrect or withdrawn subclaims remain qualified rather than
being
presented as additional fixes.

The original checkpoint covers session identity/certificate validation
and
revocation; certificate/key ownership and transactional updates; TCP
reconnect,
receive/admission and buffer lifetime; HTTP quotas; subscription
admission,
durability and pooled payloads; file-operation lifecycle; historian
aggregates;
fluent/state-machine behavior; and related utilities. It includes
targeted
regressions and directly related documentation.

The approved acceptance cleanup is separate in the commit history:

| Cleanup | Verified result |
| --- | --- |
| Invalid identity-policy and KeyCredential fixture inputs | Fixtures
explicitly advertise Username and use its generated policy ID.
Credential persistence uses valid clear mode inside the existing
encrypted admin context. Original permission, lockout and negative
encryption assertions remain. |
| Native GDS lifetime and concurrent connection failures | Retain
ApplicationInstance until client teardown. A deterministic native
reproducer proved the test-only channel cap of 10 was exhausted;
removing that override restores the existing bounded default of 1000. No
global serialization or production quota change. |
| Stress certificate ownership and close accounting | Balance two helper
references and the fake transport's server-certificate ownership.
Replace the invalid event-count bound with exactly-once physical
disposal, opens = closes, zero active entries and one logical entry
lifetime. |
| Participant deadlines | Use the injected TimeProvider with the
existing cross-framework WaitAsync overload. Tests pin one tick before,
exact expiry, terminal notification, recovery and timely completion. |
| Reproducibility and platform assertions | Preserve canonical WoT
bytes/hashes, explicit display-culture contracts and exact
numeric-vector assertions. Accept Apple's CryptographicException subtype
without weakening disposal balance. |
| Missing user-signature proof | Reject null/empty proofs before OS
crypto. Return BadUserSignatureInvalid per Part 4, 5.7.3.3. None-policy
and valid-signature controls remain. A CI-only test nullable-context
error was corrected without changing production behavior. |
| Durable sample restore and shared test storage | Dequeue waits for
actual batch residency instead of treating failed restore completion as
success, and restored files close before deletion. Five deterministic
regressions failed before the fix. The two fixtures that clear the
shared batch directory are isolated; all original 3,000-item assertions
remain unchanged. |

## Latest review follow-up

Commit `828856850219518970e8e2f580b8f94b3c1fc748` fixes the deferred
robotics
disposal reported in the latest review.
`RobotIntentNodeManager.DisposeAsync`
now joins a shared controller-host drain and then explicitly awaits base
node-manager disposal. A controller's bounded shutdown wait may defer
its
cleanup, but no longer lets the node-manager override finish early.
Concurrent
callers, including an asynchronous call after synchronous `Dispose`,
cannot
bypass the drain after the host list has been detached.

Synchronous disposal remains nonblocking for deferred work. Deferred
initialization failures are logged and retained, and base cleanup
failures
propagate to asynchronous callers. The existing controller-level bounded
disposal contract is unchanged; the node-manager contract is documented
in
`docs/Robotics.md`.

| Regression requirement | Evidence |
| --- | --- |
| Wait for deferred hosts; preserve synchronous compatibility and
concurrent joins | `NodeManagerAsyncDisposalWaitsForDeferredHostsAsync`:
four cases covering both entry-point orders and zero/positive host
shutdown waits. All four failed before the fix. |
| Wait through base cleanup and observe failures |
`NodeManagerDisposalWaitsForBaseCleanupAndReportsItsFailureAsync`: four
cases hold the actual base-resource semaphore, verify pending callers
after the host finishes, then check success or the same propagated
exception and exactly-once cleanup. |

The focused `IntentHostingCoverageTests` and
`IntentControllerHostTests.Dispose`
selection passed **30/30 on net10.0 and 30/30 on net48**, with no
failures or
skips. The final net10 solution build passed with zero errors and only
the four
previously documented CA2249 warnings in unchanged fuzz-test code. No
new
warnings, analyzer suppressions, dependencies, or weakened assertions
were added.
The review thread received a commit-specific reply and was explicitly
resolved.

This verification is scoped to the robotics feedback; the earlier full
Subscriptions-run limitation remains recorded in the merge section
below.


## Documentation review follow-up (2026-09-14)

Commit `f9a9686fcc5d3cfd19cbbc8c2d826e20f5c7e72f` addresses the six
follow-up
threads on documentation and implementation rationale. Each has an
in-thread
reply citing the commit and an explicit resolution.

Multiline XML documentation now covers the declarations introduced or
modified
throughout the entire PR, including internal contracts, samples, tests,
and
public members of related private helpers. A syntax-aware inventory
checked
**2,031 declarations across all 256 PR C# paths with zero documentation
gaps**.
A supplementary check also covers **75 previously unchanged public
members**
of private helpers involved in the changed code. Token comparison across
the
conditional-compilation branches proves this follow-up changes **no
executable
C# or test assertions**. Project-file changes are rationale comments
only.

The implementation questions are documented at their source:

| Question | Resolution |
| --- | --- |
| Certificate assembly friendship | Server Push configuration reuses the
internal explicit-subject CSR helper with the existing key; no public
crypto API was added. |
| Core.Types assembly friendship | Server builders use internal
transition callbacks/revision guards; Server.Tests checks the revision
deterministically without creating a public compatibility contract. |
| Always-single-reader notifications | One dequeuer preserves event
order; optional concurrency fans out within one event and joins before
the next event. |
| Asynchronous disposal completion | `DisposeAsync` already joins the
completion published after admitted operations and all cleanup attempts.
The compatibility `Dispose` remains nonblocking to avoid deadlocking an
admitted callback against its own drain. |

The requested detailed review disposition follows. It distinguishes
actual
fixes from confirmed policies and incorrect subclaims rather than
counting every
comment as a new runtime change.


### Detailed disposition of review 5191795284

The [SDK
review](https://github.com/OPCFoundation/UA-.NETStandard/pull/4477#pullrequestreview-5191795284)
was submitted against `1c4a9df6c`, before the subsequent merge and
feedback fixes.
Its **15 inline threads** have each received an action-specific reply
and an
explicit resolution. A fresh GraphQL query verified those replies and
resolution
states; the fixes in `b8a59990f` remain ancestors of the current branch.

| Review concern | Action and disposition | Concrete regression evidence
|
| --- | --- | --- |
| Injected certificate-store providers bypassed | **Fixed.**
Registration, configuration snapshots, and validation use the injected
providers. Explicit custom types remain authoritative; the built-in
Directory catch-all is deferred so native X509 paths and later custom
providers are not captured. Failed store opens dispose the created
store. | `InjectedProviderResolvesRegisteredAndConfiguredTrustStores`
covers default-provider ordering and both registration paths; provider
validation/open-failure cases and the unchanged
`TrustedCertificateInjectedViaIndependentX509StoreIsObservedAsync` cover
real use and independent-store visibility. |
| Unguarded timer-thread leadership callbacks | **Fixed.** Each
subscriber's exception is logged independently and cannot suppress
subsequent subscribers or fail lease operations. `IsLeader` reads the
deadline without invoking application callbacks. |
`ThrowingLeadershipSubscriberDoesNotFailAcquireOrDisposeAsync`,
`ExpiryNotifiesOtherSubscribersWhenOneThrowsAsync`, and
`IsLeaderDoesNotDispatchExpiryNotificationsAsync` were all demonstrated
failing before the fix. |
| Legacy pending-key provider accepts an unfinishable regenerated CSR |
**Fixed by explicit capability validation.** A provider implementing
only `IPendingCertificateKeyStore` receives `BadNotSupported` before key
generation or persistence when regeneration is requested. Existing-key
CSR/update remains supported. Regeneration requires matching-only
consumption; no unsafe destructive probing was introduced. |
`LegacyPendingProviderRejectsRegenerationBeforeCreatingOrSavingAKeyAsync`
checks no key or store mutation;
`LegacyPendingProviderCompletesExistingKeySigningAndUpdateAsync` proves
the supported flow completes. |
| Shutdown waits behind user deactivation | **Fixed.** Accepted
deactivations drain before the master's lifecycle gate is taken.
Configuration retains its binding until the shared drain completes,
including concurrent shutdown callers and direct master disposal. | Four
cases of `ShutdownDrainsDeactivationOutsideTheSessionClosingGateAsync`
exercise real configuration/binding/ServerInternalData/master reentry,
retained address space, exact session/subscription closure, and no
production cancellation shortcut. |
| Subclass cleanup runs under the operation-lifetime lock | **Fixed.**
Cleanup starts outside that lock. A pre-published completion joins all
disposers, and only the active asynchronous teardown context may use
guarded helpers. Captured contexts lose that privilege when the callback
exits. |
`SynchronousOwnedCleanupLetsSamplingWorkerReachClosedAdmissionAsync`,
`DisposeCoreCanUseGuardedMembersWithoutReopeningCallerAdmissionAsync`,
captured-context rejection, and exact cleanup-failure/ownership
assertions cover both synchronous and suspended cleanup. |
| Discovery requestId-zero chunk read after pool return | **Fixed by the
earlier merge.** Discovery validation precedes `SaveIntermediateChunk`;
no read occurs after that path can return the buffer. The ordering is
retained. | `DiscoveryChannelReadsTheFirstChunkBeforeItIsReturnedAsync`
uses a poisoning pool and the requestId-zero path. |
| T1.8 OPN cleanup misses post-decryption throws | **Fixed by the
earlier merge and pinned with new regressions.** Both sequence rejection
and certificate mismatch release the decrypted body and input ownership.
| `OpenSecureChannelRejectionAfterDecryptionReturnsEveryRentalAsync`
checks the exact audit status and pool conservation. Omitting either
corresponding release in an isolated snapshot made its case fail; no
mutation remains. |
| T1.2 mount-root protection depends on the provider | **Fixed at the
host boundary.** Separator-only aliases resolve to the protected root.
Traversal, empty/whitespace segments, backslashes, and drive/stream
qualifiers are rejected before arbitrary providers are invoked. Valid
nested paths and root destinations remain supported. | A deliberately
permissive provider is used by
`NonCanonicalObjectPathsCannotDeleteMoveOrCopyAsync` and invalid-target
cases; exact zero-call checks are paired with legitimate
nested-path/root-destination controls. |
| Inline trusted/issuer entries now face CRL checks | **Intentional
strict behavior, documented and tested.** A configured backing store
with strict unknown-status rejection requires current revocation
information. Explicit-only lists without a store retain their no-store
behavior. Actual revocation cannot be suppressed; a matching store
entry's policy takes precedence. |
`InlineIssuerWithStoreHonorsStrictUnknownRevocationPolicyAsync`,
`InlineIssuerRevocationIsNeverSuppressedAsync`, store-precedence cases,
and explicit-only controls preserve the missing-information versus
actual-revocation distinction in Part 4 6.1.3. |
| RSA minimum applied to unspecified ECC identifiers | **Fixed at the
application-certificate check.** The RSA minimum is compared only with
an actual RSA public key. An unspecified ECC identity is not replaced
simply because its key size is below an RSA threshold. |
`SilentCertificateCheckPreservesUnspecifiedEccIdentityAsync` checks DER,
thumbprint, store contents, and private-key retention; explicit RSA and
HTTPS minimum controls remain. |
| Failed directory refresh wedges corrective mutations | **Fixed.**
Recoverable automatic pre-refresh failures are logged and leave
reconciliation pending, but do not block corrective provider mutations.
Explicit refresh errors, capacity admission, caller cancellation, and
actual provider denials remain enforced. |
`CorrectiveDeleteRecoversAfterPersistentOverLimitRefreshFailureAsync`
converges an externally over-limit directory back to its exact allowed
contents. Capacity, cancellation/token, and six refresh-error controls
check negative paths. |
| Timed-transition rejection logs at 10 Hz | **Fixed.** Repeated
`(state, revision, status)` warnings are deduplicated without changing
the retry cadence. A different status or state entry permits another
warning. | `RepeatedTimedRejectionLogsOnceAndRecoversAsync` checks one
warning over eleven rejections and success on the next attempt.
Status-change and same/different-state reentry controls pin reset
behavior. |
| Bad unsubscribe strands retired-generation bookkeeping | **Fixed.**
Retired all-event bookkeeping completes even when unsubscribe reports
Bad; the first Bad is still returned to the caller. | Good and
BadCommunicationError cases of
`RetiredAllEventsDeletionCompletesBookkeepingAndReleasesGenerationAsync`
check event/snapshot removal, generation release, and exactly one
unsubscribe without finalization replay. |
| CSR subject parsing and fallback | **Fixed for existing and
regenerated keys.** Omitted subjects fall back to the actual stored
certificate. Regeneration resolves the public certificate's subject
before creating a key. Malformed or unresolvable subjects return
`BadInvalidArgument` before pending-key mutation. | Store-only fallback,
real-generator null/empty cases, and missing-subject/malformed-DN
controls verify the signed CSR, matching fresh pending key, unchanged
active identity, and no destructive side effects. |
| Committed RemoveUser change reported as failed | **Fixed for
RemoveUser and ModifyUser.** Accepted cleanup is still awaited and close
failures remain logged, but cannot replace the committed mutation
result. Precommit failures retain their original result and side
effects. |
`CommittedUserChangeKeepsResultAndPublishedUsersWhenCloseFailsAsync`
checks published Users, Good result, logging, and continued closes.
`FailedUserChangeKeepsPublishedUsersAndDoesNotCloseSessionsAsync` checks
BadNotFound with no publication or close. |
| Alleged pooled decoded-request aliasing | **No runtime redesign
needed; reviewer rebuttal confirmed.** The decoder copies the relevant
input data. Misleading ownership comments were corrected rather than
introducing a new dispatch model. |
`DecodedRequestRetainsValuesAfterInputBuffersAreReturnedAsync` checks
the exact decoded NodeId and ByteString after returned arrays are
poisoned. |

The current documentation-only follow-up rechecked **104 Core, 473
Server, and
4 Configuration cases** covering these concerns: **581 passed, zero
failed or
skipped**. The final net10 solution build also passed with **zero
warnings and
errors**. The complete earlier Core/Server/Configuration and net48
results remain
listed separately rather than being presented as rerun current-head
acceptance.

## Earlier review follow-up

Commit `b8a59990fe2e8e967a12ffd5d9158beed7831143` addresses all 19
review
threads. Each thread has an in-thread reply naming this commit and was
explicitly
resolved after verification; the final paginated query found 19 resolved
and
zero unresolved threads.

The follow-up fixes injected certificate-store selection (including
default
provider ordering and native X509 paths), actual-RSA minimum
enforcement, and
CSR capability/subject validation before key mutation. Node-manager
teardown
uses simpler scoped ownership and drains accepted user revocation
outside the
master gate; committed user results and retired-event cleanup remain
correct
when cleanup reports failure. File mutations can recover after failed
refresh
without bypassing capacity or cancellation, provider paths protect mount
roots,
and repeated timed rejections no longer flood logs. Leadership
subscriber
failures are isolated without callbacks from `IsLeader`.

Regressions also pin the already-merged OPN/discovery buffer fixes and
prove
decoded request values do not borrow returned input buffers. The strict
inline
issuer revocation policy is documented and tested, not weakened.
Existing tests
were changed only for the reviewed behavior: the incorrect
committed-user result
was corrected, and inline-issuer/root-alias controls were expanded.

### Review-fix validation

| Suite | Scope | Framework | Passed | Existing TRX NotExecuted |
| --- | --- | --- | ---: | ---: |
| Core | Complete project | net10.0 | 4,970 | 88 |
| Server | Complete project | net10.0 | 6,414 | 69 |
| Configuration | Complete project | net10.0 | 231 | 0 |
| Core | Certificate, lease and buffer compatibility slice | net48 | 153
| 0 |
| Server | Reviewed lifecycle, user, file, timer and CSR compatibility
slice | net48 | 692 | 0 |
| Configuration | Complete project | net48 | 231 | 0 |
| CSR provider regressions | Final portable-mock follow-up | net10.0 |
11 | 0 |

All runs exited successfully with normal concurrency and leak detection.
The Server console reports 5 skips; its TRX additionally records
existing
assumption/explicit cases, hence the 69-record count above. The
Framework build
caught two modern-only `ValueTask.CompletedTask` usages in new mock
setup; only
those expressions changed to the equivalent portable
`default(ValueTask)`.
The complete modern suites precede that test-only correction; all
production
files remained byte-identical, and the corrected fixture passed on both
targets.

The final net10 solution build succeeded with 0 errors and the same 4
CA2249
warnings in unchanged fuzz-test code. Diff checks and changed-line
whitespace
checks are clean; whole-file formatting still identifies 10 unchanged
baseline
lines, which were not reformatted outside the feedback scope. The
committed
35-file tree was hash-verified against the validation inputs.

This is the review-feedback verification scope, not a rerun or sign-off
of the
historical complete platform/native/soak acceptance campaign below.


## Latest upstream merge

Merge `9d9bf679f317164c5b2b90924cd3cdc27a86d451` integrates `master`
through `162fc5ebb` (#4486),
including the AddNodes child-name index, shared certificate-error
reporting,
CTT sample-data corrections, and the accompanying regressions and
documentation.

The CreateSession conflict preserves this branch's certificate
ownership,
required certificate, application-URI, and channel-certificate checks.
The
validation result now retains nested errors through `ThrowIfInvalid`,
allowing
the upstream shared reporting helper to mask trust, policy, and
revocation
failures consistently without losing audit detail.

The new indexed AddNodes regression initially attempted to reuse a
deterministic
NodeId after renaming its existing node. Renaming frees the BrowseName,
not that
NodeId. `IndexedBrowseNameReusePreservesExistingNodeIdentityAsync` now
proves
both contracts: the derived-ID collision leaves the original node
intact, then
a fresh explicitly requested ID can reuse the old name while the new
name
remains protected against duplicates. No production identity rule was
relaxed.
The incoming seeded random-index test uses the existing `UnsecureRandom`
wrapper
with the same seed and sequence; no new analyzer suppression was added.

| Local merge validation | Framework | Passed | Existing exclusions |
| --- | --- | ---: | ---: |
| Core, complete project | net10.0 | 5,081 | 88 |
| NodeState / BaseInstanceState | net10.0 | 579 | 0 |
| Server node management, certificate and session-security selection |
net10.0 | 112 | 0 |
| InformationModel node-management services | net10.0 | 29 | 1 |
| NodeState / BaseInstanceState | net48 | 578 | 0 |
| Core certificate validation, error reporting and TCP buffers | net48 |
111 | 0 |
| Server node management, certificate and session-security selection |
net48 | 112 | 0 |

All selected runs exited successfully at normal concurrency: **6,602
passing
test executions**. Modified test assemblies were explicitly rebuilt, and
the
combined name/identity regression was verified by name in both net10 and
net48
results. No skips or weakened assertions were introduced.

The final `dotnet build UA.slnx -c Release --no-restore -m:1` with
`CustomTestTarget=net10.0` succeeded with **zero warnings and zero
errors**.
The resolved source and adjusted tests pass whitespace verification, and
their
source hashes match the validation inputs.

This is merge-specific verification. Earlier review results and the
previously
reported incomplete full Subscriptions runs below remain scoped to their
own
commits; they were not reopened or relabeled as passing by this
operation.

## Previous upstream merge (2026-09-15 morning)

Merge `c4694e20ab089c555af233642042133ff3a03ac8` integrates `master`
through `484d6c887`, retaining
#4481's shared Like parser/GDS query fixes, #4488's RequestHandle
recovery in
ServiceFaults, and #4487's expired-session and durable-transfer
corrections.

Three conflict resolutions preserve both sides:

- Alias searches now use the shared `LikePattern` implementation,
including
upstream malformed-list validation. A timeout overload retains this
branch's
100 ms per-alias deadline and `BadTimeout` reporting without keeping a
second
wildcard translator. The shared matcher's original unlimited overload
remains
unchanged for existing GDS/ContentFilter callers. New regressions cover
bounded prefix/list scans, reuse after timeout, and the alias service
result.
- Expired activation closes outside the session-manager gate using
upstream's
once-only timeout claim, audit, and diagnostic accounting. This branch's
  original-channel and security-policy checks remain in place.
- TCP buffer tests retain this branch's ownership, poisoning-pool and
post-decryption checks alongside the upstream RequestHandle/fault
overload
  tests; neither set was discarded.

The upstream HTTPS binary behavior is intentionally retained: a decoder
`ServiceResultException` produces HTTP 200 with an OPC UA ServiceFault
carrying
the decoder status, response timestamp, and recoverable RequestHandle,
rather
than the previous HTTP 500 text response. Other exceptions retain their
existing HTTP error handling.

| Local validation | Framework | Passed | TRX NotExecuted |
| --- | --- | ---: | ---: |
| Core, complete project | net10.0 | 5,054 | 88 |
| Sessions, complete project | net10.0 | 780 | 610 |
| Server aliases, expiry/security and subscriptions | net10.0 | 288 | 0
|
| GDS database, paging and live directory calls | net10.0 | 263 | 0 |
| Shared Like patterns and bounded evaluation | net10.0 | 109 | 0 |
| Subscriptions transfer/durability/service selection | net10.0 | 23 | 1
|
| HTTPS WebApi, complete project | net10.0 | 346 | 0 |
| InformationModel aliases | net10.0 | 54 | 5 |
| Shared Like patterns and bounded evaluation | net48 | 109 | 0 |
| Core fault handles, TCP/HTTPS and filters | net48 | 268 | 0 |
| Server aliases, expiry/security and subscriptions | net48 | 288 | 0 |

These selected runs exited successfully with normal test concurrency:
**7,582 passing test executions**. Sessions' console reports 379 skips;
the
TRX also contains conditional/assumption exclusions, so the table uses
the
actual 610 NotExecuted records. No skips or weakened assertions were
added.
The new `EvaluationTimeoutBoundsWorkAndPreservesPatternReuse` and
`SharedPatternTimeoutRemainsAnAliasServiceError` tests passed on both
targets.

**Validation limitation:** two unfiltered Subscriptions runs did not
produce
a final result and were stopped, including a retry with a hang guard.
They
are not counted as passing. The isolated failover fixture passed 2/2,
and the
transfer/durability/service selection above passed. The stall's cause
was not
established; no unrelated code or test was changed to conceal it.

The final `dotnet build UA.slnx -c Release --no-restore -m:1` with
`CustomTestTarget=net10.0` succeeded with **0 warnings and 0 errors**.
Whitespace verification passed for all hand-reconciled files, and staged
source hashes match the validation inputs.

These are merge-specific checks, not a new full platform/native/soak
acceptance
sign-off. Earlier review and acceptance evidence below remains dated to
its
own commit.

## Previous upstream merge (2026-09-14)

Merge `c3e09553e4d1509502d4ba751960d58491bea2cf` integrates `master`
through `6313e38a0` (#4478),
including the CTT-driven Part 13 aggregate corrections and FindAlias
argument
validation. The upstream tests and CTT documentation are retained.

The single conflict in `AliasNameWildcardMatcher` is reconciled through
the
branch's shared regex compiler: upstream malformed-pattern rejection and
`IsValidPattern` coexist with OPC UA character escaping,
whole-string/newline
matching, the 100 ms timeout, and service-level error reporting. The
upstream
ReferenceTypeFilter checks and chronological aggregate behavior are
preserved.

Local merge verification:

| Suite / selection | Framework | Passed | Existing skips |
| --- | --- | ---: | ---: |
| Server aliases, aggregates and historian | net10.0 | 1,011 | 0 |
| Server aliases, aggregates and historian | net48 | 1,011 | 0 |
| InformationModel aliases | net10.0 | 54 | 5 |
| Features aliases | net10.0 | 11 | 0 |
| Client aliases | net10.0 | 28 | 0 |
| History aggregate/historical-access consumers | net10.0 | 265 | 0 |

All selected tests passed at normal concurrency. The final net10
solution
build passed with 0 errors and 0 warnings.
The resolved file's whitespace check and the staged diff check passed.
These
are merge-specific checks, not a new full platform/native/soak
acceptance claim.

## Previous upstream merges

Integrates `master` through `8e874d073`, including the Client, Types,
Core and
source-generator audit fixes in #4471, #4473, #4474 and #4475, plus the
event queue-size revision from #4480. The merge commits are `150ea0439`
(26 conflict resolutions) and `a0e17c07c` (the base update that arrived
during
validation).

The overlapping fixes are reconciled rather than replacing either
branch:

- **Rejected-certificate retention follows this branch, as explicitly
requested:**
zero means unlimited history; negative values disable new storage while
preserving existing backend trimming behavior (Directory clears prior
history;
shared key/value leaves it intact). The processor and manager policy are
not
  redefined, and documentation/tests describe the distinction.
- Preserve owned certificate snapshots and borrowed validation-core
lifetimes
  alongside upstream cache invalidation, store-first CRL checks and
  `BadCertificateRevocationUnknown` for unsupported revocation lookup.
- Combine exactly-once chunk ownership, receive-loop cancellation,
stale-epoch
rejection and detach-before-adoption with upstream
signature-before-padding
  verification and secret cleanup.
- Preserve pooled `HttpClient` per-request configuration together with
bounded
response reads and deadlines that remain active through body
consumption.
Regression cases exercise already-used clients and both timeout
precedence cases.
- Retain the binary-compatible channel-key constructor and normalize
omitted
profiles; preserve exact profile filtering and upstream binary-first
HTTPS/WSS
  endpoint ordering.
- Preserve monotonic lease expiry and stale-confirmation fencing while
adding
ordered, non-reentrant leadership notifications outside the state lock.

## Validation

Local validation of the conflict-resolution merge:

| Suite | Framework | Passed | Existing skips |
| --- | --- | ---: | ---: |
| Core | net10.0 | 4,940 | 88 |
| Client | net10.0 | 2,278 | 0 |
| Server | net10.0 | 6,088 | 5 |
| Types | net10.0 | 10,394 | 1 |
| SourceGeneration.Core | net10.0 | 3,940 | 8 |
| SourceGeneration | net10.0 | 185 | 0 |
| Core | net48 | 4,855 | 104 |

After the subsequent four-file #4480 update, the related Server slice
passed
811 tests with 5 existing skips, the real durable queue-size cases
passed 6/6,
and EventManager's 21 cases passed again after analyzer-only
expected-array
cleanup. The final `dotnet build UA.slnx -c Release` with
`CustomTestTarget=net10.0`
succeeded: 0 errors and 4 pre-existing CA2249 warnings in unchanged
fuzz-test
code. Whitespace verification passed for the resolved files and the
follow-up.

Merge-specific proof includes
`RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync` on
already-used
shared clients with either the request or HttpClient timeout shorter,
the
existing ownership/expiry regressions from both branches, and both
branches'
retention tests adjusted only to the explicitly chosen zero/negative
contract.

Earlier acceptance at `1c4a9df6c` passed hosted Linux/macOS CI, CodeQL,
image
builds and the canonical coverage gate (87.15% line, 77.27% branch,
92.14% patch).
Both Framework project matrices, all three Windows native suites (183
cases),
and the unfiltered net10 stress project (230 cases, including full
60-minute
chaos and 30-minute memory soaks) also passed. Those are **pre-merge
results**,
not a claim that the full framework/platform/native/soak matrix has been
rerun
on this merge. The previous acceptance workflow remains separate from
this
review-feedback operation.

Security checks, meaningful assertions, leak detection, default native
concurrency, full soak durations and coverage thresholds remain intact.
The user-set ready-for-review state is unchanged. No architectural or
complete
merged-head acceptance sign-off is claimed.

## Related Issues

- Fixes #4476

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.
 2026-09-16 04:42:09 +00:00
Marc Schier 91d3305b1c Scope FileType handles to Sessions (OPC 10000-5) (#4048) 
# Description

This focused draft extracts the FileType handle ownership fix from
#4021. It contains only the server FileSystem handle implementation,
focused tests, and FileSystem client documentation.

## Failure

`FileHandle` issued predictable per-file identifiers without recording
the owning Session. A different Session that obtained a handle could
read, write, seek, or close the stream, and open streams survived
Session closure.

## Fix

- Generate non-zero opaque handles and bind every open stream to the
Session that called `Open`.
- Require a valid Session for `FileType.Open`, `CreateFile` when it
requests an open handle, and all subsequent handle operations.
- Reject a handle presented by another Session.
- Close only the departing Session's handles from
`FileSystemNodeManager.SessionClosingAsync`.
- Document the Session ownership and lifetime.

OPC 10000-5 Section 6.2.5.2, Table 27 states that a `fileHandle` is
valid only for the Session used to call `Open` and that the Server shall
automatically close the file when that Session closes.

## Tests

- `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f
net10.0 --filter
"FullyQualifiedName~FileHandleTests|FullyQualifiedName~FileObjectStateTests|FullyQualifiedName~DirectoryObjectStateTests"
--no-restore`: 71 passed.
- `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f
net48 --filter
"FullyQualifiedName~FileHandleTests|FullyQualifiedName~FileObjectStateTests|FullyQualifiedName~DirectoryObjectStateTests"
--no-restore`: 71 passed.

Both builds emitted the same existing CA1873 warnings in unchanged
`Opc.Ua.Client` files; the changed files produced no warnings.

## Related Issues

- Focused split from #4021 (closed umbrella draft); no separate issue.

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-07-22 16:50:56 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00