mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-28 22:55:49 +08:00
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:
@@ -171,7 +171,7 @@ namespace Opc.Ua.Server
|
|||||||
/// <summary>
|
/// <summary>
|
||||||
/// Called when the client wishes to cancel one or more requests.
|
/// Called when the client wishes to cancel one or more requests.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public void CancelRequests(uint requestHandle, out uint cancelCount)
|
public void CancelRequests(NodeId sessionId, uint requestHandle, out uint cancelCount)
|
||||||
{
|
{
|
||||||
var cancelledRequests = new List<uint>();
|
var cancelledRequests = new List<uint>();
|
||||||
|
|
||||||
@@ -180,7 +180,8 @@ namespace Opc.Ua.Server
|
|||||||
{
|
{
|
||||||
foreach (OperationContext request in m_requests.Values)
|
foreach (OperationContext request in m_requests.Values)
|
||||||
{
|
{
|
||||||
if (request.ClientHandle == requestHandle)
|
if (request.SessionId == sessionId &&
|
||||||
|
request.ClientHandle == requestHandle)
|
||||||
{
|
{
|
||||||
request.RequestLifetime.TryCancel(StatusCodes.BadRequestCancelledByRequest);
|
request.RequestLifetime.TryCancel(StatusCodes.BadRequestCancelledByRequest);
|
||||||
cancelledRequests.Add(request.RequestId);
|
cancelledRequests.Add(request.RequestId);
|
||||||
|
|||||||
@@ -1101,7 +1101,10 @@ namespace Opc.Ua.Server
|
|||||||
CancelResponse response;
|
CancelResponse response;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
ServerInternal.RequestManager.CancelRequests(requestHandle, out uint cancelCount);
|
ServerInternal.RequestManager.CancelRequests(
|
||||||
|
context.SessionId,
|
||||||
|
requestHandle,
|
||||||
|
out uint cancelCount);
|
||||||
|
|
||||||
response = new CancelResponse
|
response = new CancelResponse
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -99,7 +99,7 @@ namespace Opc.Ua.Server.Tests
|
|||||||
};
|
};
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
m_requestManager.CancelRequests(42, out uint cancelCount);
|
m_requestManager.CancelRequests(context.SessionId, 42, out uint cancelCount);
|
||||||
|
|
||||||
// Assert
|
// Assert
|
||||||
Assert.That(cancelCount, Is.EqualTo(1));
|
Assert.That(cancelCount, Is.EqualTo(1));
|
||||||
@@ -123,7 +123,7 @@ namespace Opc.Ua.Server.Tests
|
|||||||
|
|
||||||
uint cancelCount = 0;
|
uint cancelCount = 0;
|
||||||
Assert.DoesNotThrow(
|
Assert.DoesNotThrow(
|
||||||
() => m_requestManager.CancelRequests(requestHandle, out cancelCount));
|
() => m_requestManager.CancelRequests(context.SessionId, requestHandle, out cancelCount));
|
||||||
|
|
||||||
Assert.That(cancelCount, Is.EqualTo(1));
|
Assert.That(cancelCount, Is.EqualTo(1));
|
||||||
Assert.That(
|
Assert.That(
|
||||||
@@ -131,6 +131,45 @@ namespace Opc.Ua.Server.Tests
|
|||||||
Is.EqualTo(StatusCodes.BadRequestCancelledByRequest));
|
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]
|
[Test]
|
||||||
public void RequestCompletedRemovesRequestAndCompletesLifetime()
|
public void RequestCompletedRemovesRequestAndCompletesLifetime()
|
||||||
{
|
{
|
||||||
@@ -154,7 +193,7 @@ namespace Opc.Ua.Server.Tests
|
|||||||
|
|
||||||
// Assert
|
// Assert
|
||||||
// To ensure it is removed, cancelling it will yield 0 count
|
// 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(cancelCount, Is.Zero);
|
||||||
// Assert that lifetime is completed (disposed), which means TryCancel returns false
|
// Assert that lifetime is completed (disposed), which means TryCancel returns false
|
||||||
Assert.That(requestLifetime.TryCancel(StatusCodes.BadTimeout), Is.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);
|
subMock.Setup(s => s.Id).Returns(1);
|
||||||
queue.Add(subMock.Object); // added but not ready, so the request must park
|
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;
|
const uint requestHandle = 77;
|
||||||
using var requestLifetime = new RequestLifetime();
|
using var requestLifetime = new RequestLifetime();
|
||||||
var context = new OperationContext(
|
var context = new OperationContext(
|
||||||
new RequestHeader { RequestHandle = requestHandle },
|
new RequestHeader { RequestHandle = requestHandle },
|
||||||
null,
|
null,
|
||||||
RequestType.Publish,
|
RequestType.Publish,
|
||||||
requestLifetime);
|
requestLifetime,
|
||||||
|
sessionMock.Object);
|
||||||
requestManager.RequestReceived(context);
|
requestManager.RequestReceived(context);
|
||||||
|
|
||||||
var sink = new TestParkSink();
|
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 parked request releases its processing worker at the park point.");
|
||||||
|
|
||||||
// A Cancel service call carrying the Publish request handle.
|
// 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.That(cancelCount, Is.EqualTo(1), "The Cancel service should match the parked Publish request.");
|
||||||
Assert.CatchAsync<OperationCanceledException>(() => task);
|
Assert.CatchAsync<OperationCanceledException>(() => task);
|
||||||
|
|||||||
Reference in New Issue
Block a user