Files
UA-.NETStandard/tests/Opc.Ua.Core.Tests/Stack/Server/RequestLifetimeTests.cs
romanett d98eb6b28c Fix server session defects found by a spec-checked review of CreateSession, ActivateSession, session lifecycle and continuation points (#4632)
# Description

Fixes defects found by a spec-checked review of the server session
implementation: CreateSession, ActivateSession, the session lifecycle
(per-request validation, CloseSession, Cancel, timeout, shutdown) and
session continuation points. Each finding was checked against OPC
10000-4 (§5.7, §7.9, §7.32), OPC 10000-5 (session diagnostics and audit
events) and OPC 10000-6 §6.8.2.

There is one commit per finding, and each commit message cites the spec
section. A max-effort code review of the fixes then found 15 more
issues, mostly regressions the first round introduced; they are fixed in
the `revfix` merges.

## What changed

**CreateSession**
- At MaxSessionCount the oldest session that was never activated is
closed to make room (§5.7.2.1). Sessions that are activated, already
closing, or have an activation in flight are never evicted. The victim
is claimed atomically, so a late activation commit on it is rejected.
- `requestedSessionTimeout` NaN is revised. A malformed `endpointUrl` no
longer fails the call with Bad_UnexpectedError. `serverUri` is ignored,
as §5.7.2.2 requires.
- Unnamed sessions get distinct names.
- Session diagnostics now report `MaxResponseMessageSize` and
`ServerUri`, and `SessionSecurityDiagnostics.Encoding` uses the
specified names ("UA Binary", "XML", "JSON").
- AuditCreateSessionEvent carries SecureChannelId and the request
certificate, also on failure.
- Every rejected CreateSession is counted, and only once. A rejection
during server startup no longer turns into Bad_UnexpectedError.
- If an `OnApplicationCertificateError` override accepts an error, the
certificate is now kept and the remaining checks still run. A
certificate accepted this way does not grant TrustedApplication or
application-based roles.

**ActivateSession**
- EphemeralKey handling follows OPC 10000-6 §6.8.2:
- A key used to decrypt a UserName or IssuedToken secret is retired and
replaced in the same commit.
  - A key that was not used is kept, and nothing is returned.
- ECDHPolicyUri is honoured; an unsupported policy gets
Bad_SecurityPolicyRejected.
  - Request parameters are no longer echoed back.
- After the activation commits, a failure or cancellation no longer
faults the response, which would hide the new server nonce. Post-commit
callbacks run uncancellably.
- An activation that overlaps a session close or timeout fails with
Bad_SessionClosed instead of returning Good.
- The PolicyId of an AnonymousIdentityToken is validated.
- Status codes now follow §5.7.3.3:
- Bad_ApplicationSignatureInvalid for a mismatched client-signature
algorithm
- Bad_UserSignatureInvalid for a mismatched X509 user-token signature
algorithm
- Bad_IdentityTokenInvalid for an undecodable token or an unknown policy
- `ClientUserIdOfSession` keeps the user of the first activation.

**Lifecycle and requests**
- Requests on an expired or closing session are rejected with
Bad_SessionClosed (§5.7.2.1). A CloseSession on an expired session that
is not yet closing is still processed as the client's close and honours
`deleteSubscriptions`.
- Closing a session aborts its outstanding requests with
Bad_SessionClosed. A request that registers after that abort pass is
rejected too.
- Cancel completes the matching requests with
Bad_RequestCancelledByClient (§5.7.5.1). It also reaches requests still
waiting in the request queue, but only when both timestamps prove the
request was sent earlier.
- There is one AuditCancelEvent per Cancel call, carrying the client's
audit entry id and user.
- Every close path goes through a single close claim, so
SessionAbortCount, SessionTimeoutCount and the "Session/Terminated" and
"Session/Timeout" audits are each counted once.
- Teardown that started cannot be cancelled. Requests rejected by
session validation are counted.

**Continuation points**
- HistoryRead returns Bad_NoContinuationPoints instead of evicting
points of the same request (§7.9). Only the request that saved a point
pins it, so points from prior requests can still be freed.
- Points saved by a HistoryRead that faults are released.
- Session-less Browse and BrowseNext no longer dereference a null
session.

## Behaviour changes reviewers should know about
- At the session cap, CreateSession evicts a non-activated session
instead of returning Bad_TooManySessions. The eviction is counted in
SessionAbortCount.
- Clients see Bad_SessionClosed for requests on expired or closing
sessions, and Bad_RequestCancelledByClient for cancelled requests.
- `ActivateSessionProcessAdditionalParameters` is now called on every
successful activation, with or without an AdditionalHeader.
- If an `OnApplicationCertificateError` override returns instead of
throwing, CreateSession now accepts the certificate.
- Timeout and termination audits are now raised after the session
teardown.
- The only public API changes are additive overloads in `AuditEvents`
and `RequestManager`.

**Known partial fixes:**
- The session's `maxResponseMessageSize` is reported but not enforced;
only the channel limit applies.
- With overlapping ActivateSession calls on one session, an earlier
response can still carry a key that a later activation has already
replaced. Closing that fully needs a new `ISession` member.

## Related Issues

- The Redundancy.Server session-mirroring findings from the same review
are tracked separately in #4627.

## Tests
- Regression tests cover each finding. The new files are:
  - `CreateSessionSpecRegressionTests`
  - `ActivateSessionValidationRegressionTests`
  - `StandardServerActivateSessionRegressionTests`
  - `SessionLifecycleRegressionTests`
  - `SessionRequestAdmissionRegressionTests`
  - `HistoryReadContinuationDispatchTests`
  
  Existing test classes were extended too.
- Local results on net10.0:
  - `Opc.Ua.Server.Tests`: 7883 passed / 0 failed / 5 skipped
- `Opc.Ua.Sessions.Tests`: 790 passed / 0 failed / 379 skipped (same as
master)
- net48 Release build of `Opc.Ua.Server.Tests`: 0 errors / 0 warnings.

## 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.
- [ ] 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.5 <noreply@anthropic.com>
2026-10-03 05:42:01 +02:00

169 lines
6.4 KiB
C#

/* ========================================================================
* Copyright (c) 2005-2025 The OPC Foundation, Inc. All rights reserved.
*
* OPC Foundation MIT License 1.00
*
* Permission is hereby granted, free of charge, to any person
* obtaining a copy of this software and associated documentation
* files (the "Software"), to deal in the Software without
* restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following
* conditions:
*
* The above copyright notice and this permission notice shall be
* included in all copies or substantial portions of the Software.
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
* OTHER DEALINGS IN THE SOFTWARE.
*
* The complete license agreement can be found here:
* http://opcfoundation.org/License/MIT/1.00/
* ======================================================================*/
// CA2000: test code; many disposables are ownership-transferred to test fixtures or short-lived,
// making CA2000 noisy without a real leak risk. Disabled file-level for the suite.
#pragma warning disable CA2000
using System.Threading;
using NUnit.Framework;
namespace Opc.Ua.Core.Tests.Stack.Server
{
[TestFixture]
[Category("Server")]
[SetCulture("en-us")]
[SetUICulture("en-us")]
[Parallelizable]
public class RequestLifetimeTests
{
[Test]
public void Constructor_Default_CreatesInstance()
{
using var lifetime = new RequestLifetime();
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.False);
Assert.That(lifetime.StatusCode, Is.EqualTo(StatusCodes.Good));
}
[Test]
public void Constructor_WithExternalToken_CreatesLinkedTokenSource()
{
using var cts = new CancellationTokenSource();
using var lifetime = new RequestLifetime(cts.Token);
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.False);
cts.Cancel();
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.True);
Assert.That(lifetime.StatusCode, Is.EqualTo(StatusCodes.Good));
}
[Test]
public void TryCancel_WhenNotCancelled_ReturnsTrue_SetsStatusCode_CancelsToken()
{
using var lifetime = new RequestLifetime();
bool result = lifetime.TryCancel(StatusCodes.BadTimeout);
Assert.That(result, Is.True);
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.True);
Assert.That(lifetime.StatusCode, Is.EqualTo(StatusCodes.BadTimeout));
}
[Test]
public void TryCancel_WhenAlreadyCancelled_ReturnsFalse_LeavesStatusCode()
{
using var lifetime = new RequestLifetime();
bool firstResult = lifetime.TryCancel(StatusCodes.BadTimeout);
bool secondResult = lifetime.TryCancel(StatusCodes.BadUnexpectedError);
Assert.That(firstResult, Is.True);
Assert.That(secondResult, Is.False);
Assert.That(lifetime.StatusCode, Is.EqualTo(StatusCodes.BadTimeout));
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.True);
}
[Test]
public void TryCancelConcurrentCallersFirstStatusWins()
{
// a client Cancel, a Session close and the request timeout race to cancel the same
// request; exactly one of them may claim it, and its status must be the one reported.
for (int iteration = 0; iteration < 2000; iteration++)
{
using var lifetime = new RequestLifetime();
using var barrier = new Barrier(2);
bool timeoutWon = false;
bool cancelWon = false;
var timeout = new Thread(() =>
{
barrier.SignalAndWait();
timeoutWon = lifetime.TryCancel(StatusCodes.BadTimeout);
});
var cancel = new Thread(() =>
{
barrier.SignalAndWait();
cancelWon = lifetime.TryCancel(StatusCodes.BadRequestCancelledByClient);
});
timeout.Start();
cancel.Start();
timeout.Join();
cancel.Join();
Assert.That(timeoutWon ^ cancelWon, Is.True, $"Iteration {iteration}: exactly one caller must win.");
Assert.That(
lifetime.StatusCode,
Is.EqualTo(timeoutWon ? StatusCodes.BadTimeout : StatusCodes.BadRequestCancelledByClient),
$"Iteration {iteration}: the winner's status must be reported.");
}
}
[Test]
public void TryCancel_AfterDispose_ReturnsFalse()
{
var lifetime = new RequestLifetime();
lifetime.Dispose();
bool result = lifetime.TryCancel(StatusCodes.BadTimeout);
Assert.That(result, Is.False);
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.False);
}
[Test]
public void MarkCompleted_DisposesToken_CannotCancel()
{
var lifetime = new RequestLifetime();
lifetime.MarkCompleted();
bool result = lifetime.TryCancel(StatusCodes.BadTimeout);
Assert.That(result, Is.False);
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.False);
}
[Test]
public void None_Property_ReturnsCompletedInstance()
{
RequestLifetime lifetime = RequestLifetime.None;
Assert.That(lifetime.StatusCode, Is.EqualTo(StatusCodes.Good));
bool result = lifetime.TryCancel(StatusCodes.BadTimeout);
Assert.That(result, Is.False);
Assert.That(lifetime.CancellationToken.IsCancellationRequested, Is.False);
}
}
}