From 205b07f6aab5713b109cba818b08571e77146410 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 09:21:46 -0400 Subject: [PATCH] fix(driver-historian-wonderware): resolve Medium code-review finding (Driver.Historian.Wonderware-002) Normalise req.Events to Array.Empty() immediately after MessagePack deserialization in HandleWriteAlarmEventsAsync. MessagePack deserializes an absent or explicit-nil array field as null, not Array.Empty, so a peer that sends a null Events array would trigger a NullReferenceException on either .Length dereference (no-writer branch or catch block), leaving the client correlation-id wait hanging with no reply frame. Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/Driver.Historian.Wonderware/findings.md | 4 ++-- .../Ipc/HistorianFrameHandler.cs | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/code-reviews/Driver.Historian.Wonderware/findings.md b/code-reviews/Driver.Historian.Wonderware/findings.md index 86c70d1..bbfa113 100644 --- a/code-reviews/Driver.Historian.Wonderware/findings.md +++ b/code-reviews/Driver.Historian.Wonderware/findings.md @@ -63,7 +63,7 @@ the reconnect path can re-open with `ReadOnly = false`) or at minimum as | Severity | Medium | | Category | Correctness and logic bugs | | Location | `Ipc/HistorianFrameHandler.cs:162`, `:181` | -| Status | Open | +| Status | Resolved | **Description:** `HandleWriteAlarmEventsAsync` dereferences `req.Events.Length` in both the `_alarmWriter is null` branch (line 162) and the catch block (line @@ -79,7 +79,7 @@ already null-guards `events`; the frame handler does not. immediately after deserialization (or guard each `.Length` access), consistent with the null-tolerance the writer already has. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — normalise `req.Events` to `Array.Empty()` immediately after deserialization so all subsequent `.Length` accesses are safe against null frames. ### Driver.Historian.Wonderware-003 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Ipc/HistorianFrameHandler.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Ipc/HistorianFrameHandler.cs index da85830..3779803 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Ipc/HistorianFrameHandler.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Ipc/HistorianFrameHandler.cs @@ -153,6 +153,11 @@ public sealed class HistorianFrameHandler : IFrameHandler private async Task HandleWriteAlarmEventsAsync(byte[] body, FrameWriter writer, CancellationToken ct) { var req = MessagePackSerializer.Deserialize(body); + + // MessagePack deserializes an absent or explicit-nil array as null, not Array.Empty. + // Normalise here so every path below can safely dereference .Length without an NRE. + req.Events ??= Array.Empty(); + var reply = new WriteAlarmEventsReply { CorrelationId = req.CorrelationId }; if (_alarmWriter is null)