From d2fa85c989ba3abcac482e9955ce138ec8570d84 Mon Sep 17 00:00:00 2001 From: Marc Schier Date: Mon, 3 Aug 2026 15:38:23 +0200 Subject: [PATCH] Fix BadContinuationPointInvalid caused by empty continuation point in Browser.BrowseAsync (#4168) Re-submission of #4167 against `master378`. # Description #4167 was merged directly into `release/1.5.378`. Maintenance fixes for the 1.5.378 lineage have to land on `master378` first, so the commit was removed from `release/1.5.378` again and the fix is submitted here instead. Servers 1.6.0 and higher return a **zero length** continuation point in the `Browse` response to indicate that no further references are available. `Browser.BrowseAsync` only checked the continuation point for `null`, so it issued a `BrowseNext` call with an empty continuation point, which the server rejects with `BadContinuationPointInvalid`. The fix follows the continuation point only if it actually contains data. This is consistent with the already existing `continuationPoint?.Length > 0` check in the `OperationCanceledException` filter of the same method. Credit for the original fix goes to @KarenKrill (#4167). ## Related Issues - Fixes #3698 - Supersedes #4167 ## 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran the affected tests locally. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ## Further comments New unit tests in `Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs` pin the behaviour for `null`, empty and non empty continuation points. Verified that the new tests fail without the one line change and pass with it: `dotnet test Tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj -f net10.0 --filter "FullyQualifiedName~BrowserUnitTests"` -> `Failed: 0, Passed: 3` The issue does not exist on `master` (2.0), where the `ByteString` refactoring already handles this. Co-authored-by: KarenKrill <74286712+KarenKrill@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 75db5878-10f9-4099-993a-5eb969fa080e --- Libraries/Opc.Ua.Client/Browser.cs | 2 +- Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs | 186 ++++++++++++++++++ 2 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs diff --git a/Libraries/Opc.Ua.Client/Browser.cs b/Libraries/Opc.Ua.Client/Browser.cs index ef7c746b62..6ff7674ada 100644 --- a/Libraries/Opc.Ua.Client/Browser.cs +++ b/Libraries/Opc.Ua.Client/Browser.cs @@ -303,7 +303,7 @@ namespace Opc.Ua.Client try { // process any continuation point. - while (continuationPoint != null) + while (continuationPoint?.Length > 0) { ReferenceDescriptionCollection additionalReferences; diff --git a/Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs b/Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs new file mode 100644 index 0000000000..6f32b5fc7b --- /dev/null +++ b/Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs @@ -0,0 +1,186 @@ +/* ======================================================================== + * 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/ + * ======================================================================*/ + +using System.Threading; +using System.Threading.Tasks; +using Moq; +using NUnit.Framework; +using Opc.Ua.Tests; + +namespace Opc.Ua.Client.Tests +{ + /// + /// Unit tests for the continuation point handling of the browser. + /// + [TestFixture] + [Category("Client")] + [Parallelizable] + public class BrowserUnitTests + { + /// + /// A server which signals "no more references" with an empty (zero length) + /// continuation point must not trigger a BrowseNext call, because such a + /// call is rejected by the server with BadContinuationPointInvalid. + /// + [Test] + [TestCase(new byte[0])] + [TestCase((byte[])null)] + public async Task BrowseAsyncDoesNotCallBrowseNextWithoutContinuationPointAsync( + byte[] continuationPoint) + { + var sessionMock = new Mock(MockBehavior.Strict); + SetupBrowse(sessionMock, CreateReferences("Browsed"), continuationPoint); + + var browser = new Browser(NUnitTelemetryContext.Create()) + { + Session = sessionMock.Object + }; + + ReferenceDescriptionCollection references = await browser + .BrowseAsync(ObjectIds.ObjectsFolder) + .ConfigureAwait(false); + + Assert.That(references, Has.Count.EqualTo(1)); + Assert.That(references[0].DisplayName.Text, Is.EqualTo("Browsed")); + + sessionMock.Verify( + x => x.BrowseNextAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny()), + Times.Never); + } + + /// + /// A non empty continuation point is followed until the server stops + /// returning one. + /// + [Test] + public async Task BrowseAsyncFollowsContinuationPointUntilExhaustedAsync() + { + var sessionMock = new Mock(MockBehavior.Strict); + SetupBrowse(sessionMock, CreateReferences("First"), [1, 2, 3]); + SetupBrowseNext(sessionMock, CreateReferences("Second"), []); + + var browser = new Browser(NUnitTelemetryContext.Create()) + { + Session = sessionMock.Object, + ContinueUntilDone = true + }; + + ReferenceDescriptionCollection references = await browser + .BrowseAsync(ObjectIds.ObjectsFolder) + .ConfigureAwait(false); + + Assert.That(references, Has.Count.EqualTo(2)); + Assert.That(references[0].DisplayName.Text, Is.EqualTo("First")); + Assert.That(references[1].DisplayName.Text, Is.EqualTo("Second")); + + sessionMock.Verify( + x => x.BrowseNextAsync( + It.IsAny(), + false, + It.IsAny(), + It.IsAny()), + Times.Once); + } + + private static void SetupBrowse( + Mock sessionMock, + ReferenceDescriptionCollection references, + byte[] continuationPoint) + { + sessionMock + .Setup(x => x.BrowseAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new BrowseResponse + { + ResponseHeader = new ResponseHeader(), + Results = CreateResults(references, continuationPoint), + DiagnosticInfos = [] + }); + } + + private static void SetupBrowseNext( + Mock sessionMock, + ReferenceDescriptionCollection references, + byte[] continuationPoint) + { + sessionMock + .Setup(x => x.BrowseNextAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new BrowseNextResponse + { + ResponseHeader = new ResponseHeader(), + Results = CreateResults(references, continuationPoint), + DiagnosticInfos = [] + }); + } + + private static BrowseResultCollection CreateResults( + ReferenceDescriptionCollection references, + byte[] continuationPoint) + { + return + [ + new BrowseResult + { + StatusCode = StatusCodes.Good, + ContinuationPoint = continuationPoint, + References = references + } + ]; + } + + private static ReferenceDescriptionCollection CreateReferences(string displayName) + { + return + [ + new ReferenceDescription + { + NodeId = new ExpandedNodeId(new NodeId(displayName, 1)), + DisplayName = displayName, + BrowseName = new QualifiedName(displayName, 1), + ReferenceTypeId = ReferenceTypeIds.HasComponent, + IsForward = true, + NodeClass = NodeClass.Object, + TypeDefinition = ObjectTypeIds.BaseObjectType + } + ]; + } + } +}