From 41e600e0694546968c514d71f23540a9bb24d91c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 16:51:20 -0400 Subject: [PATCH] fix(s7): classic STRING/DT unrepresentable writes map to BadOutOfRange, not BadCommunicationError (#632) S7.Net's own STRING/DT encoders throw ArgumentException; that fell into the generic write catch (BadCommunicationError + Degraded health). Now surfaced as S7ValueRangeException like the WSTRING/DATE codecs. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs | 11 +++++-- .../S7ScalarBlockTests.cs | 4 +-- .../S7ValueCodecTests.cs | 29 +++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs index 43a5dc08..6d95a8aa 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -1555,14 +1555,21 @@ public sealed class S7Driver // ReadScalarBlockAsync read, so WriteBytesAsync writes the whole reserved STRING. A value // longer than StringLength throws ArgumentException (S7.Net rejects overflow; we do NOT // silently truncate). A null value encodes as the empty string. - return global::S7.Net.Types.S7String.ToByteArray(Convert.ToString(value) ?? "", tag.StringLength); + // #632: surfaced as S7ValueRangeException (→ BadOutOfRange) like the WSTRING codec — a + // client value error, not a comms fault (the raw ArgumentException used to land in the + // generic catch as BadCommunicationError + Degraded). + try { return global::S7.Net.Types.S7String.ToByteArray(Convert.ToString(value) ?? "", tag.StringLength); } + catch (ArgumentException ex) { throw new S7ValueRangeException($"S7 STRING value does not fit the declared {tag.StringLength}-character field: {ex.Message}"); } case S7DataType.DateTime: // S7.Net's DateTime.ToByteArray builds the 8-byte DT BCD block. It validates the // 1990–2089 DT range and throws ArgumentOutOfRangeException for an out-of-range year — // we surface that (no silent clamp). Value coerced via Convert.ToDateTime (accepts a // System.DateTime or a parseable timestamp string). - return global::S7.Net.Types.DateTime.ToByteArray(Convert.ToDateTime(value)); + // #632: the range refusal maps to BadOutOfRange (see STRING above). + var dt = Convert.ToDateTime(value); + try { return global::S7.Net.Types.DateTime.ToByteArray(dt); } + catch (ArgumentOutOfRangeException ex) { throw new S7ValueRangeException($"S7 DATE_AND_TIME value {dt:O} is outside the DT range (1990–2089): {ex.Message}"); } // Writes refuse (S7ValueRangeException → BadOutOfRange) rather than truncate or clamp. case S7DataType.WString: return S7ValueCodecs.EncodeWString(value, tag.StringLength); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs index 583d658e..83b911e8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs @@ -299,7 +299,7 @@ public sealed class S7ScalarBlockTests { var tag = Tag(S7DataType.String, stringLength: 5); // 6 chars into a 5-char reserved field — S7.Net throws ArgumentException. - Should.Throw(() => S7Driver.EncodeScalarBlock(tag, "ABCDEF")); + Should.Throw(() => S7Driver.EncodeScalarBlock(tag, "ABCDEF")); } // ── String round-trip identity (encode → decode) ────────────────────────────────────── @@ -376,7 +376,7 @@ public sealed class S7ScalarBlockTests { var tag = Tag(S7DataType.DateTime); var value = new System.DateTime(year, 1, 1, 0, 0, 0); - Should.Throw(() => S7Driver.EncodeScalarBlock(tag, value)); + Should.Throw(() => S7Driver.EncodeScalarBlock(tag, value)); } // ── DateTime round-trip identity (encode → decode) ──────────────────────────────────── diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs index bb285f52..506d5d67 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs @@ -407,6 +407,35 @@ public sealed class S7ValueCodecTests drv.GetHealth().State.ShouldBe(DriverState.Healthy); } + /// + /// #632 live-coverage finding: classic STRING and DT writes use S7.Net's own encoders, which throw + /// / for an unrepresentable + /// value. That fell into the generic catch (BadCommunicationError + Degraded health) while the + /// sibling WSTRING/DATE codecs return BadOutOfRange — a client error must not look like a comms fault. + /// + [Fact] + public async Task Classic_STRING_and_DT_unrepresentable_writes_are_BadOutOfRange_without_degrading_health() + { + var plc = new MemoryS7Plc(); + var opts = new S7DriverOptions + { + Host = "192.0.2.1", + Probe = new S7ProbeOptions { Enabled = false }, + RawTags = S7RawTags.Entries( + new S7TagDefinition("Str", "DB1.DBB0", S7DataType.String, StringLength: 4), + new S7TagDefinition("Dt", "DB1.DBB20", S7DataType.DateTime)), + }; + using var drv = new S7Driver(opts, "s7-classic-range", new MemoryS7PlcFactory(plc)); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + var ct = TestContext.Current.CancellationToken; + + (await drv.WriteAsync([new WriteRequest("Str", "abcd")], ct))[0].StatusCode.ShouldBe(0u); + (await drv.WriteAsync([new WriteRequest("Str", "abcde")], ct))[0].StatusCode.ShouldBe(0x803C0000u); + (await drv.WriteAsync([new WriteRequest("Dt", new DateTime(1980, 1, 1))], ct))[0].StatusCode.ShouldBe(0x803C0000u); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + (await drv.ReadAsync(["Str"], ct))[0].Value.ShouldBe("abcd"); + } + // ---- fakes ---- private sealed class MemoryS7PlcFactory(MemoryS7Plc plc) : IS7PlcFactory