Skip to content

Normalize endpoint URIs in DiscoveryClient while preserving IPv6 scope IDs#3301

Closed
Copilot wants to merge 3 commits intomasterfrom
copilot/fix-discoveryclient-uri-issue
Closed

Normalize endpoint URIs in DiscoveryClient while preserving IPv6 scope IDs#3301
Copilot wants to merge 3 commits intomasterfrom
copilot/fix-discoveryclient-uri-issue

Conversation

Copy link
Contributor

Copilot AI commented Oct 30, 2025

DiscoveryClient.Create() uses Uri.OriginalString, causing semantically identical URIs to behave differently due to surface-level differences (e.g., trailing slashes). This breaks discovery against some OPC UA servers (Siemens Sinumerik ONE NC-Server).

var uriWithSlash = new Uri("opc.tcp://hostname:4840/");
var uriNoSlash = new Uri("opc.tcp://hostname:4840");

// Before: Different endpoint counts (7 vs 14) with Siemens server
// After: Consistent behavior - both normalize to "opc.tcp://hostname:4840/"

Changes

  • Added GetNormalizedEndpointUrl() helper: Reconstructs URIs from parsed components (Scheme, DnsSafeHost, Port, AbsolutePath) to normalize while preserving IPv6 scope IDs that Uri.ToString() strips
  • Updated CreateChannelAsync() overloads: Replace Uri.OriginalString with normalized URL in all three overloads
  • Added test coverage: Verify normalization behavior and IPv6 scope ID preservation (%eth0, %12)

Technical Note

Cannot use Uri.ToString() directly—it strips IPv6 scope IDs. DnsSafeHost preserves them while providing normalized host representation.

Original prompt

This section details on the original issue you should resolve

<issue_title>DiscoveryClient.Create should not use Uri.OriginalString</issue_title>
<issue_description>### Type of issue

  • Bug
  • Enhancement
  • Compliance
  • Question
  • Help wanted

Current Behavior

As evident in the source code, DiscoveryClient.Create() currently uses Uri.OriginalString.

This negates any and all advantages of using an Uri over a regular string.

Some OPC UA Servers (in my test case: Siemens Sinumerik ONE NC-Server) behave differently, even though at first glance the URIs appear to be identical - see the following screenshot:

Image

Obviously, changing the behavior of DiscoveryClient.Create won't fix the Siemens Server - but at least identical URIs passed to the method will behave identically.

Expected Behavior

"Identical" URIs should result in identical behavior.
DiscoveryClient.Create() should use the parsed value (Uri.ToString()) instead of the original input value (Uri.OriginalString)

Steps To Reproduce

This behavior as shown in the screenshot was not reproducible with a local Opc.Ua.Core.Server ApplicationInstance - only with a Siemens Sinumerik ONE server.
Regardless - using URIs should provide a benefit over strings.
Sample code as in the screenshot above:

using Opc.Ua;

var hostname = "REDACTED";
var uriWithTrailingSlash = new Uri($"opc.tcp://{hostname}:4840/");
var uriWithoutTrailingSlash = new Uri($"opc.tcp://{hostname}:4840");

var discoveryClientWithoutTrailingSlash = DiscoveryClient.Create(uriWithoutTrailingSlash);
var allEndpointsWithoutTrailingSlash = await discoveryClientWithoutTrailingSlash.GetEndpointsAsync(null);

var discoveryClientWithTrailingSlash = DiscoveryClient.Create(uriWithTrailingSlash);
var allEndpointsWithTrailingSlash = await discoveryClientWithTrailingSlash.GetEndpointsAsync(null);

Console.WriteLine($"WithTrailingSlash: {allEndpointsWithTrailingSlash.Count}, WithoutTrailingSlash: {allEndpointsWithoutTrailingSlash.Count}");

Environment

- OS: Windows
- Environment: Visual Studio 2022
- Runtime: dotnet 8
- Nuget Version: 1.5.376.244
- Component: Opc.Ua.Core
- Server: Siemens Sinumerik ONE
- Client: Opc.Ua.Core.DiscoveryClient

Anything else?

No response</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI and others added 2 commits October 30, 2025 14:33
…ng slash issue

- Added GetNormalizedEndpointUrl helper method to normalize URIs while preserving IPv6 scope IDs
- Updated all three CreateChannelAsync methods to use normalized URLs
- Added comprehensive unit tests to verify normalization behavior
- All tests pass including existing IPv6 scope ID preservation tests

Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
- Changed GetNormalizedEndpointUrl from private to internal
- Updated test to call internal method directly instead of using reflection
- All tests still pass

Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix DiscoveryClient.Create to avoid using Uri.OriginalString Normalize endpoint URIs in DiscoveryClient while preserving IPv6 scope IDs Oct 30, 2025
Copilot AI requested a review from marcschier October 30, 2025 14:42
@marcschier marcschier marked this pull request as ready for review November 1, 2025 18:33
@codecov
Copy link

codecov bot commented Nov 1, 2025

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 58.10%. Comparing base (820f6a2) to head (bce4e13).
⚠️ Report is 24 commits behind head on master.

Files with missing lines Patch % Lines
Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3301      +/-   ##
==========================================
+ Coverage   57.86%   58.10%   +0.23%     
==========================================
  Files         367      367              
  Lines       79969    80103     +134     
  Branches    13875    13921      +46     
==========================================
+ Hits        46274    46540     +266     
+ Misses      29490    29350     -140     
- Partials     4205     4213       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcschier marcschier marked this pull request as draft November 2, 2025 16:38
[TestCase("opc.tcp://[fe80::de39:6fff:feae:c78%12]:4840/Endpoint1",
"opc.tcp://[fe80::de39:6fff:feae:c78%12]:4840/Endpoint1",
"opc.tcp://[fe80::de39:6fff:feae:c78%12]:4840/Endpoint1")]
public void DiscoveryEndPointUrlNormalization(string url1, string url2, string expectedNormalized)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method signature is confusing. Why test 2 inputs at the same time?

Refactor to one input and the expected compare value.

}

// Reconstruct the URL
return $"{uri.Scheme}://{host}:{uri.Port}{uri.AbsolutePath}";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are "normal" URIs that do not contain any IPv6 scope id also treated differently? In this case uri.ToString() should suffice.

{
Host = endpointUrl.IdnHost
};
discoveryEndPoint.EndpointUrl = builder.Uri.OriginalString;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed one?!

@marcschier marcschier closed this Nov 13, 2025
@romanett romanett deleted the copilot/fix-discoveryclient-uri-issue branch January 14, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DiscoveryClient.Create should not use Uri.OriginalString

4 participants