fix: scope Cancel request matching to the current session (#4023)

### Summary

This PR scopes `Cancel` request matching to the caller's session and
adds regression tests that verify a request handle can no longer cancel
work that belongs to another session.

### Problem

According to OPC UA Part 4, `Cancel` applies to a request handle from
the same session that issued the original request. The server should not
use a client-supplied `requestHandle` to match outstanding requests
across all sessions.

Previously, `StandardServer.CancelAsync()` validated the caller's
session but then forwarded only the raw `requestHandle` to
`RequestManager.CancelRequests()`. `RequestManager` iterated the global
outstanding-request table and canceled every request whose
`ClientHandle` matched, without checking the owning session. Since
request handles are only session-scoped and different sessions can
legitimately reuse the same value, one session could cancel another
session's in-flight request.

### Changes

- Pass the validated caller session id from
`StandardServer.CancelAsync()` into `RequestManager.CancelRequests()`.
- Restrict `RequestManager.CancelRequests()` to requests whose
`SessionId` and `ClientHandle` both match the `Cancel` request.
- Update existing request-manager and publish-queue tests to pass the
session id and add a regression test that proves matching request
handles in different sessions no longer interfere with each other.
This commit is contained in:
MrAlaskan
2026-07-19 12:23:52 +08:00
committed by GitHub
parent 2d55126cb8
commit 70c74e7cb8
4 changed files with 55 additions and 8 deletions
+3 -2
View File
@@ -171,7 +171,7 @@ namespace Opc.Ua.Server
/// <summary>
/// Called when the client wishes to cancel one or more requests.
/// </summary>
public void CancelRequests(uint requestHandle, out uint cancelCount)
public void CancelRequests(NodeId sessionId, uint requestHandle, out uint cancelCount)
{
var cancelledRequests = new List<uint>();
@@ -180,7 +180,8 @@ namespace Opc.Ua.Server
{
foreach (OperationContext request in m_requests.Values)
{
if (request.ClientHandle == requestHandle)
if (request.SessionId == sessionId &&
request.ClientHandle == requestHandle)
{
request.RequestLifetime.TryCancel(StatusCodes.BadRequestCancelledByRequest);
cancelledRequests.Add(request.RequestId);
+4 -1
View File
@@ -1101,7 +1101,10 @@ namespace Opc.Ua.Server
CancelResponse response;
try
{
ServerInternal.RequestManager.CancelRequests(requestHandle, out uint cancelCount);
ServerInternal.RequestManager.CancelRequests(
context.SessionId,
requestHandle,
out uint cancelCount);
response = new CancelResponse
{
@@ -99,7 +99,7 @@ namespace Opc.Ua.Server.Tests
};
// Act
m_requestManager.CancelRequests(42, out uint cancelCount);
m_requestManager.CancelRequests(context.SessionId, 42, out uint cancelCount);
// Assert
Assert.That(cancelCount, Is.EqualTo(1));
@@ -123,7 +123,7 @@ namespace Opc.Ua.Server.Tests
uint cancelCount = 0;
Assert.DoesNotThrow(
() => m_requestManager.CancelRequests(requestHandle, out cancelCount));
() => m_requestManager.CancelRequests(context.SessionId, requestHandle, out cancelCount));
Assert.That(cancelCount, Is.EqualTo(1));
Assert.That(
@@ -131,6 +131,45 @@ namespace Opc.Ua.Server.Tests
Is.EqualTo(StatusCodes.BadRequestCancelledByRequest));
}
[Test]
public void CancelRequestsDoesNotCancelMatchingHandleFromDifferentSession()
{
var cancellingSession = new Mock<ISession>();
cancellingSession.Setup(s => s.Id).Returns(new NodeId(1));
var otherSession = new Mock<ISession>();
otherSession.Setup(s => s.Id).Returns(new NodeId(2));
const uint requestHandle = 42;
using var ownRequestLifetime = new RequestLifetime();
using var otherRequestLifetime = new RequestLifetime();
var ownContext = new OperationContext(
new RequestHeader { RequestHandle = requestHandle },
null,
RequestType.Read,
ownRequestLifetime,
cancellingSession.Object);
var otherContext = new OperationContext(
new RequestHeader { RequestHandle = requestHandle },
null,
RequestType.Read,
otherRequestLifetime,
otherSession.Object);
m_requestManager.RequestReceived(ownContext);
m_requestManager.RequestReceived(otherContext);
m_requestManager.CancelRequests(cancellingSession.Object.Id, requestHandle, out uint cancelCount);
Assert.That(cancelCount, Is.EqualTo(1));
Assert.That(ownRequestLifetime.CancellationToken.IsCancellationRequested, Is.True);
Assert.That(otherRequestLifetime.CancellationToken.IsCancellationRequested, Is.False);
Assert.That(
otherContext.OperationStatus.Code,
Is.EqualTo(StatusCodes.Good));
}
[Test]
public void RequestCompletedRemovesRequestAndCompletesLifetime()
{
@@ -154,7 +193,7 @@ namespace Opc.Ua.Server.Tests
// Assert
// To ensure it is removed, cancelling it will yield 0 count
m_requestManager.CancelRequests(42, out uint cancelCount);
m_requestManager.CancelRequests(context.SessionId, 42, out uint cancelCount);
Assert.That(cancelCount, Is.Zero);
// Assert that lifetime is completed (disposed), which means TryCancel returns false
Assert.That(requestLifetime.TryCancel(StatusCodes.BadTimeout), Is.False);
@@ -161,13 +161,17 @@ namespace Opc.Ua.Server.Tests
subMock.Setup(s => s.Id).Returns(1);
queue.Add(subMock.Object); // added but not ready, so the request must park
var sessionMock = new Mock<ISession>();
sessionMock.Setup(s => s.Id).Returns(new NodeId(1));
const uint requestHandle = 77;
using var requestLifetime = new RequestLifetime();
var context = new OperationContext(
new RequestHeader { RequestHandle = requestHandle },
null,
RequestType.Publish,
requestLifetime);
requestLifetime,
sessionMock.Object);
requestManager.RequestReceived(context);
var sink = new TestParkSink();
@@ -181,7 +185,7 @@ namespace Opc.Ua.Server.Tests
"A parked request releases its processing worker at the park point.");
// A Cancel service call carrying the Publish request handle.
requestManager.CancelRequests(requestHandle, out uint cancelCount);
requestManager.CancelRequests(context.SessionId, requestHandle, out uint cancelCount);
Assert.That(cancelCount, Is.EqualTo(1), "The Cancel service should match the parked Publish request.");
Assert.CatchAsync<OperationCanceledException>(() => task);