diff --git a/S7.Net.UnitTest/TypeTests/StringExTests.cs b/S7.Net.UnitTest/TypeTests/StringExTests.cs index 164ba4f..aecf905 100644 --- a/S7.Net.UnitTest/TypeTests/StringExTests.cs +++ b/S7.Net.UnitTest/TypeTests/StringExTests.cs @@ -1,5 +1,8 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using S7.Net.Types; +using System; +using System.Collections; +using System.Linq; namespace S7.Net.UnitTest.TypeTests { @@ -24,6 +27,24 @@ namespace S7.Net.UnitTest.TypeTests AssertFromByteArrayEquals("", 1, 0, (byte) 'a'); } + [TestMethod] + public void ReadMalformedStringTooShort() + { + Assert.ThrowsException(() => AssertFromByteArrayEquals("", 1)); + } + + [TestMethod] + public void ReadMalformedStringSizeLargerThanCapacity() + { + Assert.ThrowsException(() => StringEx.FromByteArray(new byte[] { 3, 5, 0, 1, 2 })); + } + + [TestMethod] + public void ReadMalformedStringCapacityTooLarge() + { + Assert.ThrowsException(() => AssertToByteArrayAndBackEquals("", 300, 0)); + } + [TestMethod] public void ReadA() { @@ -33,83 +54,82 @@ namespace S7.Net.UnitTest.TypeTests [TestMethod] public void ReadAbc() { - AssertFromByteArrayEquals("Abc", 1, 3, (byte) 'A', (byte) 'b', (byte) 'c'); + AssertFromByteArrayEquals("Abc", 3, 3, (byte) 'A', (byte) 'b', (byte) 'c'); } [TestMethod] public void WriteNullWithReservedLengthZero() { - AssertToByteArrayEquals(null, 0, 0, 0); + Assert.ThrowsException(() => AssertToByteArrayAndBackEquals(null, 0, 0, 0)); } [TestMethod] public void WriteEmptyStringWithReservedLengthZero() { - AssertToByteArrayEquals("", 0, 0, 0); + AssertToByteArrayAndBackEquals("", 0, 0, 0); } [TestMethod] public void WriteAWithReservedLengthZero() { - AssertToByteArrayEquals("A", 0, 0, 0); + AssertToByteArrayAndBackEquals("", 0, 0, 0); } [TestMethod] public void WriteNullWithReservedLengthOne() { - AssertToByteArrayEquals(null, 1, 1, 0); + Assert.ThrowsException(() => AssertToByteArrayAndBackEquals(null, 1, 1, 0)); } [TestMethod] public void WriteEmptyStringWithReservedLengthOne() { - AssertToByteArrayEquals("", 1, 1, 0); + AssertToByteArrayAndBackEquals("", 1, 1, 0, 0); } [TestMethod] public void WriteAWithReservedLengthOne() { - AssertToByteArrayEquals("A", 1, 1, 1, (byte) 'A'); + AssertToByteArrayAndBackEquals("A", 1, 1, 1, (byte) 'A'); } [TestMethod] public void WriteAWithReservedLengthTwo() { - AssertToByteArrayEquals("A", 2, 2, 1, (byte) 'A'); + AssertToByteArrayAndBackEquals("A", 2, 2, 1, (byte) 'A', 0); } [TestMethod] - public void WriteAbcWithReservedLengthOne() + public void WriteAbcWithStringLargetThanReservedLength() { - AssertToByteArrayEquals("Abc", 1, 1, 1, (byte) 'A'); - } - - [TestMethod] - public void WriteAbcWithReservedLengthTwo() - { - AssertToByteArrayEquals("Abc", 2, 2, 2, (byte) 'A', (byte) 'b'); + Assert.ThrowsException(() => StringEx.ToByteArray("Abc", 2)); } [TestMethod] public void WriteAbcWithReservedLengthThree() { - AssertToByteArrayEquals("Abc", 3, 3, 3, (byte) 'A', (byte) 'b', (byte) 'c'); + AssertToByteArrayAndBackEquals("Abc", 3, 3, 3, (byte) 'A', (byte) 'b', (byte) 'c'); } [TestMethod] public void WriteAbcWithReservedLengthFour() { - AssertToByteArrayEquals("Abc", 4, 4, 3, (byte) 'A', (byte) 'b', (byte) 'c'); + AssertToByteArrayAndBackEquals("Abc", 4, 4, 3, (byte) 'A', (byte) 'b', (byte) 'c', 0); } private static void AssertFromByteArrayEquals(string expected, params byte[] bytes) { - Assert.AreEqual(expected, StringEx.FromByteArray(bytes)); + var convertedString = StringEx.FromByteArray(bytes); + Assert.AreEqual(expected, convertedString); } - private static void AssertToByteArrayEquals(string value, int reservedLength, params byte[] expected) + + private static void AssertToByteArrayAndBackEquals(string value, int reservedLength, params byte[] expected) { - CollectionAssert.AreEqual(expected, StringEx.ToByteArray(value, reservedLength)); + var convertedData = StringEx.ToByteArray(value, reservedLength); + CollectionAssert.AreEqual(expected, convertedData); + var convertedBack = StringEx.FromByteArray(convertedData); + Assert.AreEqual(value, convertedBack); } } } diff --git a/S7.Net/PlcAsynchronous.cs b/S7.Net/PlcAsynchronous.cs index c6d7388..2f481e4 100644 --- a/S7.Net/PlcAsynchronous.cs +++ b/S7.Net/PlcAsynchronous.cs @@ -324,9 +324,9 @@ namespace S7.Net if (bitAdr != -1) { //Must be writing a bit value as bitAdr is specified - if (value is bool) + if (value is bool boolean) { - await WriteBitAsync(dataType, db, startByteAdr, bitAdr, (bool) value); + await WriteBitAsync(dataType, db, startByteAdr, bitAdr, boolean); } else if (value is int intValue) { diff --git a/S7.Net/PlcSynchronous.cs b/S7.Net/PlcSynchronous.cs index bf0a1a3..8cf5f89 100644 --- a/S7.Net/PlcSynchronous.cs +++ b/S7.Net/PlcSynchronous.cs @@ -289,9 +289,9 @@ namespace S7.Net if (bitAdr != -1) { //Must be writing a bit value as bitAdr is specified - if (value is bool) + if (value is bool boolean) { - WriteBit(dataType, db, startByteAdr, bitAdr, (bool) value); + WriteBit(dataType, db, startByteAdr, bitAdr, boolean); } else if (value is int intValue) { @@ -442,7 +442,6 @@ namespace S7.Net private byte[] BuildWriteBitPackage(DataType dataType, int db, int startByteAdr, bool bitValue, int bitAdr) { - var stream = GetStreamIfAvailable(); var value = new[] { bitValue ? (byte)1 : (byte)0 }; int varCount = 1; // first create the header diff --git a/S7.Net/Protocol/S7WriteMultiple.cs b/S7.Net/Protocol/S7WriteMultiple.cs index 2738a34..853e1e5 100644 --- a/S7.Net/Protocol/S7WriteMultiple.cs +++ b/S7.Net/Protocol/S7WriteMultiple.cs @@ -17,7 +17,6 @@ namespace S7.Net.Protocol (ushort) (2 + paramSize)); var paramOffset = Header.Template.Length; - var dataOffset = paramOffset + paramSize; var data = new ByteArray(); var itemCount = 0; diff --git a/S7.Net/Protocol/Serialization.cs b/S7.Net/Protocol/Serialization.cs index 01d1583..b9ac9a5 100644 --- a/S7.Net/Protocol/Serialization.cs +++ b/S7.Net/Protocol/Serialization.cs @@ -79,9 +79,9 @@ namespace S7.Net.Protocol { var start = startByte * 8 + bitNumber; buffer[index + 2] = (byte)start; - start = start >> 8; + start >>= 8; buffer[index + 1] = (byte)start; - start = start >> 8; + start >>= 8; buffer[index] = (byte)start; } diff --git a/S7.Net/Types/StringEx.cs b/S7.Net/Types/StringEx.cs index 81733ca..5f152b2 100644 --- a/S7.Net/Types/StringEx.cs +++ b/S7.Net/Types/StringEx.cs @@ -17,10 +17,17 @@ namespace S7.Net.Types /// public static string FromByteArray(byte[] bytes) { - if (bytes.Length < 2) return ""; + if (bytes.Length < 2) + { + throw new PlcException(ErrorCode.ReadData, "Malformed S7 String / too short"); + } int size = bytes[0]; int length = bytes[1]; + if (length > size) + { + throw new PlcException(ErrorCode.ReadData, "Malformed S7 String / length larger than capacity"); + } try { @@ -43,18 +50,21 @@ namespace S7.Net.Types /// A containing the string header and string value with a maximum length of + 2. public static byte[] ToByteArray(string value, int reservedLength) { + if (value is null) + { + throw new ArgumentNullException(nameof(value)); + } + if (reservedLength > byte.MaxValue) throw new ArgumentException($"The maximum string length supported is {byte.MaxValue}."); - var length = value?.Length; - if (length > reservedLength) length = reservedLength; + var bytes = Encoding.ASCII.GetBytes(value); + if (bytes.Length > reservedLength) throw new ArgumentException($"The provided string length ({bytes.Length} is larger than the specified reserved length ({reservedLength})."); - var bytes = new byte[(length ?? 0) + 2]; - bytes[0] = (byte) reservedLength; - - if (value == null) return bytes; - - bytes[1] = (byte) Encoding.ASCII.GetBytes(value, 0, length.Value, bytes, 2); - return bytes; + var buffer = new byte[2 + reservedLength]; + Array.Copy(bytes, 0, buffer, 2, bytes.Length); + buffer[0] = (byte)reservedLength; + buffer[1] = (byte)bytes.Length; + return buffer; } } }