From 41a926a9290e2a608cf5c77b27b1790841938894 Mon Sep 17 00:00:00 2001 From: Gerald Pohn Date: Fri, 11 Sep 2026 11:31:33 +0200 Subject: [PATCH 1/4] Fix thread-unsafe LinkedList access in OpenSSLConnector pending buffer lists (GML-1187) --- .../OpenSSL/OpenSSLConnector.cs | 60 +++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs index da21d9a..5f513a4 100644 --- a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs +++ b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs @@ -33,6 +33,8 @@ public class OpenSSLConnector : IDisposable private readonly byte[] m_buffer = new byte[4096]; private readonly DataBufferList m_pendingWriteList; private readonly DataBufferList m_pendingReadList; + private readonly object m_readListLock = new object(); + private readonly object m_writeListLock = new object(); private bool m_fatalError; private bool m_disposed; @@ -277,8 +279,16 @@ protected void RunSSL() while (!m_fatalError && ((!m_readRequired && dataToWrite) || dataToRead)) { - int pendingReadBefore = m_pendingReadList.Count; - int pendingWriteBefore = m_pendingWriteList.Count; + int pendingReadBefore; + int pendingWriteBefore; + lock (m_readListLock) + { + pendingReadBefore = m_pendingReadList.Count; + } + lock (m_writeListLock) + { + pendingWriteBefore = m_pendingWriteList.Count; + } bool readRequiredBefore = m_readRequired; var bioPendingBefore = Native.BIO_ctrl_pending(m_pBioOut); @@ -304,8 +314,18 @@ protected void RunSSL() GetPendingOperations(ref dataToRead, ref dataToWrite); var bioPendingAfter = Native.BIO_ctrl_pending(m_pBioOut); - if (pendingReadBefore == m_pendingReadList.Count && - pendingWriteBefore == m_pendingWriteList.Count && + int pendingReadAfter; + int pendingWriteAfter; + lock (m_readListLock) + { + pendingReadAfter = m_pendingReadList.Count; + } + lock (m_writeListLock) + { + pendingWriteAfter = m_pendingWriteList.Count; + } + if (pendingReadBefore == pendingReadAfter && + pendingWriteBefore == pendingWriteAfter && readRequiredBefore == m_readRequired && bioPendingBefore == bioPendingAfter) { @@ -341,8 +361,14 @@ public void ReadCompleted(byte[] pData, int dataLen) private void GetPendingOperations(ref bool dataToRead, ref bool dataToWrite) { - dataToRead = m_pendingReadList.Count > 0; - dataToWrite = m_pendingWriteList.Count > 0; + lock (m_readListLock) + { + dataToRead = m_pendingReadList.Count > 0; + } + lock (m_writeListLock) + { + dataToWrite = m_pendingWriteList.Count > 0; + } } private void PerformRead() @@ -437,23 +463,35 @@ private void UseData(DataBufferList list, DataBuffer pBuffer, int result) private void AppendBuffer(DataBufferList list, DataBuffer pBuffer) { - list.AddLast(pBuffer); + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) + { + list.AddLast(pBuffer); + } } DataBuffer GetNextBuffer(DataBufferList list) { DataBuffer head = null; - if (list.Count > 0) + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) { - head = list.First.Value; - list.RemoveFirst(); + if (list.Count > 0) + { + head = list.First.Value; + list.RemoveFirst(); + } } return head; } void PutbackBuffer(DataBufferList list, DataBuffer pBuffer) { - list.AddFirst(pBuffer); + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) + { + list.AddFirst(pBuffer); + } } /// From f65c8d93f9a5e70033d8df05feb493a1af9c5865 Mon Sep 17 00:00:00 2001 From: Gerald Pohn Date: Thu, 10 Sep 2026 12:53:53 +0200 Subject: [PATCH 2/4] Add PlcTag.GetValue/SetValue and POffsetInfoType.MaxStringLength Port net functional changes from fork package 01 (DataTypes extension / string array fix, commits 6aef271/519cdeb/ffe8daa): - PlcTag: add virtual GetValue()/SetValue(object) reflecting the concrete tag's public Value property, for generic tag value access. - POffsetInfoType: add virtual MaxStringLength(), returning 0 by default, UnspecifiedOffsetinfo1 for Array1Dim (string array max length), and Min(UnspecifiedOffsetinfo1, UnspecifiedOffsetinfo2) for String (fixes cases where firmware swaps both fields). All other changes from this fork package are already superseded by the target repo's own PlcTag/PlcTags implementation and are documented as excluded in package 16. --- src/S7CommPlusDriver/ClientApi/PlcTag.cs | 20 ++++++++++++++++++++ src/S7CommPlusDriver/Core/POffsetInfoType.cs | 10 ++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/S7CommPlusDriver/ClientApi/PlcTag.cs b/src/S7CommPlusDriver/ClientApi/PlcTag.cs index 7cdef54..c40b5c0 100644 --- a/src/S7CommPlusDriver/ClientApi/PlcTag.cs +++ b/src/S7CommPlusDriver/ClientApi/PlcTag.cs @@ -64,6 +64,26 @@ public virtual void ProcessWriteResult(UInt64 error) internal abstract PValue GetWriteValue(); + /// + /// Gets the tag's current typed value as a boxed via its public Value property. + /// + /// The boxed value, or when the concrete tag type has no public Value property. + public virtual object GetValue() + { + var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance); + return valueProperty?.GetValue(this); + } + + /// + /// Sets the tag's current typed value from a boxed via its public Value property. + /// + /// The value to assign, which must be assignable to the concrete tag's Value property type. + public virtual void SetValue(object value) + { + var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance); + valueProperty?.SetValue(this, value); + } + internal void SetTraceAddressMetadata(VarInfo variable) { if (variable == null) diff --git a/src/S7CommPlusDriver/Core/POffsetInfoType.cs b/src/S7CommPlusDriver/Core/POffsetInfoType.cs index 1972a65..3eefd3f 100644 --- a/src/S7CommPlusDriver/Core/POffsetInfoType.cs +++ b/src/S7CommPlusDriver/Core/POffsetInfoType.cs @@ -69,6 +69,12 @@ public enum OffsetInfoType public abstract bool Is1Dim(); public abstract bool IsMDim(); + /// + /// Gets the maximum string length for a String/WString offset info, or 0 when not applicable. + /// + /// The maximum length in characters, or 0 for non-string offset info types. + public virtual int MaxStringLength() { return 0; } + public static POffsetInfoType Deserialize(Stream buffer, int offsetinfotype, out int length) { switch ((OffsetInfoType)offsetinfotype) @@ -542,6 +548,8 @@ public class POffsetInfoType_Array1Dim : POffsetInfoType, IOffsetInfoType_1Dim public override bool HasRelation() { return false; } public override bool Is1Dim() { return true; } public override bool IsMDim() { return false; } + // For a string array, UnspecifiedOffsetinfo1 carries the maximum string length of the array elements. + public override int MaxStringLength() { return UnspecifiedOffsetinfo1; } public static POffsetInfoType_Array1Dim Deserialize(Stream buffer, out int length) { @@ -599,6 +607,8 @@ public class POffsetInfoType_String: POffsetInfoType public override bool HasRelation() { return false; } public override bool Is1Dim() { return false; } public override bool IsMDim() { return false; } + // Some firmware versions swap UnspecifiedOffsetinfo1/2, so take the smaller value as the max. string length. + public override int MaxStringLength() { return Math.Min(UnspecifiedOffsetinfo1, UnspecifiedOffsetinfo2); } public static POffsetInfoType_String Deserialize(Stream buffer, out int length) { From 46c1e577cbb22b82ea8586c0a551ecde69b3d9f9 Mon Sep 17 00:00:00 2001 From: Gerald Pohn Date: Mon, 14 Sep 2026 09:22:12 +0200 Subject: [PATCH 3/4] Fix TagFactory not forwarding declared PLC string length, causing STRING/WSTRING writes to fail --- .../PlcTagStringMaxLengthTests.cs | 91 +++++++++++++++++++ src/S7CommPlusDriver/ClientApi/Browser.cs | 7 ++ src/S7CommPlusDriver/ClientApi/PlcTag.cs | 13 +++ src/S7CommPlusDriver/ClientApi/PlcTags.cs | 27 +++++- .../S7CommPlusProtocolSession.cs | 15 ++- .../S7CommPlusTagAccessorCatalog.cs | 31 +++++-- 6 files changed, 166 insertions(+), 18 deletions(-) create mode 100644 src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs diff --git a/src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs b/src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs new file mode 100644 index 0000000..42921b2 --- /dev/null +++ b/src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs @@ -0,0 +1,91 @@ +using S7CommPlusDriver.ClientApi; +using Xunit; + +namespace S7CommPlusDriver.Tests +{ + /// + /// Verifies that forwards the PLC-declared maximum string length to + /// , , and , and that the resulting + /// write telegram matches the declared length instead of always falling back to the built-in default of 254. + /// + /// + /// Regression test for a bug where TagFactory never received the declared maximum string length, so every + /// String/WString tag was created with the built-in default of 254, regardless of its actual PLC + /// declaration (e.g. String[10]). Writing such a tag then sent a telegram of the wrong size and with the wrong + /// declared-length header byte, which the PLC rejected. + /// + public sealed class PlcTagStringMaxLengthTests + { + [Fact] + public void TagFactoryAppliesDeclaredMaxStringLengthToScalarStringTag() + { + var tag = PlcTags.TagFactory( + "DB.Text", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_STRING, + Is1Dim: false, + maxStringLength: 10); + + var stringTag = Assert.IsType(tag); + stringTag.Value = "0123456789"; // exactly 10 characters, must not throw + + var writeValue = Assert.IsType(GetWriteValue(stringTag)); + var bytes = writeValue.GetValue(); + + Assert.Equal(12, bytes.Length); // declared max length (10) + 2 header bytes + Assert.Equal(10, bytes[0]); // declared max length, not the built-in default of 254 + Assert.Equal(10, bytes[1]); // actual length + } + + [Fact] + public void TagFactoryFallsBackToDefaultMaxStringLengthWhenNotSupplied() + { + var tag = PlcTags.TagFactory( + "DB.Text", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_STRING); + + var stringTag = Assert.IsType(tag); + stringTag.Value = "Hello"; + + var writeValue = Assert.IsType(GetWriteValue(stringTag)); + var bytes = writeValue.GetValue(); + + Assert.Equal(256, bytes.Length); // built-in default of 254 + 2 header bytes + Assert.Equal(254, bytes[0]); + } + + [Fact] + public void TagFactoryAppliesDeclaredMaxStringLengthToStringArrayElementTag() + { + var tag = PlcTags.TagFactory( + "DB.Texts", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_STRING, + Is1Dim: true, + maxStringLength: 5); + + Assert.IsType(tag); + Assert.Equal(5, tag.GetMaxStringLength()); + } + + [Fact] + public void TagFactoryAppliesDeclaredMaxStringLengthToWStringTag() + { + var tag = PlcTags.TagFactory( + "DB.UnicodeText", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_WSTRING, + Is1Dim: false, + maxStringLength: 20); + + var wstringTag = Assert.IsType(tag); + Assert.Equal(20, wstringTag.GetMaxStringLength()); + } + + private static PValue GetWriteValue(PlcTag tag) + { + return tag.GetWriteValue(); + } + } +} diff --git a/src/S7CommPlusDriver/ClientApi/Browser.cs b/src/S7CommPlusDriver/ClientApi/Browser.cs index 8fc194e..d111bbd 100644 --- a/src/S7CommPlusDriver/ClientApi/Browser.cs +++ b/src/S7CommPlusDriver/ClientApi/Browser.cs @@ -174,6 +174,7 @@ private void AddFlatSubnodes( ArrayElementCount = GetArrayElementCount(node), ArrayDimensions = GetArrayDimensions(node), ContainsIndexedArray = containsIndexedArray, + MaxStringLength = node.Vte?.OffsetInfoType?.MaxStringLength() ?? 0, }; // If an Array element of basic datatype, the Vte is here from the parent array base element and offsets not valid here. if (node.NodeType == eNodeType.Array) @@ -940,6 +941,12 @@ public class VarInfo /// public IReadOnlyList ArrayDimensions = Array.Empty(); + /// + /// Gets the maximum string length declared in the PLC for String/WString values, or zero for every + /// other datatype. Required to build a correctly sized write telegram; see . + /// + public int MaxStringLength; + /// Retains the parsed symbol path so tag creation can reuse the exact CRC inputs. internal List SymbolCrcPath; diff --git a/src/S7CommPlusDriver/ClientApi/PlcTag.cs b/src/S7CommPlusDriver/ClientApi/PlcTag.cs index c40b5c0..790d148 100644 --- a/src/S7CommPlusDriver/ClientApi/PlcTag.cs +++ b/src/S7CommPlusDriver/ClientApi/PlcTag.cs @@ -64,6 +64,13 @@ public virtual void ProcessWriteResult(UInt64 error) internal abstract PValue GetWriteValue(); + /// + /// Gets the PLC-declared maximum string length for , , and + /// tags, or zero for every other tag type. Used to persist and recreate the exact + /// wire length required by (see ). + /// + internal virtual int GetMaxStringLength() => 0; + /// /// Gets the tag's current typed value as a boxed via its public Value property. /// @@ -1001,6 +1008,8 @@ public PlcTagString(string name, ItemAddress address, uint softdatatype, byte ma m_MaxLength = maxlength; } + internal override int GetMaxStringLength() => m_MaxLength; + public override void ProcessReadResult(object valueObj, ulong error) { LastReadError = error; @@ -1503,6 +1512,8 @@ public PlcTagWString(string name, ItemAddress address, uint softdatatype, ushort m_MaxLength = maxlength; } + internal override int GetMaxStringLength() => m_MaxLength; + public override void ProcessReadResult(object valueObj, ulong error) { LastReadError = error; @@ -2639,6 +2650,8 @@ public PlcTagStringArray(string name, ItemAddress address, uint softdatatype, by m_MaxLength = maxlength; } + internal override int GetMaxStringLength() => m_MaxLength; + public override void ProcessReadResult(object valueObj, ulong error) { LastReadError = error; diff --git a/src/S7CommPlusDriver/ClientApi/PlcTags.cs b/src/S7CommPlusDriver/ClientApi/PlcTags.cs index 09ad4e7..afd20a1 100644 --- a/src/S7CommPlusDriver/ClientApi/PlcTags.cs +++ b/src/S7CommPlusDriver/ClientApi/PlcTags.cs @@ -78,8 +78,13 @@ public static int WriteTags(this S7CommPlusProtocolSession conn, IEnumerable + /// + /// The PLC-declared maximum length for values, or zero when unknown. + /// Ignored for every other datatype. A value of zero falls back to the tag's built-in default of 254, matching the previous + /// behavior for callers that cannot supply this metadata. + /// /// The matching concrete tag implementation, or for an unsupported datatype. - public static PlcTag TagFactory(string name, ItemAddress address, uint softdatatype, bool Is1Dim = false) + public static PlcTag TagFactory(string name, ItemAddress address, uint softdatatype, bool Is1Dim = false, int maxStringLength = 0) { switch (softdatatype) { @@ -127,9 +132,15 @@ public static PlcTag TagFactory(string name, ItemAddress address, uint softdatat return new PlcTagDateAndTime(name, address, softdatatype); case Softdatatype.S7COMMP_SOFTDATATYPE_STRING: - if (Is1Dim) - return new PlcTagStringArray(name, address, softdatatype); - return new PlcTagString(name, address, softdatatype); + { + // A value of 0 (unknown/not supplied by the caller) keeps the tag's own built-in default (254). + var stringMaxLength = maxStringLength > 0 && maxStringLength <= byte.MaxValue + ? (byte)maxStringLength + : (byte)254; + if (Is1Dim) + return new PlcTagStringArray(name, address, softdatatype, stringMaxLength); + return new PlcTagString(name, address, softdatatype, stringMaxLength); + } case Softdatatype.S7COMMP_SOFTDATATYPE_POINTER: return new PlcTagPointer(name, address, softdatatype); @@ -178,7 +189,13 @@ public static PlcTag TagFactory(string name, ItemAddress address, uint softdatat case Softdatatype.S7COMMP_SOFTDATATYPE_WCHAR: return new PlcTagWChar(name, address, softdatatype); case Softdatatype.S7COMMP_SOFTDATATYPE_WSTRING: - return new PlcTagWString(name, address, softdatatype); + { + // A value of 0 (unknown/not supplied by the caller) keeps the tag's own built-in default (254). + var wstringMaxLength = maxStringLength > 0 && maxStringLength <= ushort.MaxValue + ? (ushort)maxStringLength + : (ushort)254; + return new PlcTagWString(name, address, softdatatype, wstringMaxLength); + } //case Softdatatype.S7COMMP_SOFTDATATYPE_VARIANT: //-> Variant isn't added inside of the instance-db as a variable! case Softdatatype.S7COMMP_SOFTDATATYPE_LTIME: diff --git a/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs b/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs index 58e8b11..e3891bf 100644 --- a/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs +++ b/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs @@ -2067,7 +2067,8 @@ internal static bool IsAggregatePrimitiveArray(PVartypeListElement varType, stri private static PlcTag CreateResolvedPlcTag(VarInfo varInfo, PVartypeListElement varType, bool isAggregateArray) { var address = CreateItemAddress(varInfo); - var tag = PlcTags.TagFactory(varInfo.Name, address, varType.Softdatatype, isAggregateArray); + var maxStringLength = varType.OffsetInfoType?.MaxStringLength() ?? 0; + var tag = PlcTags.TagFactory(varInfo.Name, address, varType.Softdatatype, isAggregateArray, maxStringLength); tag?.SetTraceAddressMetadata(varInfo); if (!isAggregateArray || tag == null) { @@ -2081,7 +2082,7 @@ private static PlcTag CreateResolvedPlcTag(VarInfo varInfo, PVartypeListElement varInfo.AccessSequence, accessId, varType.OffsetInfoType.HasRelation()); - return PlcTags.TagFactory($"{varInfo.Name}[#{accessId}]", elementAddress, varType.Softdatatype); + return PlcTags.TagFactory($"{varInfo.Name}[#{accessId}]", elementAddress, varType.Softdatatype, false, maxStringLength); }) .Where(elementTag => elementTag != null) .ToList(); @@ -2106,7 +2107,7 @@ internal static PlcTag CreateResolvedPlcTag(VarInfo varInfo) var address = CreateItemAddress(varInfo); var isAggregateArray = varInfo.ArrayElementCount > 0; - var tag = PlcTags.TagFactory(varInfo.Name, address, varInfo.Softdatatype, isAggregateArray); + var tag = PlcTags.TagFactory(varInfo.Name, address, varInfo.Softdatatype, isAggregateArray, varInfo.MaxStringLength); tag?.SetTraceAddressMetadata(varInfo); if (!isAggregateArray || tag == null) { @@ -2120,7 +2121,9 @@ internal static PlcTag CreateResolvedPlcTag(VarInfo varInfo) varInfo.AccessSequence, accessId, RequiresArrayElementRelationSelector(varInfo.Softdatatype)), - varInfo.Softdatatype)) + varInfo.Softdatatype, + false, + varInfo.MaxStringLength)) .Where(elementTag => elementTag != null) .ToList(); tag.SetAggregateElements(elementTags); @@ -2321,7 +2324,9 @@ internal static bool TryCreateResolvedPrimitiveArrayElement( aggregateVariable.AccessSequence, accessId, RequiresArrayElementRelationSelector(aggregateVariable.Softdatatype)), - aggregateVariable.Softdatatype); + aggregateVariable.Softdatatype, + false, + aggregateVariable.MaxStringLength); return tag != null; } diff --git a/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs b/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs index 7658b5f..4a4d310 100644 --- a/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs +++ b/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs @@ -19,7 +19,7 @@ namespace S7CommPlusDriver /// public sealed class S7CommPlusTagAccessorCatalog { - private const int CurrentFormatVersion = 2; + private const int CurrentFormatVersion = 3; private const int MinimumSupportedFormatVersion = 1; private const int MaximumEntryCount = 2_000_000; private const int MaximumAggregateElementCount = 2_000_000; @@ -238,7 +238,7 @@ public static S7CommPlusTagAccessorCatalog ReadFrom(Stream source, string expect entries.Add( symbol, reader.ReadBoolean() - ? TagDescriptor.ReadFrom(reader, 0, version >= 2 ? symbol : null) + ? TagDescriptor.ReadFrom(reader, 0, version, version >= 2 ? symbol : null) : null); } return new S7CommPlusTagAccessorCatalog(structureHash, entries); @@ -329,16 +329,21 @@ private sealed class TagDescriptor /// The Siemens soft-datatype identifier. /// The immutable item-address descriptor. /// Scalar elements used to transfer an aggregate array. + /// + /// The PLC-declared maximum string length for String/WString tags, or zero for every other datatype. + /// private TagDescriptor( string name, uint datatype, AddressDescriptor address, - IReadOnlyList aggregateElements) + IReadOnlyList aggregateElements, + int maxStringLength) { Name = name; Datatype = datatype; _address = address; _aggregateElements = aggregateElements; + MaxStringLength = maxStringLength; } /// Gets the symbolic or generated element name. @@ -347,6 +352,9 @@ private TagDescriptor( /// Gets the Siemens soft-datatype identifier. private uint Datatype { get; } + /// Gets the PLC-declared maximum string length, or zero for every non-string datatype. + private int MaxStringLength { get; } + /// Creates a detached immutable descriptor from a resolved mutable driver tag. /// The resolved source tag. /// A recursively captured descriptor. @@ -357,14 +365,15 @@ internal static TagDescriptor FromTag(PlcTag tag) tag.Name, tag.Datatype, AddressDescriptor.FromAddress(tag.Address), - tag.AggregateElements.Select(FromTag).ToArray()); + tag.AggregateElements.Select(FromTag).ToArray(), + tag.GetMaxStringLength()); } /// Creates a fresh mutable driver tag and recursively recreates aggregate element tags. /// An accessor that shares no mutable state with previous materializations. internal PlcTag CreateTag(string nameOverride = null) { - var tag = PlcTags.TagFactory(nameOverride ?? Name, _address.CreateAddress(), Datatype, _aggregateElements.Count > 0); + var tag = PlcTags.TagFactory(nameOverride ?? Name, _address.CreateAddress(), Datatype, _aggregateElements.Count > 0, MaxStringLength); if (tag == null) { throw new InvalidDataException($"The cached datatype {Datatype} for symbol '{Name}' is not supported by this driver version."); @@ -391,13 +400,18 @@ internal void WriteTo(BinaryWriter writer, bool includeName = true) { element.WriteTo(writer); } + writer.Write(MaxStringLength); } /// Reads and validates one recursive descriptor from the catalog stream. /// The source reader. /// The current nesting depth used to reject maliciously recursive cache files. + /// + /// The catalog format version being read. Versions below 3 () predate persisted + /// string-length metadata; such caches fall back to 0, which recreates the tag's built-in default (254). + /// /// The validated descriptor. - internal static TagDescriptor ReadFrom(BinaryReader reader, int depth, string nameOverride = null) + internal static TagDescriptor ReadFrom(BinaryReader reader, int depth, int formatVersion, string nameOverride = null) { if (depth > MaximumDescriptorDepth) { @@ -414,9 +428,10 @@ internal static TagDescriptor ReadFrom(BinaryReader reader, int depth, string na var elements = new TagDescriptor[elementCount]; for (var index = 0; index < elementCount; index++) { - elements[index] = ReadFrom(reader, depth + 1); + elements[index] = ReadFrom(reader, depth + 1, formatVersion); } - return new TagDescriptor(name, datatype, address, elements); + var maxStringLength = formatVersion >= 3 ? reader.ReadInt32() : 0; + return new TagDescriptor(name, datatype, address, elements, maxStringLength); } } From a66defb56e859fe282c1595215db66887c1aea8e Mon Sep 17 00:00:00 2001 From: Gerald Pohn Date: Mon, 14 Sep 2026 13:06:10 +0200 Subject: [PATCH 4/4] Reshape multidimensional aggregate array reads/writes into genuine multidimensional CLR arrays --- .../PlcTagMultiDimensionalArrayTests.cs | 268 ++++++++++++++++++ src/S7CommPlusDriver/ClientApi/PlcTag.cs | 140 ++++++++- .../S7CommPlusProtocolSession.cs | 27 +- 3 files changed, 421 insertions(+), 14 deletions(-) create mode 100644 src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs diff --git a/src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs b/src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs new file mode 100644 index 0000000..16896a8 --- /dev/null +++ b/src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs @@ -0,0 +1,268 @@ +using System; +using System.Linq; +using S7CommPlusDriver.ClientApi; +using Xunit; + +namespace S7CommPlusDriver.Tests +{ + /// + /// Verifies that a multidimensional primitive PLC array (e.g. Array[1..2, 1..3] of Int) is reshaped into a genuine + /// multidimensional CLR array on read, and correctly distributed back into its individual element tags on write, instead of + /// collapsing into (or requiring) a flat one-dimensional array. + /// + /// + /// Regression test for a design gap ported from the upstream fork's e91d8f2 ("Extended DataTypes") commit, which + /// introduced dedicated PlcTagXxxArrayMDim types there. This driver instead extends the existing + /// aggregate-element mechanism (, , + /// ) to reshape/distribute values without introducing new tag types. + /// + public sealed class PlcTagMultiDimensionalArrayTests + { + private static VarInfo CreateTwoByThreeIntArrayVarInfo() + { + return new VarInfo + { + Name = "DB.Matrix", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x11223344, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_INT, + ArrayElementCount = 6, + ArrayDimensions = new[] + { + new S7CommPlusArrayDimension(1, 2), + new S7CommPlusArrayDimension(1, 3), + }, + }; + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByThreeIntArrayVarInfo()); + + Assert.Equal(6, tag.AggregateElements.Count); + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueInt((short)(index + 1)), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(3, value.GetLength(1)); + // Row-major: outermost (first declared) dimension varies slowest. + Assert.Equal(1, value[0, 0]); + Assert.Equal(2, value[0, 1]); + Assert.Equal(3, value[0, 2]); + Assert.Equal(4, value[1, 0]); + Assert.Equal(5, value[1, 1]); + Assert.Equal(6, value[1, 2]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalWriteIntoElementTags() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByThreeIntArrayVarInfo()); + + var writeValue = new short[2, 3] { { 10, 20, 30 }, { 40, 50, 60 } }; + tag.SetValue(writeValue); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagInt)element).Value) + .ToArray(); + Assert.Equal(new short[] { 10, 20, 30, 40, 50, 60 }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagKeepsFlatArrayForOneDimensionalAggregate() + { + var varInfo = new VarInfo + { + Name = "DB.Values", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x55667788, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_INT, + ArrayElementCount = 3, + ArrayDimensions = new[] { new S7CommPlusArrayDimension(0, 3) }, + }; + + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueInt((short)(index + 1)), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(new short[] { 1, 2, 3 }, value); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalBoolReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_BOOL, "DB.Flags", 0x22334455)); + var expected = new[] { true, false, false, true }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueBool(expected[index]), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.True(value[0, 0]); + Assert.False(value[0, 1]); + Assert.False(value[1, 0]); + Assert.True(value[1, 1]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalBoolWriteIntoElementTags() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_BOOL, "DB.Flags", 0x22334455)); + tag.SetValue(new[,] { { true, false }, { false, true } }); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagBool)element).Value) + .ToArray(); + Assert.Equal(new[] { true, false, false, true }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalRealReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_REAL, "DB.Measurements", 0x33445566)); + var expected = new[] { 1.5f, 2.5f, 3.5f, 4.5f }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueReal(expected[index]), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.Equal(1.5f, value[0, 0]); + Assert.Equal(2.5f, value[0, 1]); + Assert.Equal(3.5f, value[1, 0]); + Assert.Equal(4.5f, value[1, 1]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalRealWriteIntoElementTags() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_REAL, "DB.Measurements", 0x33445566)); + tag.SetValue(new[,] { { 1.5f, 2.5f }, { 3.5f, 4.5f } }); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagReal)element).Value) + .ToArray(); + Assert.Equal(new[] { 1.5f, 2.5f, 3.5f, 4.5f }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalStringReadIntoGenuineMultiDimensionalArray() + { + var varInfo = CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_STRING, "DB.Texts", 0x44556677); + varInfo.MaxStringLength = 10; + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + var expected = new[] { "aa", "bb", "cc", "dd" }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(CreateStringReadValue(expected[index], 10), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.Equal("aa", value[0, 0]); + Assert.Equal("bb", value[0, 1]); + Assert.Equal("cc", value[1, 0]); + Assert.Equal("dd", value[1, 1]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalStringWriteIntoElementTags() + { + var varInfo = CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_STRING, "DB.Texts", 0x44556677); + varInfo.MaxStringLength = 10; + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + tag.SetValue(new[,] { { "aa", "bb" }, { "cc", "dd" } }); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagString)element).Value) + .ToArray(); + Assert.Equal(new[] { "aa", "bb", "cc", "dd" }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalDtlStructReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_DTL, "DB.Timestamps", 0x66778899)); + var expected = new[] + { + new DateTime(2020, 1, 1), + new DateTime(2020, 1, 2), + new DateTime(2020, 1, 3), + new DateTime(2020, 1, 4), + }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(CreateDtlReadValue(expected[index]), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.Equal(expected[0], value[0, 0]); + Assert.Equal(expected[1], value[0, 1]); + Assert.Equal(expected[2], value[1, 0]); + Assert.Equal(expected[3], value[1, 1]); + } + + private static VarInfo CreateTwoByTwoVarInfo(uint softdatatype, string name, uint symbolCrc) + { + return new VarInfo + { + Name = name, + AccessSequence = "8A0E0001.F", + SymbolCrc = symbolCrc, + Softdatatype = softdatatype, + ArrayElementCount = 4, + ArrayDimensions = new[] + { + new S7CommPlusArrayDimension(0, 2), + new S7CommPlusArrayDimension(0, 2), + }, + }; + } + + private static ValueUSIntArray CreateStringReadValue(string text, byte maxLength) + { + var bytes = System.Text.Encoding.GetEncoding("ISO-8859-1").GetBytes(text); + var b = new byte[maxLength + 2]; + b[0] = maxLength; + b[1] = (byte)bytes.Length; + Array.Copy(bytes, 0, b, 2, bytes.Length); + return new ValueUSIntArray(b); + } + + private static ValueStruct CreateDtlReadValue(DateTime value) + { + var structValue = new ValueStruct(0x02000043); + var barr = new byte[12]; + barr[0] = (byte)(value.Year >> 8); + barr[1] = (byte)value.Year; + barr[2] = (byte)value.Month; + barr[3] = (byte)value.Day; + barr[5] = (byte)value.Hour; + barr[6] = (byte)value.Minute; + barr[7] = (byte)value.Second; + structValue.AddStructElement(0x02000043, new ValueByteArray(barr)); + return structValue; + } + } +} diff --git a/src/S7CommPlusDriver/ClientApi/PlcTag.cs b/src/S7CommPlusDriver/ClientApi/PlcTag.cs index 790d148..986e7ec 100644 --- a/src/S7CommPlusDriver/ClientApi/PlcTag.cs +++ b/src/S7CommPlusDriver/ClientApi/PlcTag.cs @@ -18,6 +18,7 @@ namespace S7CommPlusDriver.ClientApi public abstract class PlcTag { private IReadOnlyList m_AggregateElements = Array.Empty(); + private IReadOnlyList m_AggregateDimensions = Array.Empty(); public string Name; public ItemAddress Address; @@ -72,21 +73,36 @@ public virtual void ProcessWriteResult(UInt64 error) internal virtual int GetMaxStringLength() => 0; /// - /// Gets the tag's current typed value as a boxed via its public Value property. + /// Gets the tag's current typed value. For a multidimensional aggregate array (see and + /// ), the reshaped multidimensional array is returned because the concrete tag's Value + /// property can only expose a flat one-dimensional array. Every other tag returns its boxed public Value property. /// /// The boxed value, or when the concrete tag type has no public Value property. public virtual object GetValue() { + if (m_AggregateDimensions.Count > 1) + { + return AggregateValue; + } var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance); return valueProperty?.GetValue(this); } /// - /// Sets the tag's current typed value from a boxed via its public Value property. + /// Sets the tag's current typed value. For a multidimensional aggregate array, is expected to be + /// a multidimensional matching the PLC-declared dimensions; it is reshaped and distributed directly + /// into the individual because the concrete tag's Value property can only hold a + /// flat one-dimensional array. Every other tag assigns to its public Value property. /// /// The value to assign, which must be assignable to the concrete tag's Value property type. public virtual void SetValue(object value) { + if (m_AggregateDimensions.Count > 1 && value is Array multiDimValue) + { + DistributeMultiDimensionalValue(multiDimValue); + AggregateValue = multiDimValue; + return; + } var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance); valueProperty?.SetValue(this, value); } @@ -125,14 +141,21 @@ protected static int CheckErrorAndType(ulong error, object valueObj, Type checkT /// /// Configures the scalar element tags that collectively represent this aggregate PLC array. /// - /// Element tags in PLC declaration order. - internal void SetAggregateElements(IReadOnlyList elements) + /// Element tags in PLC declaration order (row-major, outermost dimension first). + /// + /// The PLC-declared dimension lengths in declaration order (outermost first), or /empty for a + /// one-dimensional aggregate array. A single-element list is treated the same as . + /// + internal void SetAggregateElements(IReadOnlyList elements, IReadOnlyList dimensions = null) { m_AggregateElements = elements ?? throw new ArgumentNullException(nameof(elements)); + m_AggregateDimensions = dimensions != null && dimensions.Count > 1 ? dimensions : Array.Empty(); } /// /// Publishes a completed set of scalar element reads as one aggregate value and mirrors it into a compatible typed array tag. + /// A multidimensional aggregate array ( has more than one entry) is reshaped into a + /// genuine multidimensional instead of the concrete tag's inherently flat Value property. /// /// The first element error, or zero when every element succeeded. internal virtual void CompleteAggregateRead(ulong itemError) @@ -153,29 +176,39 @@ internal virtual void CompleteAggregateRead(ulong itemError) return; } - var values = Array.CreateInstance(elementValueProperty.PropertyType, m_AggregateElements.Count); + var flatValues = Array.CreateInstance(elementValueProperty.PropertyType, m_AggregateElements.Count); for (var index = 0; index < m_AggregateElements.Count; index++) { - values.SetValue(elementValueProperty.GetValue(m_AggregateElements[index]), index); + flatValues.SetValue(elementValueProperty.GetValue(m_AggregateElements[index]), index); } - AggregateValue = values; - var aggregateValueProperty = GetType().GetProperty("Value", BindingFlags.Instance | BindingFlags.Public); - if (aggregateValueProperty?.PropertyType == values.GetType()) + if (m_AggregateDimensions.Count > 1) { - aggregateValueProperty.SetValue(this, values); + AggregateValue = ReshapeToMultiDimensional(elementValueProperty.PropertyType, flatValues, m_AggregateDimensions); + } + else + { + AggregateValue = flatValues; + var aggregateValueProperty = GetType().GetProperty("Value", BindingFlags.Instance | BindingFlags.Public); + if (aggregateValueProperty?.PropertyType == flatValues.GetType()) + { + aggregateValueProperty.SetValue(this, flatValues); + } } Quality = PlcTagQC.TAG_QUALITY_GOOD; } /// /// Copies a typed parent array value into its scalar element tags before an aggregate write is encoded. - /// Unsupported native array types intentionally keep their explicitly populated element values unchanged. + /// Unsupported native array types intentionally keep their explicitly populated element values unchanged. A + /// multidimensional aggregate array is intentionally skipped here because already distributed + /// its values directly into at assignment time (its concrete tag's Value property + /// cannot hold a multidimensional shape). /// /// A typed parent array does not contain exactly one value per aggregate element. internal virtual void PrepareAggregateWrite() { - if (m_AggregateElements.Count == 0) + if (m_AggregateElements.Count == 0 || m_AggregateDimensions.Count > 1) { return; } @@ -204,6 +237,89 @@ internal virtual void PrepareAggregateWrite() } } + /// + /// Distributes a caller-supplied multidimensional array directly into the scalar element tags' Value properties, + /// in the same row-major order used by and the PLC's own element access IDs. + /// + /// The multidimensional array to distribute. Its shape must match . + /// The array's rank or per-dimension lengths do not match the PLC-declared dimensions. + private void DistributeMultiDimensionalValue(Array multiDimValue) + { + if (multiDimValue.Rank != m_AggregateDimensions.Count) + { + throw new ArgumentException( + $"Aggregate tag '{Name}' requires an array of rank {m_AggregateDimensions.Count}, but received rank {multiDimValue.Rank}.", + nameof(multiDimValue)); + } + for (var dimension = 0; dimension < m_AggregateDimensions.Count; dimension++) + { + if (multiDimValue.GetLength(dimension) != m_AggregateDimensions[dimension]) + { + throw new ArgumentException( + $"Aggregate tag '{Name}' requires dimension {dimension} to have length {m_AggregateDimensions[dimension]}, but received {multiDimValue.GetLength(dimension)}.", + nameof(multiDimValue)); + } + } + + var flatIndex = 0; + foreach (var indices in EnumerateRowMajorIndices(m_AggregateDimensions)) + { + var elementValueProperty = m_AggregateElements[flatIndex].GetType().GetProperty("Value", BindingFlags.Instance | BindingFlags.Public); + if (elementValueProperty == null) + { + throw new InvalidOperationException($"Aggregate element '{m_AggregateElements[flatIndex].Name}' has no writable Value property."); + } + elementValueProperty.SetValue(m_AggregateElements[flatIndex], multiDimValue.GetValue(indices)); + flatIndex++; + } + } + + /// + /// Reshapes a flat array, ordered exactly like , into a genuine multidimensional array. + /// + /// The CLR element type shared by every scalar element tag's Value property. + /// The flat values in row-major order (outermost dimension first). + /// The PLC-declared dimension lengths in declaration order (outermost first). + /// A multidimensional array of with one entry per element of . + private static Array ReshapeToMultiDimensional(Type elementType, Array flatValues, IReadOnlyList dimensions) + { + var lengths = dimensions.Select(dimension => checked((int)dimension)).ToArray(); + var multiDimValues = Array.CreateInstance(elementType, lengths); + var flatIndex = 0; + foreach (var indices in EnumerateRowMajorIndices(dimensions)) + { + multiDimValues.SetValue(flatValues.GetValue(flatIndex), indices); + flatIndex++; + } + return multiDimValues; + } + + /// + /// Enumerates every index combination of a multidimensional shape in row-major order (outermost dimension varies + /// slowest), matching the order in which the PLC protocol enumerates aggregate array element access IDs. + /// + /// The dimension lengths in declaration order (outermost first). + private static IEnumerable EnumerateRowMajorIndices(IReadOnlyList dimensions) + { + var indices = new int[dimensions.Count]; + var totalCount = 1L; + foreach (var dimension in dimensions) + { + totalCount = checked(totalCount * dimension); + } + + for (var flatIndex = 0L; flatIndex < totalCount; flatIndex++) + { + var remainder = flatIndex; + for (var dimension = dimensions.Count - 1; dimension >= 0; dimension--) + { + indices[dimension] = checked((int)(remainder % dimensions[dimension])); + remainder /= dimensions[dimension]; + } + yield return indices; + } + } + protected static string ResultString(PlcTag tag, string value ) { return String.Format("{0:X02}: {1}", tag.Quality, value); diff --git a/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs b/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs index e3891bf..c06a260 100644 --- a/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs +++ b/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs @@ -2086,10 +2086,30 @@ private static PlcTag CreateResolvedPlcTag(VarInfo varInfo, PVartypeListElement }) .Where(elementTag => elementTag != null) .ToList(); - tag.SetAggregateElements(elementTags); + tag.SetAggregateElements(elementTags, GetAggregateArrayDimensions(varType)); return tag; } + /// + /// Extracts the PLC-declared dimension lengths (outermost first) of an aggregate primitive array, for reshaping the + /// flat element-tag list into a genuine multidimensional array. Returns an empty list for one-dimensional arrays. + /// + /// The resolved member datatype and dimension metadata. + private static IReadOnlyList GetAggregateArrayDimensions(PVartypeListElement varType) + { + if (varType.OffsetInfoType is not IOffsetInfoType_MDim multiDimensional) + { + return Array.Empty(); + } + + // GetMdimArrayElementCount() stores dimensions inside-out; reverse to declaration order (outermost first) and + // drop trailing zero-length entries, matching GetAggregateArrayElementAccessIds's own filtering. + return multiDimensional.GetMdimArrayElementCount() + .TakeWhile(count => count > 0) + .Reverse() + .ToArray(); + } + /// /// Creates a typed tag directly from one aggregate browse result, avoiding a second walk through the PLC type catalog. /// @@ -2126,7 +2146,10 @@ internal static PlcTag CreateResolvedPlcTag(VarInfo varInfo) varInfo.MaxStringLength)) .Where(elementTag => elementTag != null) .ToList(); - tag.SetAggregateElements(elementTags); + var dimensions = varInfo.ArrayDimensions != null && varInfo.ArrayDimensions.Count > 1 + ? varInfo.ArrayDimensions.Select(dimension => dimension.ElementCount).ToArray() + : Array.Empty(); + tag.SetAggregateElements(elementTags, dimensions); return tag; }