Skip to content

Commit

Permalink
Truncate inner diagnostics if nested depth too high (#2247)
Browse files Browse the repository at this point in the history 
- `DiagnosticInfo` contains the `InnerDiagnosticInfo` which can be nested. Limit the recursion to `MaxInnerDepth` in multiple places to prevent stack overflow / endless loops.
- The default value for MaxInnerDepth should not be more than 5.
  • Loading branch information
@mregen
mregen committed Jul 31, 2023
1 parent 1f17aad commit e61664f
Show file tree
Hide file tree
Showing 11 changed files with 971 additions and 835 deletions.
3 changes: 1 addition & 2 deletions Libraries/Opc.Ua.Client/SessionClientBatched.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1871,9 +1871,8 @@ internal set
DiagnosticInfo diagnosticInfo,
int stringTableOffset)
{
const int MaxDepth = 10;
int depth = 0;
while (diagnosticInfo != null && depth++ < MaxDepth)
while (diagnosticInfo != null && depth++ < DiagnosticInfo.MaxInnerDepth)
{
if (diagnosticInfo.LocalizedText >= 0)
{
Expand Down
3 changes: 2 additions & 1 deletion Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,8 @@ public void RemoveReferences(List<LocalReference> referencesToRemove)
{
DiagnosticInfo diagnosticInfo = diagnosticInfos[ii];

while (diagnosticInfo != null)
int depth = 0;
while (diagnosticInfo != null && depth++ < DiagnosticInfo.MaxInnerDepth)
{
if (!String.IsNullOrEmpty(diagnosticInfo.AdditionalInfo))
{
Expand Down
241 changes: 156 additions & 85 deletions Stack/Opc.Ua.Core/Types/BuiltIn/DiagnosticInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,13 @@ namespace Opc.Ua
/// <br/></para>
/// </remarks>
[DataContract(Namespace = Namespaces.OpcUaXsd)]
public class DiagnosticInfo : ICloneable, IFormattable
public sealed class DiagnosticInfo : ICloneable, IFormattable
{
/// <summary>
/// Limits the recursion depth for the InnerDiagnosticInfo field.
/// </summary>
public const int MaxInnerDepth = 5;

#region Constructors
/// <summary>
/// Initializes the object with default values.
Expand All @@ -46,10 +51,18 @@ public DiagnosticInfo()
/// </summary>
/// <remarks>
/// Creates a new instance of the object while copying the value passed in.
/// If InnerDiagnosticInfo exceeds the recursion limit, it is not copied.
/// </remarks>
/// <param name="value">The value to copy</param>
/// <exception cref="ArgumentNullException">Thrown when the value is null</exception>
public DiagnosticInfo(DiagnosticInfo value)
public DiagnosticInfo(DiagnosticInfo value) : this(value, 0)
{
}

/// <summary>
/// Creates a deep copy of the value, but limits the recursion depth.
/// </summary>
private DiagnosticInfo(DiagnosticInfo value, int depth)
{
if (value == null) throw new ArgumentNullException(nameof(value));

Expand All @@ -60,9 +73,9 @@ public DiagnosticInfo(DiagnosticInfo value)
m_additionalInfo = value.m_additionalInfo;
m_innerStatusCode = value.m_innerStatusCode;

if (value.m_innerDiagnosticInfo != null)
if (value.m_innerDiagnosticInfo != null && depth < MaxInnerDepth)
{
m_innerDiagnosticInfo = new DiagnosticInfo(value.m_innerDiagnosticInfo);
m_innerDiagnosticInfo = new DiagnosticInfo(value.m_innerDiagnosticInfo, depth + 1);
}
}

Expand All @@ -89,7 +102,7 @@ public DiagnosticInfo(DiagnosticInfo value)
}

/// <summary>
/// Initializes the object with an exception.
/// Initializes the object with a ServiceResult.
/// </summary>
/// <param name="diagnosticsMask">The bitmask describing the diagnostic data</param>
/// <param name="result">The overall transaction result</param>
Expand All @@ -100,6 +113,25 @@ public DiagnosticInfo(DiagnosticInfo value)
DiagnosticsMasks diagnosticsMask,
bool serviceLevel,
StringTable stringTable)
: this(result, diagnosticsMask, serviceLevel, stringTable, 0)
{
}

/// <summary>
/// Initializes the object with a ServiceResult.
/// Limits the recursion depth for the InnerDiagnosticInfo field.
/// </summary>
/// <param name="diagnosticsMask">The bitmask describing the diagnostic data</param>
/// <param name="result">The overall transaction result</param>
/// <param name="serviceLevel">The service level</param>
/// <param name="stringTable">A table of strings carrying more diagnostic data</param>
/// <param name="depth">The recursion depth of the inner diagnostics field</param>
private DiagnosticInfo(
ServiceResult result,
DiagnosticsMasks diagnosticsMask,
bool serviceLevel,
StringTable stringTable,
int depth)
{
uint mask = (uint)diagnosticsMask;

Expand All @@ -110,7 +142,7 @@ public DiagnosticInfo(DiagnosticInfo value)

diagnosticsMask = (DiagnosticsMasks)mask;

Initialize(result, diagnosticsMask, stringTable);
Initialize(result, diagnosticsMask, stringTable, depth);
}

/// <summary>
Expand All @@ -135,7 +167,7 @@ public DiagnosticInfo(DiagnosticInfo value)

diagnosticsMask = (DiagnosticsMasks)mask;

Initialize(new ServiceResult(exception), diagnosticsMask, stringTable);
Initialize(new ServiceResult(exception), diagnosticsMask, stringTable, 0);
}

/// <summary>
Expand Down Expand Up @@ -172,10 +204,12 @@ private void Initialize()
/// <param name="diagnosticsMask">The bitmask describing the type of diagnostic data</param>
/// <param name="result">The transaction result</param>
/// <param name="stringTable">An array of strings that may be used to provide additional diagnostic details</param>
/// <param name="depth">The depth of the inner diagnostics property</param>
private void Initialize(
ServiceResult result,
DiagnosticsMasks diagnosticsMask,
StringTable stringTable)
StringTable stringTable,
int depth)
{
if (stringTable == null) throw new ArgumentNullException(nameof(stringTable));

Expand Down Expand Up @@ -250,11 +284,21 @@ private void Initialize()
// recursively append the inner diagnostics.
if ((DiagnosticsMasks.ServiceInnerDiagnostics & diagnosticsMask) != 0)
{
m_innerDiagnosticInfo = new DiagnosticInfo(
result.InnerResult,
diagnosticsMask,
true,
stringTable);
if (depth < MaxInnerDepth)
{
m_innerDiagnosticInfo = new DiagnosticInfo(
result.InnerResult,
diagnosticsMask,
true,
stringTable,
depth + 1);
}
else
{
Utils.LogWarning(
"Inner diagnostics truncated. Max depth of {0} exceeded.",
MaxInnerDepth);
}
}
}
}
Expand Down Expand Up @@ -359,60 +403,7 @@ public bool IsNullDiagnosticInfo
/// </summary>
public override bool Equals(object obj)
{
if (Object.ReferenceEquals(this, obj))
{
return true;
}

if (obj == null && IsNullDiagnosticInfo)
{
return true;
}

DiagnosticInfo value = obj as DiagnosticInfo;

if (value != null)
{

if (this.m_symbolicId != value.m_symbolicId)
{
return false;
}

if (this.m_namespaceUri != value.m_namespaceUri)
{
return false;
}

if (this.m_locale != value.m_locale)
{
return false;
}

if (this.m_localizedText != value.m_localizedText)
{
return false;
}

if (this.m_additionalInfo != value.m_additionalInfo)
{
return false;
}

if (this.m_innerStatusCode != value.m_innerStatusCode)
{
return false;
}

if (this.m_innerDiagnosticInfo != null)
{
return this.m_innerDiagnosticInfo.Equals(value.m_innerDiagnosticInfo);
}

return value.m_innerDiagnosticInfo == null;
}

return false;
return Equals(obj, 0);
}

/// <summary>
Expand All @@ -421,23 +412,7 @@ public override bool Equals(object obj)
public override int GetHashCode()
{
var hash = new HashCode();
hash.Add(this.m_symbolicId);
hash.Add(this.m_namespaceUri);
hash.Add(this.m_locale);
hash.Add(this.m_localizedText);

if (this.m_additionalInfo != null)
{
hash.Add(this.m_additionalInfo);
}

hash.Add(this.m_innerStatusCode);

if (this.m_innerDiagnosticInfo != null)
{
hash.Add(this.m_innerDiagnosticInfo);
}

GetHashCode(ref hash, 0);
return hash.ToHashCode();
}

Expand Down Expand Up @@ -476,7 +451,7 @@ public string ToString(string format, IFormatProvider formatProvider)

#region ICloneable Members
/// <inheritdoc/>
public virtual object Clone()
public object Clone()
{
return this.MemberwiseClone();
}
Expand All @@ -490,6 +465,102 @@ public new object MemberwiseClone()
}
#endregion

#region Private Methods
/// <summary>
/// Adds the hashcodes for the object.
/// Limits the recursion depth to prevent stack overflow.
/// </summary>
private void GetHashCode(ref HashCode hash, int depth)
{
hash.Add(this.m_symbolicId);
hash.Add(this.m_namespaceUri);
hash.Add(this.m_locale);
hash.Add(this.m_localizedText);

if (this.m_additionalInfo != null)
{
hash.Add(this.m_additionalInfo);
}

hash.Add(this.m_innerStatusCode);

if (this.m_innerDiagnosticInfo != null && depth < MaxInnerDepth)
{
this.m_innerDiagnosticInfo.GetHashCode(ref hash, depth + 1);
}
}

/// <summary>
/// Determines if the specified object is equal to this object.
/// Limits the depth of the comparison to avoid infinite recursion.
/// </summary>
private bool Equals(object obj, int depth)
{
if (Object.ReferenceEquals(this, obj))
{
return true;
}

if (obj == null && IsNullDiagnosticInfo)
{
return true;
}

DiagnosticInfo value = obj as DiagnosticInfo;

if (value != null)
{

if (this.m_symbolicId != value.m_symbolicId)
{
return false;
}

if (this.m_namespaceUri != value.m_namespaceUri)
{
return false;
}

if (this.m_locale != value.m_locale)
{
return false;
}

if (this.m_localizedText != value.m_localizedText)
{
return false;
}

if (this.m_additionalInfo != value.m_additionalInfo)
{
return false;
}

if (this.m_innerStatusCode != value.m_innerStatusCode)
{
return false;
}

if (this.m_innerDiagnosticInfo != null)
{
if (depth < MaxInnerDepth)
{
return this.m_innerDiagnosticInfo.Equals(value.m_innerDiagnosticInfo, depth + 1);
}
else
{
// ignore the remaining inner diagnostic info and consider it equal.
return true;
}
}

return value.m_innerDiagnosticInfo == null;
}

return false;
}
#endregion

#region Private Members
private int m_symbolicId;
private int m_namespaceUri;
Expand Down

0 comments on commit e61664f

Please sign in to comment.