From 7c43d9a4ce5721f418f9ce128b789b0484565a73 Mon Sep 17 00:00:00 2001 From: Joshua Vlantis Date: Tue, 12 May 2026 12:19:40 +0200 Subject: [PATCH] fix: validate byte array length in RegisterFunctions converters (#38) ReadIntHoldingRegisters, ReadUintHoldingRegisters and ReadFloatHoldingRegisters on ModbusMasterEnhanced crashed with a confusing "ArgumentOutOfRangeException: startIndex" thrown from BitConverter when the configured wordSize was smaller than the target numeric type (e.g. wordSize=16 for a 32-bit int read). Add an explicit length check to ByteValueArraysToInts, ByteValueArraysToUInts and ByteValueArraysToFloats so callers see an actionable ArgumentException naming the parameter, the offending element index, and the minimum wordSize required for that read. Also multi-target NModbus.UnitTests at net6.0 and net8.0 so the test suite can run on Linux .NET runtimes alongside the existing net4.6 target. Tests: * ReadIntHoldingRegisters_WordSize16_ThrowsClearArgumentException * ReadUintHoldingRegisters_WordSize16_ThrowsClearArgumentException * ReadFloatHoldingRegisters_WordSize16_ThrowsClearArgumentException * ReadShortHoldingRegisters_WordSize16_StillWorks * ReadIntHoldingRegisters_WordSize32_StillWorks * ByteValueArraysToInts_FrontPaddingWithSmallArray_ThrowsClearArgumentException --- .../Extensions/ModbusMasterEnhancedFixture.cs | 100 ++++++++++++++++++ NModbus.UnitTests/NModbus.UnitTests.csproj | 3 +- .../Extensions/Functions/RegisterFunctions.cs | 23 ++++ 3 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 NModbus.UnitTests/Extensions/ModbusMasterEnhancedFixture.cs diff --git a/NModbus.UnitTests/Extensions/ModbusMasterEnhancedFixture.cs b/NModbus.UnitTests/Extensions/ModbusMasterEnhancedFixture.cs new file mode 100644 index 0000000..96d96fc --- /dev/null +++ b/NModbus.UnitTests/Extensions/ModbusMasterEnhancedFixture.cs @@ -0,0 +1,100 @@ +using System; +using Moq; +using NModbus.Extensions; +using NModbus.Extensions.Functions; +using Xunit; + +namespace NModbus.UnitTests.Extensions +{ + /// + /// Regression coverage for issue #38: + /// (and the other Read*HoldingRegisters helpers) crashed with a confusing + /// "Destination array is not long enough" exception when the configured + /// wordSize was smaller than the requested numeric type. + /// + public class ModbusMasterEnhancedFixture + { + private static Mock BuildMaster(ushort[] response) + { + var master = new Mock(MockBehavior.Strict); + master + .Setup(m => m.ReadHoldingRegisters(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(response); + return master; + } + + [Fact] + public void ReadIntHoldingRegisters_WordSize16_ThrowsClearArgumentException() + { + // Arrange: wordSize=16 means a "value" is a single 16-bit register, + // which is too small to hold a 32-bit int. The library should reject + // the call with an actionable error rather than crashing inside + // BitConverter with "Destination array is not long enough" (issue #38). + var master = BuildMaster(new ushort[] { 0x1234 }); + var enhanced = new ModbusMasterEnhanced(master.Object, wordSize: 16); + + // Act + Assert + var ex = Assert.Throws(() => enhanced.ReadIntHoldingRegisters(1, 10, 1)); + Assert.Contains("wordSize", ex.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("32", ex.Message); + } + + [Fact] + public void ReadUintHoldingRegisters_WordSize16_ThrowsClearArgumentException() + { + var master = BuildMaster(new ushort[] { 0x1234 }); + var enhanced = new ModbusMasterEnhanced(master.Object, wordSize: 16); + + var ex = Assert.Throws(() => enhanced.ReadUintHoldingRegisters(1, 10, 1)); + Assert.Contains("wordSize", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void ReadFloatHoldingRegisters_WordSize16_ThrowsClearArgumentException() + { + var master = BuildMaster(new ushort[] { 0x1234 }); + var enhanced = new ModbusMasterEnhanced(master.Object, wordSize: 16); + + var ex = Assert.Throws(() => enhanced.ReadFloatHoldingRegisters(1, 10, 1)); + Assert.Contains("wordSize", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void ReadShortHoldingRegisters_WordSize16_StillWorks() + { + // 16-bit values fit in a 16-bit word, so this should succeed and not regress. + var master = BuildMaster(new ushort[] { 0x1234 }); + var enhanced = new ModbusMasterEnhanced(master.Object, wordSize: 16); + + short[] result = enhanced.ReadShortHoldingRegisters(1, 10, 1); + Assert.Single(result); + Assert.Equal(unchecked((short)0x1234), result[0]); + } + + [Fact] + public void ReadIntHoldingRegisters_WordSize32_StillWorks() + { + // Sanity check that the wordSize=32 path is unchanged by the validation guard. + // We don't pin the exact byte order here, that's already covered by the + // existing extension tests and depends on host endianness. + var master = BuildMaster(new ushort[] { 0x1234, 0x5678 }); + var enhanced = new ModbusMasterEnhanced(master.Object, wordSize: 32); + + int[] result = enhanced.ReadIntHoldingRegisters(1, 10, 1); + Assert.Single(result); + } + + [Fact] + public void ByteValueArraysToInts_FrontPaddingWithSmallArray_ThrowsClearArgumentException() + { + // Direct coverage on the low-level helper: passing 2-byte arrays + // would previously throw ArgumentOutOfRangeException from BitConverter + // with "index" name and a confusing "Destination array is not long enough" + // chain. We now throw ArgumentException up front with a clearer message. + byte[][] tooSmall = { new byte[] { 0x12, 0x34 } }; + + var ex = Assert.Throws(() => RegisterFunctions.ByteValueArraysToInts(tooSmall)); + Assert.Contains("4 bytes", ex.Message); + } + } +} diff --git a/NModbus.UnitTests/NModbus.UnitTests.csproj b/NModbus.UnitTests/NModbus.UnitTests.csproj index 77fb2fa..223ec4f 100644 --- a/NModbus.UnitTests/NModbus.UnitTests.csproj +++ b/NModbus.UnitTests/NModbus.UnitTests.csproj @@ -1,12 +1,11 @@ - net4.6 + net4.6;net6.0;net8.0 true NModbus.UnitTests NModbus.UnitTests true - 1.0.4 false diff --git a/NModbus/Extensions/Functions/RegisterFunctions.cs b/NModbus/Extensions/Functions/RegisterFunctions.cs index aa71f58..1c19f6a 100644 --- a/NModbus/Extensions/Functions/RegisterFunctions.cs +++ b/NModbus/Extensions/Functions/RegisterFunctions.cs @@ -67,6 +67,7 @@ public static ushort[] ByteValueArraysToUShorts(byte[][] data, bool frontPadding public static int[] ByteValueArraysToInts(byte[][] data, bool frontPadding = true) { + RequireMinimumByteLength(data, 4, nameof(ByteValueArraysToInts)); return frontPadding ? data.Select(e => BitConverter.ToInt32(e, e.Length - 4)).ToArray() : data.Select(e => BitConverter.ToInt32(e, 0)).ToArray(); @@ -74,6 +75,7 @@ public static int[] ByteValueArraysToInts(byte[][] data, bool frontPadding = tru public static uint[] ByteValueArraysToUInts(byte[][] data, bool frontPadding = true) { + RequireMinimumByteLength(data, 4, nameof(ByteValueArraysToUInts)); return frontPadding ? data.Select(e => BitConverter.ToUInt32(e, e.Length - 4)).ToArray() : data.Select(e => BitConverter.ToUInt32(e, 0)).ToArray(); @@ -81,11 +83,32 @@ public static uint[] ByteValueArraysToUInts(byte[][] data, bool frontPadding = t public static float[] ByteValueArraysToFloats(byte[][] data, bool frontPadding = true) { + RequireMinimumByteLength(data, 4, nameof(ByteValueArraysToFloats)); return frontPadding ? data.Select(e => BitConverter.ToSingle(e, e.Length - 4)).ToArray() : data.Select(e => BitConverter.ToSingle(e, 0)).ToArray(); } + private static void RequireMinimumByteLength(byte[][] data, int minBytes, string operation) + { + // Issue #38: previously, supplying byte arrays narrower than the target + // numeric type leaked through to BitConverter and raised an opaque + // ArgumentOutOfRangeException about "startIndex". Validate up front so + // callers configuring ModbusMasterEnhanced with too small a wordSize + // see an actionable message instead. + if (data == null) throw new ArgumentNullException(nameof(data)); + for (int i = 0; i < data.Length; i++) + { + if (data[i] == null || data[i].Length < minBytes) + { + throw new ArgumentException( + $"{operation} requires each byte array to be at least {minBytes} bytes; element {i} has {data[i]?.Length ?? 0}. " + + "When using ModbusMasterEnhanced, set wordSize to at least the size of the value being read (e.g. 32 for int/uint/float).", + nameof(data)); + } + } + } + public static byte[][] CharsToByteValueArrays(char[] data, uint wordSize, bool frontPadding = true, bool singleCharPerRegister = true) {