From 86d129de6ccfadebaebf9008ae9f5dbdc9b568bf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 23 Jul 2026 16:58:44 -0400 Subject: [PATCH] =?UTF-8?q?fix(dcl):=20harden=20endpoint=20host-rewrite=20?= =?UTF-8?q?=E2=80=94=20IPv6=20brackets=20+=20portless=20URLs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review of a1abbff7 found two edge cases: a discovery/advertised URL with no explicit port emitted a malformed ':-1' (opc.tcp has no Uri default port), and an IPv6 literal host could lose/double its brackets. Omit the port when neither URL carries one; bracket an IPv6 host only when missing. +4 tests (12 total). --- .../Adapters/RealOpcUaClient.cs | 12 ++++++++- .../RealOpcUaClientEndpointRewriteTests.cs | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs index 0c76acb5..8b511f26 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs @@ -191,11 +191,21 @@ public class RealOpcUaClient : IOpcUaClient && advertised.Port == reachable.Port) return; + // An IPv6 literal must keep its brackets, or the reconstructed authority (`::1:4840`) is + // ambiguous and unparseable. Uri.Host may or may not include them depending on the scheme, + // so bracket only when missing (never double-wrap). + var host = reachable.Host; + if (reachable.HostNameType == UriHostNameType.IPv6 && !host.StartsWith('[')) + host = $"[{host}]"; + // opc.tcp has no default port known to Uri, so Uri.Port is -1 when the text omits one. + // Prefer the reachable URL's port, fall back to the advertised one, and emit no ":port" + // at all when neither carries one (rather than a malformed ":-1"). var port = reachable.Port >= 0 ? reachable.Port : advertised.Port; + var authority = port >= 0 ? $"{host}:{port}" : host; // Authority-only advertisements parse to AbsolutePath "/" — drop it so we don't append a // spurious trailing slash the server never advertised. var path = advertised.AbsolutePath == "/" ? string.Empty : advertised.AbsolutePath; - endpoint.EndpointUrl = $"{advertised.Scheme}://{reachable.Host}:{port}{path}"; + endpoint.EndpointUrl = $"{advertised.Scheme}://{authority}{path}"; } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaClientEndpointRewriteTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaClientEndpointRewriteTests.cs index d9ed9182..ba7fb899 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaClientEndpointRewriteTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaClientEndpointRewriteTests.cs @@ -59,6 +59,31 @@ public class RealOpcUaClientEndpointRewriteTests Rewrite("opc.tcp://0.0.0.0:4840", "opc.tcp://host-b:4840")); } + [Fact] + public void Re_wraps_an_ipv6_reachable_host_in_brackets() + { + Assert.Equal( + "opc.tcp://[::1]:4840/OtOpcUa", + Rewrite("opc.tcp://0.0.0.0:4840/OtOpcUa", "opc.tcp://[::1]:4840/OtOpcUa")); + } + + [Fact] + public void Emits_no_port_when_neither_url_carries_one() + { + // opc.tcp has no default port, so Uri.Port is -1 on both — must not emit ":-1". + Assert.Equal( + "opc.tcp://myhost/UaServer", + Rewrite("opc.tcp://0.0.0.0/UaServer", "opc.tcp://myhost/UaServer")); + } + + [Fact] + public void Uses_the_reachable_port_when_the_advertisement_omits_one() + { + Assert.Equal( + "opc.tcp://myhost:4840/UaServer", + Rewrite("opc.tcp://0.0.0.0/UaServer", "opc.tcp://myhost:4840/UaServer")); + } + [Theory] [InlineData("")] [InlineData(" ")]