Skip to content

Commit

Permalink
Nested complex types which contain structure derived from an abstract…
Browse files Browse the repository at this point in the history
… type are not correctly encoded (#2349)

- bug: if the nested complex type is derived from an abstract type like a Union, the decoder uses the abstract type instead of the actual type to decode
- fix: the loop to determine the supertype of the type to be decoded stops too early and returns an invalid type reference, e.g. for Unions. Instead the loop will now continue until a valid BuiltInType or a Structure or Enumeration type is found.
  • Loading branch information
mregen authored Oct 26, 2023
1 parent d47bc85 commit 71ad9af
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 15 deletions.
2 changes: 1 addition & 1 deletion Applications/ConsoleReferenceClient/ClientSamples.cs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ public async Task<IList<INode>> FetchAllNodesNodeCacheAsync(
}
else
{
nodeDictionary[node.NodeId] = node; ;
nodeDictionary[node.NodeId] = node;
}
}
else
Expand Down
28 changes: 24 additions & 4 deletions Libraries/Opc.Ua.Client.ComplexTypes/ComplexTypeSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -319,6 +318,13 @@ void CollectAllDataTypeDefinitions(NodeId nodeId, NodeIdDictionary<DataTypeDefin
}
}

/// <summary>
/// Clear references in datatype cache.
/// </summary>
public void ClearDataTypeCache()
{
m_dataTypeDefinitionCache.Clear();
}
#endregion Public Members

#region Internal Properties
Expand Down Expand Up @@ -1134,8 +1140,11 @@ private async Task<Type> GetFieldTypeAsync(StructureField field, bool allowSubTy
/// </summary>
private async Task<NodeId> GetBuiltInSuperTypeAsync(NodeId dataType, bool allowSubTypes, bool isOptional, CancellationToken ct = default)
{
const int MaxSuperTypes = 100;

int iterations = 0;
NodeId superType = dataType;
while (true)
while (iterations++ < MaxSuperTypes)
{
superType = await m_complexTypeResolver.FindSuperTypeAsync(superType, ct).ConfigureAwait(false);
if (superType?.IsNullNodeId != false)
Expand Down Expand Up @@ -1172,10 +1181,21 @@ await IsAbstractTypeAsync(dataType, ct).ConfigureAwait(false))
}
return null;
}
break;
// end search if a valid BuiltInType is found. Treat type as opaque.
else if (superType.IdType == IdType.Numeric &&
(uint)superType.Identifier >= (uint)BuiltInType.Boolean &&
(uint)superType.Identifier <= (uint)BuiltInType.DiagnosticInfo)
{
return superType;
}
// no valid supertype found
else if (superType == DataTypeIds.BaseDataType)
{
break;
}
}
}
return superType;
return null;
}

/// <summary>
Expand Down
5 changes: 4 additions & 1 deletion Stack/Opc.Ua.Core/Stack/Bindings/ArraySegmentStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ public override long Seek(long offset, SeekOrigin origin)

int position = (int)offset;

CheckEndOfStream();
if (position >= GetAbsolutePosition())
{
CheckEndOfStream();
}

for (int ii = 0; ii < m_buffers.Count; ii++)
{
Expand Down
19 changes: 14 additions & 5 deletions Stack/Opc.Ua.Core/Types/Encoders/BinaryDecoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2126,22 +2126,31 @@ private ExtensionObject ReadExtensionObject()

// verify the decoder did not exceed the length of the encodeable object
int used = Position - start;
if (length < used)
if (length != used)
{
throw ServiceResultException.Create(
StatusCodes.BadEncodingLimitsExceeded,
"The encodeable.Decoder operation exceeded the length of the extension object. {0} > {1}",
StatusCodes.BadDecodingError,
"The encodeable.Decoder operation did not match the length of the extension object. {0} != {1}",
used, length);
}
}
catch (ServiceResultException sre) when (sre.StatusCode == StatusCodes.BadEncodingLimitsExceeded)
catch (EndOfStreamException eofStream)
{
// type was known but decoding failed, reset stream!
m_reader.BaseStream.Position = start;
encodeable = null;
Utils.LogWarning(sre, "Failed to decode encodeable type '{0}', NodeId='{1}'. BinaryDecoder recovered.",
Utils.LogWarning(eofStream, "End of stream, failed to decode encodeable type '{0}', NodeId='{1}'. BinaryDecoder recovered.",
systemType.Name, extension.TypeId);
}
catch (ServiceResultException sre) when
((sre.StatusCode == StatusCodes.BadEncodingLimitsExceeded) || (sre.StatusCode == StatusCodes.BadDecodingError))
{
// type was known but decoding failed, reset stream!
m_reader.BaseStream.Position = start;
encodeable = null;
Utils.LogWarning(sre, "{0}, failed to decode encodeable type '{1}', NodeId='{2}'. BinaryDecoder recovered.",
sre.Message, systemType.Name, extension.TypeId);
}
finally
{
m_nestingLevel = nestingLevel;
Expand Down
2 changes: 2 additions & 0 deletions Stack/Opc.Ua.Core/Types/Encoders/BinaryEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,13 @@ protected virtual void Dispose(bool disposing)
{
m_writer.Flush();
m_writer.Dispose();
m_writer = null;
}

if (!m_leaveOpen)
{
m_ostrm?.Dispose();
m_ostrm = null;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions Stack/Opc.Ua.Core/Types/Encoders/JsonDecoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ protected virtual void Dispose(bool disposing)
if (m_reader != null)
{
m_reader.Close();
m_reader = null;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions Stack/Opc.Ua.Core/Types/Encoders/JsonEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ protected virtual void Dispose(bool disposing)
if (m_writer != null)
{
Close();
m_writer = null;
}

if (!m_leaveOpen)
Expand Down
107 changes: 103 additions & 4 deletions Tests/Opc.Ua.Client.ComplexTypes.Tests/TypeSystemClientTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ public class TypeSystemClientTest : IUAClient
string m_pkiRoot;
Uri m_url;

// for test that fetched and browsed node count match
int m_fetchedNodesCount;
int m_browsedNodesCount;

public TypeSystemClientTest()
{
m_uriScheme = Utils.UriSchemeOpcTcp;
Expand All @@ -79,6 +83,9 @@ public TypeSystemClientTest(string uriScheme)
[OneTimeSetUp]
public Task OneTimeSetUp()
{
m_fetchedNodesCount = -1;
m_browsedNodesCount = -1;

return OneTimeSetUpAsync(null);
}

Expand All @@ -97,6 +104,7 @@ public async Task OneTimeSetUpAsync(TextWriter writer = null)
SecurityNone = true,
AutoAccept = true,
AllNodeManagers = true,
OperationLimits = true,
};
if (writer != null)
{
Expand Down Expand Up @@ -202,18 +210,21 @@ public async Task BrowseComplexTypesServerAsync()
await samples.BrowseFullAddressSpaceAsync(this, Objects.RootFolder).ConfigureAwait(false);

TestContext.Out.WriteLine("References: {0}", referenceDescriptions.Count);
m_browsedNodesCount = referenceDescriptions.Count;

NodeIdCollection variableIds = new NodeIdCollection(referenceDescriptions
.Where(r => r.NodeClass == NodeClass.Variable && r.TypeDefinition.NamespaceIndex != 0)
.Where(r => r.NodeClass == NodeClass.Variable)
.Select(r => ExpandedNodeId.ToNodeId(r.NodeId, m_session.NamespaceUris)));

TestContext.Out.WriteLine("VariableIds: {0}", variableIds.Count);

(var values, var serviceResults) = await samples.ReadAllValuesAsync(this, variableIds).ConfigureAwait(false);

int ii = 0;
foreach (var serviceResult in serviceResults)
{
Assert.IsTrue(ServiceResult.IsGood(serviceResult));
var result = serviceResults[ii++];
Assert.IsTrue(ServiceResult.IsGood(serviceResult), $"Expected good result, but received {serviceResult}");
}
}

Expand All @@ -226,12 +237,14 @@ public async Task FetchComplexTypesServerAsync()

IList<INode> allNodes = null;
allNodes = await samples.FetchAllNodesNodeCacheAsync(
this, Objects.RootFolder, true, true, false).ConfigureAwait(false);
this, Objects.RootFolder, true, false, false).ConfigureAwait(false);

TestContext.Out.WriteLine("References: {0}", allNodes.Count);

m_fetchedNodesCount = allNodes.Count;

NodeIdCollection variableIds = new NodeIdCollection(allNodes
.Where(r => r.NodeClass == NodeClass.Variable && ((VariableNode)r).DataType.NamespaceIndex != 0)
.Where(r => r.NodeClass == NodeClass.Variable && r is VariableNode && ((VariableNode)r).DataType.NamespaceIndex != 0)
.Select(r => ExpandedNodeId.ToNodeId(r.NodeId, m_session.NamespaceUris)));

TestContext.Out.WriteLine("VariableIds: {0}", variableIds.Count);
Expand All @@ -242,6 +255,92 @@ public async Task FetchComplexTypesServerAsync()
{
Assert.IsTrue(ServiceResult.IsGood(serviceResult));
}

// check if complex type is properly decoded
bool testFailed = false;
for (int ii = 0; ii < values.Count; ii++)
{
DataValue value = values[ii];
NodeId variableId = variableIds[ii];
ExpandedNodeId variableExpandedNodeId = NodeId.ToExpandedNodeId(variableId, m_session.NamespaceUris);
var variableNode = allNodes.Where(n => n.NodeId == variableId).FirstOrDefault() as VariableNode;
if (variableNode != null &&
variableNode.DataType.NamespaceIndex != 0)
{
TestContext.Out.WriteLine("Check for custom type: {0}", variableNode);
ExpandedNodeId fullTypeId = NodeId.ToExpandedNodeId(variableNode.DataType, m_session.NamespaceUris);
Type type = m_session.Factory.GetSystemType(fullTypeId);
if (type == null)
{
// check for opaque type
NodeId superType = m_session.NodeCache.FindSuperType(fullTypeId);
NodeId lastGoodType = variableNode.DataType;
while (!superType.IsNullNodeId && superType != DataTypes.BaseDataType)
{
if (superType == DataTypeIds.Structure)
{
testFailed = true;
break;
}
lastGoodType = superType;
superType = m_session.NodeCache.FindSuperType(superType);
}

if (testFailed)
{
TestContext.Out.WriteLine("-- Variable: {0} complex type unavailable --> {1}", variableNode.NodeId, variableNode.DataType);
(_, _) = await samples.ReadAllValuesAsync(this, new NodeIdCollection() { variableId }).ConfigureAwait(false);
}
else
{
TestContext.Out.WriteLine("-- Variable: {0} opaque typeid --> {1}", variableNode.NodeId, lastGoodType);
}
continue;
}

if (value.Value is ExtensionObject)
{
Type valueType = ((ExtensionObject)value.Value).Body.GetType();
if (valueType != type)
{
testFailed = true;
TestContext.Out.WriteLine("Variable: {0} type is decoded as ExtensionObject --> {1}", variableNode, value.Value);
(_, _) = await samples.ReadAllValuesAsync(this, new NodeIdCollection() { variableId }).ConfigureAwait(false);
}
continue;
}

if (value.Value is Array array &&
array.GetType().GetElementType() == typeof(ExtensionObject))
{
foreach (ExtensionObject valueItem in array)
{
Type valueType = valueItem.Body.GetType();
if (valueType != type)
{
testFailed = true;
TestContext.Out.WriteLine("Variable: {0} type is decoded as ExtensionObject --> {1}", variableNode, valueItem);
(_, _) = await samples.ReadAllValuesAsync(this, new NodeIdCollection() { variableId }).ConfigureAwait(false);
}
}
}
}
}

if (testFailed)
{
Assert.Fail("Test failed, unknown or undecodable complex type detected. See log for information.");
}
}

[Test, Order(330)]
public void ValidateFetchedAndBrowsedNodesMatch()
{
if (m_browsedNodesCount < 0 || m_fetchedNodesCount < 0)
{
Assert.Ignore("The browse or fetch test did not run.");
}
Assert.AreEqual(m_fetchedNodesCount, m_browsedNodesCount);
}

[Test, Order(400)]
Expand Down
2 changes: 2 additions & 0 deletions Tests/Opc.Ua.Client.Tests/ClientFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public async Task LoadClientConfiguration(string pkiRoot = null, string clientNa
.Build(
"urn:localhost:opcfoundation.org:" + clientName,
"http://opcfoundation.org/UA/" + clientName)
.SetMaxByteStringLength(4 * 1024 * 1024)
.SetMaxArrayLength(1024 * 1024)
.AsClient()
.SetClientOperationLimits(new OperationLimits {
MaxNodesPerBrowse = kDefaultOperationLimits,
Expand Down
8 changes: 8 additions & 0 deletions Tests/Opc.Ua.Server.Tests/ServerFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public async Task LoadConfiguration(string pkiRoot = null)
var serverConfig = Application.Build(
"urn:localhost:UA:" + typeof(T).Name,
"uri:opcfoundation.org:" + typeof(T).Name)
.SetMaxByteStringLength(4 * 1024 * 1024)
.SetMaxArrayLength(1024 * 1024)
.AsServer(
new string[] {
endpointUrl
Expand Down Expand Up @@ -99,6 +101,12 @@ public async Task LoadConfiguration(string pkiRoot = null)
MaxNodesPerWrite = 1000,
MaxNodesPerMethodCall = 1000,
MaxMonitoredItemsPerCall = 1000,
MaxNodesPerHistoryReadData = 1000,
MaxNodesPerHistoryReadEvents = 1000,
MaxNodesPerHistoryUpdateData = 1000,
MaxNodesPerHistoryUpdateEvents = 1000,
MaxNodesPerNodeManagement = 1000,
MaxNodesPerRegisterNodes = 1000,
MaxNodesPerTranslateBrowsePathsToNodeIds = 1000
});
}
Expand Down

0 comments on commit 71ad9af

Please sign in to comment.