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 7cdef54..790d148 100644 --- a/src/S7CommPlusDriver/ClientApi/PlcTag.cs +++ b/src/S7CommPlusDriver/ClientApi/PlcTag.cs @@ -64,6 +64,33 @@ 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. + /// + /// 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) @@ -981,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; @@ -1483,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; @@ -2619,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/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) { 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); + } } /// 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); } }