diff --git a/src/Opc.Ua.Server/Server/RequestManager.cs b/src/Opc.Ua.Server/Server/RequestManager.cs index 297a506fe5..ce8a74b843 100644 --- a/src/Opc.Ua.Server/Server/RequestManager.cs +++ b/src/Opc.Ua.Server/Server/RequestManager.cs @@ -171,7 +171,7 @@ namespace Opc.Ua.Server /// /// Called when the client wishes to cancel one or more requests. /// - public void CancelRequests(uint requestHandle, out uint cancelCount) + public void CancelRequests(NodeId sessionId, uint requestHandle, out uint cancelCount) { var cancelledRequests = new List(); @@ -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); diff --git a/src/Opc.Ua.Server/Server/StandardServer.cs b/src/Opc.Ua.Server/Server/StandardServer.cs index e63b7e32a9..89ad0396f0 100644 --- a/src/Opc.Ua.Server/Server/StandardServer.cs +++ b/src/Opc.Ua.Server/Server/StandardServer.cs @@ -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 { diff --git a/tests/Opc.Ua.Server.Tests/RequestManagerTests.cs b/tests/Opc.Ua.Server.Tests/RequestManagerTests.cs index dc6196a575..a5979720a6 100644 --- a/tests/Opc.Ua.Server.Tests/RequestManagerTests.cs +++ b/tests/Opc.Ua.Server.Tests/RequestManagerTests.cs @@ -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(); + cancellingSession.Setup(s => s.Id).Returns(new NodeId(1)); + + var otherSession = new Mock(); + 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); diff --git a/tests/Opc.Ua.Server.Tests/SessionPublishQueueTests.cs b/tests/Opc.Ua.Server.Tests/SessionPublishQueueTests.cs index dcfe960f77..1b03b8fee1 100644 --- a/tests/Opc.Ua.Server.Tests/SessionPublishQueueTests.cs +++ b/tests/Opc.Ua.Server.Tests/SessionPublishQueueTests.cs @@ -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(); + 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(() => task);