mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-24 02:58:36 +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>
|
||||
/// 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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user