Files
lmxopcua/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs
T
Joseph Doherty 4ec01bdfc1 fix(mqtt): bound browse observation against a chatty or malicious broker
Three review findings, all "unbounded work/memory from untrusted broker input"
rather than correctness bugs - but this points at a live plant broker.

1. InferPayload decoded the WHOLE payload on MQTTnet's dispatcher thread before
   trimming to 64 chars. The node cap never engages here: a multi-MB blob on an
   already-known topic creates no node, it just updates one, so the cost repeats
   per message forever. Now at most PayloadInspectMaxBytes (512) are examined.
   A blind slice can cut a multi-byte sequence, which the strict decoder would
   report as "binary" - TrimToUtf8Boundary backs the window off to the last whole
   sequence so good UTF-8 is never mislabelled. A clipped payload is String by
   construction rather than inferred from a partial view.

2. The node cap bounded COUNT, not bytes. MQTT topics run to ~65KB, so 50k nodes
   near that ceiling is a multi-GB tree. Topics now bounded at 1024 chars and
   segments at 256, rejected whole (a truncated topic is a DIFFERENT topic the
   picker would commit as a tag address) and surfaced via an __oversized__ marker
   kept distinct from __truncated__ - the operator's remedy differs.

3. Control characters in a snippet would break the picker's single-line row;
   Trim() only strips the ends. Now scrubbed to spaces.

Each guard was falsified independently by neutering it and confirming exactly one
test reddens. That found a real gap: the oversized-topic test was passing via the
SEGMENT bound, leaving the whole-topic bound untested - the test now uses many
short segments so only the topic bound can reject it.

Also records the .Driver project reference as a known, deliberate exception to the
.Browser -> .Contracts pattern, with its cost (AdminUI gains Core + Polly + Serilog)
and the clean fix (a leaf transport project; NOT a move into .Contracts, which is
deliberately transport-free).

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
2026-07-24 16:30:45 -04:00

438 lines
19 KiB
C#

using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Commons.Browsing;
using ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser;
namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests;
/// <summary>
/// Covers the passive <c>#</c>-observation browse session and the browser that opens it. The
/// session is exercised through its test seam (<c>ObserveTopicForTest</c>) rather than a live
/// broker: the accumulating tree is the whole of the P1 logic, and the MQTTnet plumbing that
/// feeds it is a one-line handler.
/// </summary>
public sealed class MqttBrowseSessionTests
{
// ---------------------------------------------------------------- tree shape
[Fact]
public async Task ExpandAsync_BuildsTopicSegmentTree_FromObservedTopics()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("factory/line3/oven/temp");
var root = await s.RootAsync(TestContext.Current.CancellationToken);
root.ShouldContain(n => n.DisplayName == "factory");
var lvl2 = await s.ExpandAsync("factory", TestContext.Current.CancellationToken);
lvl2.ShouldContain(n => n.DisplayName == "line3");
var lvl3 = await s.ExpandAsync("factory/line3", TestContext.Current.CancellationToken);
lvl3.ShouldContain(n => n.DisplayName == "oven");
var lvl4 = await s.ExpandAsync("factory/line3/oven", TestContext.Current.CancellationToken);
var leaf = lvl4.ShouldHaveSingleItem();
leaf.DisplayName.ShouldBe("temp");
leaf.NodeId.ShouldBe("factory/line3/oven/temp");
leaf.Kind.ShouldBe(BrowseNodeKind.Leaf);
leaf.HasChildrenHint.ShouldBeFalse();
}
[Fact]
public async Task ObservedTopics_AccumulateAcrossMessages_AndDoNotDuplicate()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("factory/line3/oven/temp");
s.ObserveTopicForTest("factory/line3/oven/temp"); // repeat — must not duplicate
s.ObserveTopicForTest("factory/line3/oven/pressure");
s.ObserveTopicForTest("factory/line4/mixer/rpm");
s.ObserveTopicForTest("plant/power");
var root = await s.RootAsync(TestContext.Current.CancellationToken);
root.Select(n => n.DisplayName).ShouldBe(["factory", "plant"]); // sorted, deduped
var lines = await s.ExpandAsync("factory", TestContext.Current.CancellationToken);
lines.Select(n => n.DisplayName).ShouldBe(["line3", "line4"]);
var oven = await s.ExpandAsync("factory/line3/oven", TestContext.Current.CancellationToken);
oven.Select(n => n.DisplayName).ShouldBe(["pressure", "temp"]);
}
[Fact]
public async Task IntermediateNode_IsFolder_WithChildrenHint()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("factory/line3/oven/temp");
var root = await s.RootAsync(TestContext.Current.CancellationToken);
var factory = root.ShouldHaveSingleItem();
factory.Kind.ShouldBe(BrowseNodeKind.Folder);
factory.HasChildrenHint.ShouldBeTrue();
}
[Fact]
public async Task ExpandAsync_UnknownNode_ReturnsEmpty()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("factory/line3");
var children = await s.ExpandAsync("nope/not/here", TestContext.Current.CancellationToken);
children.ShouldBeEmpty();
}
[Fact]
public async Task RootAsync_BeforeAnyTraffic_IsEmpty()
{
// MQTT has no discovery protocol: an observation window that has seen nothing shows nothing.
await using var s = new MqttBrowseSession(MqttMode.Plain);
(await s.RootAsync(TestContext.Current.CancellationToken)).ShouldBeEmpty();
}
// ---------------------------------------------------------------- read-only guarantee
[Fact]
public async Task BrowseCalls_PublishNothing() // browse is read-only
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("factory/line3/oven/temp", "23.5");
await s.RootAsync(TestContext.Current.CancellationToken);
await s.ExpandAsync("factory", TestContext.Current.CancellationToken);
await s.AttributesAsync("factory/line3/oven/temp", TestContext.Current.CancellationToken);
s.PublishCountForTest.ShouldBe(0);
}
// ---------------------------------------------------------------- attributes
[Theory]
[InlineData("23.5", nameof(Core.Abstractions.DriverDataType.Float64))]
[InlineData("42", nameof(Core.Abstractions.DriverDataType.Int64))]
[InlineData("true", nameof(Core.Abstractions.DriverDataType.Boolean))]
[InlineData("{\"v\":1}", nameof(Core.Abstractions.DriverDataType.String))]
public async Task AttributesAsync_InfersTypeFromLastPayload(string payload, string expectedType)
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("factory/oven/temp", payload);
var attrs = await s.AttributesAsync("factory/oven/temp", TestContext.Current.CancellationToken);
var a = attrs.ShouldHaveSingleItem();
a.Name.ShouldBe("temp");
a.DriverDataType.ShouldBe(expectedType);
a.IsArray.ShouldBeFalse();
a.IsAlarm.ShouldBeFalse();
a.SecurityClass.ShouldContain(payload); // the last-payload snippet
}
[Fact]
public async Task AttributesAsync_LongPayload_IsTruncatedToASnippet()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("t/long", new string('x', 500));
var a = (await s.AttributesAsync("t/long", TestContext.Current.CancellationToken)).ShouldHaveSingleItem();
a.SecurityClass.Length.ShouldBeLessThanOrEqualTo(MqttBrowseSession.PayloadSnippetMaxChars + 1);
}
[Fact]
public async Task AttributesAsync_UnobservedFolder_ReturnsEmpty()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("factory/oven/temp", "1");
// "factory" is a synthesised path segment — nothing was ever published to it.
(await s.AttributesAsync("factory", TestContext.Current.CancellationToken)).ShouldBeEmpty();
(await s.AttributesAsync("no/such/node", TestContext.Current.CancellationToken)).ShouldBeEmpty();
}
// ---------------------------------------------------------------- concurrency
[Fact]
public async Task ObserveDuringBrowse_IsThreadSafe()
{
// Messages arrive on MQTTnet's dispatcher thread while the picker calls ExpandAsync.
await using var s = new MqttBrowseSession(MqttMode.Plain);
using var stop = new CancellationTokenSource();
var observed = 0;
var writer = Task.Run(() =>
{
var i = 0;
while (!stop.Token.IsCancellationRequested)
{
s.ObserveTopicForTest($"factory/line{i % 8}/dev{i % 32}/m{i}", i.ToString());
Interlocked.Increment(ref observed);
i++;
}
}, CancellationToken.None);
// The read loop is pure in-memory work and would otherwise finish before the writer thread
// is even scheduled, leaving the two never actually overlapping.
while (Volatile.Read(ref observed) == 0) await Task.Yield();
for (var i = 0; i < 400; i++)
{
await s.RootAsync(TestContext.Current.CancellationToken);
await s.ExpandAsync("factory", TestContext.Current.CancellationToken);
await s.ExpandAsync("factory/line3", TestContext.Current.CancellationToken);
}
await stop.CancelAsync();
await writer; // rethrows anything the observer thread hit
(await s.RootAsync(TestContext.Current.CancellationToken)).ShouldNotBeEmpty();
}
// ---------------------------------------------------------------- lifetime
[Fact]
public async Task LastUsedUtc_AdvancesOnEveryCall()
{
var s = new MqttBrowseSession(MqttMode.Plain);
var before = s.LastUsedUtc;
await Task.Delay(15, TestContext.Current.CancellationToken);
await s.RootAsync(TestContext.Current.CancellationToken);
s.LastUsedUtc.ShouldBeGreaterThan(before);
var afterRoot = s.LastUsedUtc;
await Task.Delay(15, TestContext.Current.CancellationToken);
await s.ExpandAsync("x", TestContext.Current.CancellationToken);
s.LastUsedUtc.ShouldBeGreaterThan(afterRoot);
await s.DisposeAsync();
}
[Fact]
public async Task DisposeAsync_OnNeverOpenedSession_IsIdempotentAndDoesNotThrow()
{
var s = new MqttBrowseSession(MqttMode.Plain);
await Should.NotThrowAsync(async () => await s.DisposeAsync());
await Should.NotThrowAsync(async () => await s.DisposeAsync());
}
[Fact]
public async Task AfterDispose_BrowseCallsThrowObjectDisposed()
{
var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("a/b");
await s.DisposeAsync();
await Should.ThrowAsync<ObjectDisposedException>(
async () => await s.RootAsync(TestContext.Current.CancellationToken));
await Should.ThrowAsync<ObjectDisposedException>(
async () => await s.ExpandAsync("a", TestContext.Current.CancellationToken));
await Should.ThrowAsync<ObjectDisposedException>(
async () => await s.AttributesAsync("a/b", TestContext.Current.CancellationToken));
}
[Fact]
public async Task ObserveAfterDispose_IsIgnored()
{
// A message already in MQTTnet's dispatcher when the reaper disposes the session.
var s = new MqttBrowseSession(MqttMode.Plain);
await s.DisposeAsync();
Should.NotThrow(() => s.ObserveTopicForTest("late/arrival", "1"));
}
// ---------------------------------------------------------------- node cap
[Fact]
public async Task NodeCap_StopsRecording_AndSurfacesATruncationMarker()
{
await using var s = new MqttBrowseSession(MqttMode.Plain, nodeCap: 4);
s.ObserveTopicForTest("a/b/c"); // 3 nodes
s.ObserveTopicForTest("d/e/f"); // would take it past 4
var root = await s.RootAsync(TestContext.Current.CancellationToken);
root.ShouldContain(n => n.NodeId == MqttBrowseSession.TruncatedNodeId);
}
[Fact]
public async Task OversizedTopic_IsRejectedWhole_AndSurfacesAMarker()
{
// A truncated topic is a DIFFERENT topic — the picker would commit it as a tag address.
// Every segment here is 4 chars, well under MaxSegmentChars, so ONLY the whole-topic bound
// can reject it — the segment guard must not be what makes this test pass.
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("good/topic", "1");
var manyShortSegments = "huge/" + string.Join('/', Enumerable.Repeat("abc", MqttBrowseSession.MaxTopicChars / 4 + 8));
manyShortSegments.Length.ShouldBeGreaterThan(MqttBrowseSession.MaxTopicChars);
manyShortSegments.Split('/').ShouldAllBe(seg => seg.Length <= MqttBrowseSession.MaxSegmentChars);
s.ObserveTopicForTest(manyShortSegments, "1");
var root = await s.RootAsync(TestContext.Current.CancellationToken);
root.ShouldContain(n => n.NodeId == MqttBrowseSession.OversizedNodeId);
root.ShouldContain(n => n.DisplayName == "good");
root.ShouldNotContain(n => n.DisplayName == "huge"); // no half-built path left behind
}
[Fact]
public async Task OversizedSegment_IsRejectedWhole_EvenWhenTheTopicFits()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("a/" + new string('y', MqttBrowseSession.MaxSegmentChars + 1) + "/b", "1");
s.OversizedRejected.ShouldBeTrue();
(await s.ExpandAsync("a", TestContext.Current.CancellationToken)).ShouldBeEmpty();
var root = await s.RootAsync(TestContext.Current.CancellationToken);
root.ShouldNotContain(n => n.DisplayName == "a");
}
[Fact]
public async Task TopicAtExactlyTheCap_IsAccepted()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest(new string('z', MqttBrowseSession.MaxSegmentChars), "1");
s.OversizedRejected.ShouldBeFalse();
(await s.RootAsync(TestContext.Current.CancellationToken)).ShouldHaveSingleItem();
}
// ---------------------------------------------------------------- payload bounds
[Fact]
public async Task PayloadBeyondTheInspectionWindow_IsNeverExamined()
{
// The discriminating test for the byte cap: content past PayloadInspectMaxBytes cannot
// influence the result at all. With an unbounded decode the trailing scalar would be found
// and typed Int64, and would show up in the snippet.
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest(
"t/tail",
new string(' ', MqttBrowseSession.PayloadInspectMaxBytes + 64) + "42");
var a = (await s.AttributesAsync("t/tail", TestContext.Current.CancellationToken)).ShouldHaveSingleItem();
a.DriverDataType.ShouldBe(nameof(Core.Abstractions.DriverDataType.String));
a.SecurityClass.ShouldNotContain("42");
}
[Fact]
public async Task HugePayload_YieldsABoundedSnippetMarkedAsClipped()
{
// A '#' subscription receives multi-MB retained blobs; the node cap never engages for a
// large payload on an already-known topic, so the decode itself must be bounded.
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("t/blob", new string('a', 5_000_000));
var a = (await s.AttributesAsync("t/blob", TestContext.Current.CancellationToken)).ShouldHaveSingleItem();
a.SecurityClass.Length.ShouldBeLessThanOrEqualTo(MqttBrowseSession.PayloadSnippetMaxChars + 1);
a.SecurityClass.ShouldEndWith("…");
a.DriverDataType.ShouldBe(nameof(Core.Abstractions.DriverDataType.String));
}
[Fact]
public async Task ClippedMultiByteUtf8_IsNotMislabelledAsBinary()
{
// The inspection window can land mid-sequence; backing off to a whole sequence keeps a
// perfectly good UTF-8 payload from being reported as binary.
await using var s = new MqttBrowseSession(MqttMode.Plain);
// 3 bytes per char, so 512 / 3 leaves a partial sequence at the window edge.
s.ObserveTopicForTest("t/utf8", new string('☃', 4000));
var a = (await s.AttributesAsync("t/utf8", TestContext.Current.CancellationToken)).ShouldHaveSingleItem();
a.SecurityClass.ShouldStartWith("☃");
a.SecurityClass.ShouldNotContain("binary");
a.SecurityClass.ShouldNotContain(""); // no replacement char from a split sequence
}
[Fact]
public async Task BinaryPayload_IsReportedAsOpaque_WithItsTrueLength()
{
await using var s = new MqttBrowseSession(MqttMode.Plain);
var binary = new byte[2048];
Array.Fill(binary, (byte)0xC0); // never legal UTF-8
s.Observe("t/bin", binary);
var a = (await s.AttributesAsync("t/bin", TestContext.Current.CancellationToken)).ShouldHaveSingleItem();
a.SecurityClass.ShouldBe("(2048 bytes, binary)"); // full length, not the clipped window
}
[Fact]
public async Task EmbeddedControlCharacters_AreScrubbedToSpaces()
{
// Trim() only strips the ends; an embedded newline would break the picker's single-line row.
await using var s = new MqttBrowseSession(MqttMode.Plain);
s.ObserveTopicForTest("t/multiline", "line one\nline two\ttabbed");
var a = (await s.AttributesAsync("t/multiline", TestContext.Current.CancellationToken)).ShouldHaveSingleItem();
a.SecurityClass.ShouldNotContain("\n");
a.SecurityClass.ShouldNotContain("\t");
a.SecurityClass.ShouldNotContain("");
a.SecurityClass.ShouldStartWith("line one line two tabbed");
}
// ---------------------------------------------------------------- the browser
[Fact]
public void Ctor_IsConnectionFree_AndReportsTheMqttDriverType()
{
// The universal-browser CanBrowse pattern constructs a throwaway instance purely to ask
// what driver type it handles — the ctor must never touch the network.
var browser = new MqttDriverBrowser();
browser.DriverType.ShouldBe("Mqtt");
}
[Fact]
public async Task OpenAsync_SparkplugMode_FailsFastWithoutConnecting()
{
var browser = new MqttDriverBrowser();
const string cfg = """{"host":"broker.invalid","port":8883,"mode":"SparkplugB"}""";
await Should.ThrowAsync<NotSupportedException>(
async () => await browser.OpenAsync(cfg, TestContext.Current.CancellationToken));
}
[Theory]
[InlineData(null, "#")]
[InlineData("", "#")]
[InlineData(" ", "#")]
[InlineData("factory", "factory/#")]
[InlineData("factory/", "factory/#")]
[InlineData("factory/line3", "factory/line3/#")]
public void BuildRootFilter_ScopesTheWildcardToTheConfiguredPrefix(string? prefix, string expected)
{
var opts = new MqttDriverOptions
{
Mode = MqttMode.Plain,
Plain = prefix is null ? null : new MqttPlainOptions { TopicPrefix = prefix },
};
MqttDriverBrowser.BuildRootFilter(opts).ShouldBe(expected);
}
[Fact]
public void BrowseClientIdSuffix_IsUnique_AndIsAppliedToTheConnectClientId()
{
// A broker disconnects an existing client when a new one CONNECTs with the same id —
// browsing must never knock the running driver offline.
var a = MqttDriverBrowser.BuildBrowseClientIdSuffix();
var b = MqttDriverBrowser.BuildBrowseClientIdSuffix();
a.ShouldNotBe(b);
a.ShouldStartWith("-browse-");
a.Length.ShouldBe("-browse-".Length + 8);
var opts = new MqttDriverOptions { ClientId = "otopcua-node1" };
MqttConnection.BuildClientOptions(opts, a).ClientId.ShouldBe("otopcua-node1" + a);
}
[Fact]
public void BrowseConnect_ForcesACleanSession()
{
// A transient browse identity must not leave queued-message state behind on the broker.
var opts = new MqttDriverOptions { CleanSession = false };
MqttDriverBrowser.ToBrowseOptions(opts).CleanSession.ShouldBeTrue();
}
[Theory]
[InlineData(1, 5)] // clamped up
[InlineData(15, 15)]
[InlineData(600, 30)] // clamped down
public void OpenBudget_IsClampedTo5To30Seconds(int configured, int expected)
{
var opts = new MqttDriverOptions { ConnectTimeoutSeconds = configured };
MqttDriverBrowser.OpenBudget(opts).ShouldBe(TimeSpan.FromSeconds(expected));
}
}