Skip to content

[master378] Bound nested array preallocation, XML depth and inbound PubSub metadata #4603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
romanett merged 5 commits into from
Oct 1, 2026
Merged

[master378] Bound nested array preallocation, XML depth and inbound PubSub metadata #4603

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
264ab00
[master378] Bound nested array preallocation, XML depth and inbound P…
romanett Sep 30, 2026
d1d15a3
Cover the JSON XmlElement depth check and the UADP metadata Publisher…
romanett Sep 30, 2026
9bbd25f
Address decoder and publisher filter review feedback
Copilot Oct 1, 2026
5e2fb5a
Share XML depth check without friend access
Copilot Oct 1, 2026
25e0f47
Fix shared XML depth helper compilation
Copilot Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion Libraries/Opc.Ua.PubSub/UaPubSubConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,12 @@ protected void ProcessDecodedNetworkMessage(UaNetworkMessage networkMessage, str

lock (Lock)
{
// check if reader's MetaData shall be updated
// check if reader's MetaData shall be updated. The metadata message is
// not authenticated: only accept it from the publisher the reader
// receives data from (6.2.8.1 PublisherId filter, null accepts any).
if (reader.DataSetWriterId != 0 &&
reader.DataSetWriterId == networkMessage.DataSetWriterId &&
MatchesPublisherId(reader, networkMessage) &&
(
reader.DataSetMetaData == null ||
!Utils.IsEqual(
Expand Down Expand Up @@ -416,6 +419,36 @@ protected void ProcessDecodedNetworkMessage(UaNetworkMessage networkMessage, str
}
}

/// <summary>
/// Checks the PublisherId of a received network message against the PublisherId
/// filter of a reader, the same way the data messages are filtered when decoded.
/// A reader without PublisherId accepts any publisher.
/// </summary>
private static bool MatchesPublisherId(
DataSetReaderDataType reader,
UaNetworkMessage networkMessage)
{
object readerPublisherId = reader.PublisherId.Value;
if (readerPublisherId == null)
{
return true;
}

switch (networkMessage)
{
case Encoding.JsonNetworkMessage jsonNetworkMessage:
return jsonNetworkMessage.PublisherId != null &&
Utils.IsEqual(
jsonNetworkMessage.PublisherId,
readerPublisherId.ToString());
case Encoding.UadpNetworkMessage uadpNetworkMessage:
return uadpNetworkMessage.PublisherId != null &&
Utils.IsEqual(uadpNetworkMessage.PublisherId, readerPublisherId);
default:
return false;
}
}

/// <summary>
/// Get all dataset readers defined for this UaSubscriber component
/// </summary>
Expand Down
2 changes: 2 additions & 0 deletions Stack/Opc.Ua.Core/Types/Encoders/JsonDecoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,8 @@ public XmlElement ReadXmlElement(string fieldName)

try
{
XmlDecoder.CheckXmlElementDepth(value, Context, ConformanceLevel.Document);

var document = new XmlDocument();

using (var reader = XmlReader.Create(
Expand Down
235 changes: 169 additions & 66 deletions Stack/Opc.Ua.Types/Encoders/BinaryDecoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Runtime.CompilerServices;
using System.Text;
Expand Down Expand Up @@ -532,6 +531,7 @@ public XmlElement ReadXmlElement(string fieldName)
utf8StringLength--;
}
string xmlString = Encoding.UTF8.GetString(bytes, 0, utf8StringLength);
XmlDecoder.CheckXmlElementDepth(xmlString, Context, ConformanceLevel.Document);
using var stream = new StringReader(xmlString);
using var reader = XmlReader.Create(stream, CoreUtils.DefaultXmlReaderSettings());
document.Load(reader);
Expand Down Expand Up @@ -1223,14 +1223,22 @@ public VariantCollection ReadVariantArray(string fieldName)
return null;
}

var values = new VariantCollection(length);
int capacity = ReserveArrayCapacity(length, s_variantElementSize, out long reserved);

@mrsuciu mrsuciu Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only these Read[Variant]Array reserve space, why not also ReadStringArray and ReadNodeIdArray and other arrays ? can't the length be "spoofed" also and allocate more than necessary ?

@romanett romanett Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only Variant Array can Nest which makes a real amplification possible

try
{
var values = new VariantCollection(capacity);

for (int ii = 0; ii < length; ii++)
for (int ii = 0; ii < length; ii++)
{
values.Add(ReadVariant(null));
}

return values;
}
finally
{
values.Add(ReadVariant(null));
m_preallocatedBytes -= reserved;
}

return values;
}

/// <inheritdoc/>
Expand All @@ -1243,14 +1251,22 @@ public DataValueCollection ReadDataValueArray(string fieldName)
return null;
}

var values = new DataValueCollection(length);
int capacity = ReserveArrayCapacity(length, IntPtr.Size, out long reserved);
try
{
var values = new DataValueCollection(capacity);

for (int ii = 0; ii < length; ii++)
for (int ii = 0; ii < length; ii++)
{
values.Add(ReadDataValue(null));
}

return values;
}
finally
{
values.Add(ReadDataValue(null));
m_preallocatedBytes -= reserved;
}

return values;
}

/// <inheritdoc/>
Expand All @@ -1263,14 +1279,22 @@ public ExtensionObjectCollection ReadExtensionObjectArray(string fieldName)
return null;
}

var values = new ExtensionObjectCollection(length);
int capacity = ReserveArrayCapacity(length, IntPtr.Size, out long reserved);
try
{
var values = new ExtensionObjectCollection(capacity);

for (int ii = 0; ii < length; ii++)
{
values.Add(ReadExtensionObject(null));
}

for (int ii = 0; ii < length; ii++)
return values;
}
finally
{
values.Add(ReadExtensionObject(null));
m_preallocatedBytes -= reserved;
}

return values;
}

/// <inheritdoc/>
Expand All @@ -1286,14 +1310,7 @@ public Array ReadEncodeableArray(
return null;
}

var values = Array.CreateInstance(systemType, length);

for (int ii = 0; ii < length; ii++)
{
values.SetValue(ReadEncodeable(null, systemType, encodeableTypeId), ii);
}

return values;
return ReadEncodeableElements(length, systemType, encodeableTypeId);
}

/// <inheritdoc/>
Expand Down Expand Up @@ -1426,18 +1443,7 @@ public Array ReadArray(
Array elements = null;
if (DetermineIEncodeableSystemType(ref systemType, encodeableTypeId))
{
elements = Array.CreateInstance(systemType, length);
for (int i = 0; i < length; i++)
{
IEncodeable element = ReadEncodeable(
null,
systemType,
encodeableTypeId);
elements.SetValue(Convert.ChangeType(
element,
systemType,
CultureInfo.InvariantCulture), i);
}
elements = ReadEncodeableElements(length, systemType, encodeableTypeId);
}

elements ??= ReadArrayElements(length, builtInType);
Expand Down Expand Up @@ -1849,41 +1855,23 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
break;
}
case BuiltInType.ExtensionObject:
{
var values = new ExtensionObject[length];

for (int ii = 0; ii < values.Length; ii++)
{
values[ii] = ReadExtensionObject();
}

array = values;
array = ReadNestedElements(
length,
IntPtr.Size,
static decoder => decoder.ReadExtensionObject());
break;
}
case BuiltInType.DataValue:
{
var values = new DataValue[length];

for (int ii = 0; ii < values.Length; ii++)
{
values[ii] = ReadDataValue(null);
}

array = values;
array = ReadNestedElements(
length,
IntPtr.Size,
static decoder => decoder.ReadDataValue(null));
break;
}
case BuiltInType.Variant:
{
var values = new Variant[length];

for (int ii = 0; ii < values.Length; ii++)
{
values[ii] = ReadVariant(null);
}

array = values;
array = ReadNestedElements(
length,
s_variantElementSize,
static decoder => decoder.ReadVariant(null));
break;
}
case BuiltInType.DiagnosticInfo:
{
var values = new DiagnosticInfo[length];
Expand Down Expand Up @@ -1938,6 +1926,103 @@ private int ReadArrayLength([CallerMemberName] string callerMemberName = "")
return length;
}

/// <summary>
/// Returns how many elements to allocate up front for an array of
/// <paramref name="length"/> elements that can contain nested arrays.
/// The length prefix of such an array is not checked against the
/// remaining message, and every level of a chain of arrays nested in
/// the first element of their parent sees the same remaining bytes,
/// so the preallocations of a chain would multiply. An array is
/// therefore allocated at its full length only while all arrays
/// allocated this way and still being read, this one included, take
/// at most <see cref="kMaxPreallocatedBytesPerRemainingByte"/> bytes
/// per remaining message byte. Otherwise the caller starts with
/// <see cref="kMaxPreallocatedArrayBytes"/> and grows the array as the
/// elements are read. The caller subtracts <paramref name="reserved"/>
/// from <see cref="m_preallocatedBytes"/> after the last element.
/// </summary>
private int ReserveArrayCapacity(int length, int elementSize, out long reserved)
{
reserved = 0;
long bytes = (long)length * elementSize;
if (bytes <= kMaxPreallocatedArrayBytes)
{
return length;
}

Stream stream = m_reader.BaseStream;
if (stream.CanSeek &&
m_preallocatedBytes + bytes <=
kMaxPreallocatedBytesPerRemainingByte * Math.Max(0, stream.Length - stream.Position))
{
reserved = bytes;
m_preallocatedBytes += bytes;
return length;
}

return kMaxPreallocatedArrayBytes / elementSize;
}

/// <summary>
/// Reads an array of values that can contain nested arrays.
/// </summary>
private T[] ReadNestedElements<T>(
int length,
int elementSize,
Func<BinaryDecoder, T> readElement)
{
int capacity = ReserveArrayCapacity(length, elementSize, out long reserved);
try
{
var values = new T[capacity];
for (int ii = 0; ii < length; ii++)
{
if (ii == values.Length)
{
Array.Resize(ref values, (int)Math.Min(length, 2L * values.Length));
}
values[ii] = readElement(this);
}
return values;
}
finally
{
m_preallocatedBytes -= reserved;
}
}

/// <summary>
/// Reads an array of encodeables of <paramref name="systemType"/>.
/// </summary>
private Array ReadEncodeableElements(
int length,
Type systemType,
ExpandedNodeId encodeableTypeId)
{
int capacity = ReserveArrayCapacity(length, IntPtr.Size, out long reserved);
try
{
Array values = Array.CreateInstance(systemType, capacity);
for (int ii = 0; ii < length; ii++)
{
if (ii == values.Length)
{
Array grown = Array.CreateInstance(
systemType,
(int)Math.Min(length, 2L * values.Length));
Array.Copy(values, grown, values.Length);
values = grown;
}
values.SetValue(ReadEncodeable(null, systemType, encodeableTypeId), ii);
}
return values;
}
finally
{
m_preallocatedBytes -= reserved;
}
}

/// <summary>
/// Reads the body of a node id.
/// </summary>
Expand Down Expand Up @@ -2684,11 +2769,29 @@ private static void ValidateStreamRequirements(Stream stream)
}
}

/// <summary>
/// Arrays of nested values up to this size are always allocated at
/// their full length.
/// </summary>
private const int kMaxPreallocatedArrayBytes = 16 * 1024;

/// <summary>
/// The preallocation budget shared by the arrays of nested values
/// that are being read, per remaining message byte.
/// </summary>
private const int kMaxPreallocatedBytesPerRemainingByte = 8;

/// <summary>
/// The size of a Variant array element (a value and a type reference).
/// </summary>
private static readonly int s_variantElementSize = 2 * IntPtr.Size;

private BinaryReader m_reader;
private ushort[] m_namespaceMappings;
private ushort[] m_serverMappings;
private uint m_nestingLevel;
private uint m_encodeablesRecovered;
private long m_preallocatedBytes;
private readonly ILogger m_logger;
}
}
Loading
Loading