fix(abcip): controller browse honours the authored EtherNet/IP port (#539)

The two discovery-path libplctag tags (@tags symbol-list enumerator, @udt template reader) still passed the bare Gateway host, so a browse against a device authored on a non-default port silently browsed whatever answers on 44818 — #534's defect surviving in its discovery half. Both sites now consume AbCipTagCreateParams.LibplctagGateway, with the Tag construction extracted into internal static builders so the seam is unit-assertable (the #534 tests pinned the property's composition but nothing pinned each site's consumption, which is exactly how these two survived). New guard: LibplctagGatewayConsumptionGuardTests scans both AB drivers' sources and fails on any libplctag Tag built from the bare Gateway, so a future call site cannot reintroduce the silent 44818 redirect. Live-gated against the fixture host: browse of ab://10.100.0.35:44818 returns the nine-tag symbol table; browse of ab://10.100.0.35:44999 (nothing listening — the issue's exact repro) now fails ErrorTimeout where it previously returned the 44818 fixture's table. Closes #539. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-06 20:42:40 -04:00
parent d0ac6fc58c
commit 97a837c147
4 changed files with 137 additions and 20 deletions
@@ -26,17 +26,7 @@ internal sealed class LibplctagTagEnumerator : IAbCipTagEnumerator
        AbCipTagCreateParams deviceParams,
        [EnumeratorCancellation] CancellationToken cancellationToken)
    {
-        // Build a tag specifically for the @tags pseudo — same gateway + path as the device,
-        // distinguished by the name alone.
-        _tag = new Tag
-        {
-            Gateway = deviceParams.Gateway,
-            Path = deviceParams.CipPath,
-            PlcType = MapPlcType(deviceParams.LibplctagPlcAttribute),
-            Protocol = Protocol.ab_eip,
-            Name = "@tags",
-            Timeout = deviceParams.Timeout,
-        };
+        _tag = BuildSymbolListTag(deviceParams);

        await _tag.InitializeAsync(cancellationToken).ConfigureAwait(false);
        await _tag.ReadAsync(cancellationToken).ConfigureAwait(false);
@@ -49,6 +39,26 @@ internal sealed class LibplctagTagEnumerator : IAbCipTagEnumerator
            yield return tag;
    }

+    /// <summary>
+    ///     Builds the (uninitialized) libplctag tag for the <c>@tags</c> pseudo — same gateway +
+    ///     path as the device, distinguished by the name alone.
+    /// </summary>
+    /// <remarks>
+    ///     Gateway must be <see cref="AbCipTagCreateParams.LibplctagGateway"/> (host:port), never the
+    ///     bare <see cref="AbCipTagCreateParams.Gateway"/> — libplctag substitutes 44818 for a bare
+    ///     host, so a browse against a device authored on another port silently returns whatever
+    ///     answers on 44818 (Gitea #539, the discovery half of #534).
+    /// </remarks>
+    internal static Tag BuildSymbolListTag(AbCipTagCreateParams deviceParams) => new()
+    {
+        Gateway = deviceParams.LibplctagGateway,
+        Path = deviceParams.CipPath,
+        PlcType = MapPlcType(deviceParams.LibplctagPlcAttribute),
+        Protocol = Protocol.ab_eip,
+        Name = "@tags",
+        Timeout = deviceParams.Timeout,
+    };
+
    /// <summary>Disposes the enumerator and releases the underlying libplctag tag.</summary>
    public void Dispose() => _tag?.Dispose();

@@ -19,20 +19,29 @@ internal sealed class LibplctagTemplateReader : IAbCipTemplateReader
        CancellationToken cancellationToken)
    {
        _tag?.Dispose();
-        _tag = new Tag
-        {
-            Gateway = deviceParams.Gateway,
-            Path = deviceParams.CipPath,
-            PlcType = MapPlcType(deviceParams.LibplctagPlcAttribute),
-            Protocol = Protocol.ab_eip,
-            Name = $"@udt/{templateInstanceId}",
-            Timeout = deviceParams.Timeout,
-        };
+        _tag = BuildTemplateTag(deviceParams, templateInstanceId);
        await _tag.InitializeAsync(cancellationToken).ConfigureAwait(false);
        await _tag.ReadAsync(cancellationToken).ConfigureAwait(false);
        return _tag.GetBuffer();
    }

+    /// <summary>Builds the (uninitialized) libplctag tag for the <c>@udt/{id}</c> pseudo.</summary>
+    /// <remarks>
+    ///     Gateway must be <see cref="AbCipTagCreateParams.LibplctagGateway"/> (host:port), never the
+    ///     bare <see cref="AbCipTagCreateParams.Gateway"/> — libplctag substitutes 44818 for a bare
+    ///     host, so a browse against a device authored on another port silently returns whatever
+    ///     answers on 44818 (Gitea #539, the discovery half of #534).
+    /// </remarks>
+    internal static Tag BuildTemplateTag(AbCipTagCreateParams deviceParams, uint templateInstanceId) => new()
+    {
+        Gateway = deviceParams.LibplctagGateway,
+        Path = deviceParams.CipPath,
+        PlcType = MapPlcType(deviceParams.LibplctagPlcAttribute),
+        Protocol = Protocol.ab_eip,
+        Name = $"@udt/{templateInstanceId}",
+        Timeout = deviceParams.Timeout,
+    };
+
    /// <summary>Disposes the template reader and releases the underlying libplctag tag.</summary>
    public void Dispose() => _tag?.Dispose();

@@ -41,4 +41,26 @@ public sealed class AbCipGatewayCompositionTests
    {
        Params("10.100.0.35", port).LibplctagGateway.ShouldBe("10.100.0.35");
    }
+
+    // Gitea #539 — #534 fixed the data path but both DISCOVERY call sites kept passing the bare
+    // Gateway, so "Browse controller" against a device authored on a non-default port silently
+    // browsed whatever answers on 44818. These pin the composed gateway at the actual libplctag
+    // Tag construction seam, which the property-only tests above structurally cannot: they proved
+    // LibplctagGateway composes correctly while nothing proved each Tag builder consumes it.
+
+    [Fact]
+    public void The_symbol_list_browse_tag_carries_the_authored_port()
+    {
+        using var tag = LibplctagTagEnumerator.BuildSymbolListTag(Params("10.100.0.35", 44999));
+        tag.Gateway.ShouldBe("10.100.0.35:44999");
+        tag.Name.ShouldBe("@tags");
+    }
+
+    [Fact]
+    public void The_template_read_tag_carries_the_authored_port()
+    {
+        using var tag = LibplctagTemplateReader.BuildTemplateTag(Params("10.100.0.35", 44999), templateInstanceId: 0x123);
+        tag.Gateway.ShouldBe("10.100.0.35:44999");
+        tag.Name.ShouldBe("@udt/291");
+    }
 }
@@ -0,0 +1,76 @@
+using System.Text.RegularExpressions;
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
+
+/// <summary>
+///     Gitea #539 — every libplctag <c>Tag</c> the AB drivers construct must take its
+///     <c>Gateway</c> from <c>LibplctagGateway</c> (host:port), never the bare <c>Gateway</c>
+///     host.
+/// </summary>
+/// <remarks>
+///     libplctag substitutes 44818 for a bare host, so a dropped port does not fail — it
+///     silently redirects to whatever answers on the default port, which on a plant network is a
+///     different PLC. #534 fixed the data-path sites and #539 found the two discovery sites had
+///     the same defect; this scan pins ALL current and future construction sites in both AB
+///     drivers, because the defect recurred precisely by being reintroduced at a call site the
+///     per-site tests did not cover.
+/// </remarks>
+[Trait("Category", "Unit")]
+public sealed class LibplctagGatewayConsumptionGuardTests
+{
+    // Matches a Gateway assignment inside an object initializer (property definitions and the
+    // LibplctagGateway composition expression itself do not start a line with "Gateway =").
+    private static readonly Regex GatewayAssignment = new(@"^\s*Gateway\s*=\s*(?<rhs>.+?),?\s*$", RegexOptions.Multiline);
+
+    [Fact]
+    public void Every_Tag_construction_in_both_AB_drivers_uses_LibplctagGateway()
+    {
+        var repoRoot = LocateRepoRoot();
+        var driverDirs = new[]
+        {
+            Path.Combine(repoRoot, "src", "Drivers", "ZB.MOM.WW.OtOpcUa.Driver.AbCip"),
+            Path.Combine(repoRoot, "src", "Drivers", "ZB.MOM.WW.OtOpcUa.Driver.AbLegacy"),
+        };
+
+        var offending = new List<string>();
+        var assignmentCount = 0;
+
+        foreach (var dir in driverDirs)
+        {
+            Directory.Exists(dir).ShouldBeTrue($"Expected driver source directory {dir} to exist.");
+            var files = Directory.EnumerateFiles(dir, "*.cs", SearchOption.AllDirectories)
+                .Where(f => !f.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}")
+                         && !f.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}"));
+
+            foreach (var file in files)
+            {
+                foreach (Match match in GatewayAssignment.Matches(File.ReadAllText(file)))
+                {
+                    assignmentCount++;
+                    if (!match.Groups["rhs"].Value.Contains("LibplctagGateway"))
+                        offending.Add($"{file}: {match.Value.Trim()}");
+                }
+            }
+        }
+
+        offending.ShouldBeEmpty(
+            "A libplctag Tag built from the bare Gateway host drops the authored EtherNet/IP port and " +
+            "silently connects to whatever answers on 44818 (Gitea #534/#539). Use LibplctagGateway.");
+
+        // The four known sites: AbCip runtime + symbol-list enumerator + template reader, AbLegacy
+        // runtime. If this drops below 4, the scan's assignment pattern no longer matches the code
+        // shape and the guard has gone blind — fix the regex, don't lower the floor.
+        assignmentCount.ShouldBeGreaterThanOrEqualTo(4);
+    }
+
+    private static string LocateRepoRoot()
+    {
+        var dir = new DirectoryInfo(AppContext.BaseDirectory);
+        while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx")))
+            dir = dir.Parent;
+        dir.ShouldNotBeNull("Could not locate the repo root (ZB.MOM.WW.OtOpcUa.slnx) above the test assembly.");
+        return dir.FullName;
+    }
+}