Skip to content

Commit 00db9a5

Browse files
gavinbarronbaywet
gavinbarron
authored and
baywet
committed
fix: circular ref guard (#3033)
* fix: harden yaml parsing * fix tests and nesting behavior on streams with SharpYaml update * CodeQL fixes * update benchmarks * fix: circular ref guard * update benchmarks * fix: harden yaml parsing * fix tests and nesting behavior on streams with SharpYaml update * CodeQL fixes * update benchmarks * fix: circular ref guard * update benchmarks * codeql updates Signed-off-by: Vincent Biret <vibiret@microsoft.com>
1 parent fdc5718 commit 00db9a5

14 files changed

Lines changed: 664 additions & 145 deletions

‎src/Microsoft.OpenApi/Models/References/BaseOpenApiReferenceHolder.cs‎

Lines changed: 117 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Runtime.CompilerServices;
34

45
namespace Microsoft.OpenApi;
56
/// <summary>
@@ -10,6 +11,11 @@ namespace Microsoft.OpenApi;
1011
/// <typeparam name="V">The type for the reference holding the additional fields and annotations</typeparam>
1112
public abstract class BaseOpenApiReferenceHolder<T, U, V> : IOpenApiReferenceHolder<T, U, V> where T : class, IOpenApiReferenceable, U where U : IOpenApiReferenceable, IOpenApiSerializable where V : BaseOpenApiReference, new()
1213
{
14+
[ThreadStatic]
15+
private static HashSet<BaseOpenApiReferenceHolder<T, U, V>>? t_activeReferenceAccesses;
16+
[ThreadStatic]
17+
private static HashSet<BaseOpenApiReferenceHolder<T, U, V>>? t_activeTargetActions;
18+
1319
/// <inheritdoc/>
1420
public virtual U? Target
1521
{
@@ -19,28 +25,129 @@ public virtual U? Target
1925
return Reference.HostDocument.ResolveReferenceTo<U>(Reference, this as IOpenApiSchema);
2026
}
2127
}
28+
29+
/// <summary>
30+
/// Gets a value from the resolved target while detecting cycles in delegated member access.
31+
/// </summary>
32+
/// <typeparam name="TResult">The type of value to get from the target.</typeparam>
33+
/// <param name="selector">Selects the value from the resolved target.</param>
34+
/// <returns>The selected value, or the default value when the target cannot be resolved.</returns>
35+
/// <remarks>
36+
/// The guard remains active while <paramref name="selector"/> reads the target member. This covers
37+
/// the complete delegated call chain without changing the immediate-resolution semantics of
38+
/// <see cref="Target"/> or walking an acyclic chain more than once.
39+
/// </remarks>
40+
private protected TResult GetFromTarget<TResult>(Func<U, TResult> selector)
41+
{
42+
Utils.CheckArgumentNull(selector);
43+
return ExecuteWithReferenceAccessGuard(this, () =>
44+
{
45+
return Target is { } target ? selector(target) : default!;
46+
});
47+
}
48+
49+
/// <summary>
50+
/// Executes an action against the resolved target while detecting cycles in delegated access.
51+
/// </summary>
52+
/// <param name="action">The action to execute against the resolved target.</param>
53+
private protected void ApplyToTarget(Action<U> action)
54+
{
55+
Utils.CheckArgumentNull(action);
56+
ExecuteWithTargetActionGuard<object?>(this, () =>
57+
{
58+
if (Target is { } target)
59+
{
60+
action(target);
61+
}
62+
return null;
63+
});
64+
}
65+
66+
private static TResult ExecuteWithReferenceAccessGuard<TResult>(
67+
BaseOpenApiReferenceHolder<T, U, V> holder,
68+
Func<TResult> action)
69+
{
70+
return ExecuteWithReferenceGuard(ref t_activeReferenceAccesses, holder, action);
71+
}
72+
73+
private static TResult ExecuteWithTargetActionGuard<TResult>(
74+
BaseOpenApiReferenceHolder<T, U, V> holder,
75+
Func<TResult> action)
76+
{
77+
return ExecuteWithReferenceGuard(ref t_activeTargetActions, holder, action);
78+
}
79+
80+
private static TResult ExecuteWithReferenceGuard<TResult>(
81+
ref HashSet<BaseOpenApiReferenceHolder<T, U, V>>? activeReferences,
82+
BaseOpenApiReferenceHolder<T, U, V> holder,
83+
Func<TResult> action)
84+
{
85+
activeReferences ??= new HashSet<BaseOpenApiReferenceHolder<T, U, V>>(ReferenceHolderComparer.Instance);
86+
if (!activeReferences.Add(holder))
87+
{
88+
throw new InvalidOperationException($"Circular reference detected while resolving reference: {holder.Reference.ReferenceV3}");
89+
}
90+
91+
try
92+
{
93+
RuntimeHelpers.EnsureSufficientExecutionStack();
94+
return action();
95+
}
96+
catch (InsufficientExecutionStackException ex)
97+
{
98+
throw new InvalidOperationException(
99+
$"The chain of references starting at {holder.Reference.ReferenceV3} is nested too deeply to resolve.",
100+
ex);
101+
}
102+
finally
103+
{
104+
activeReferences.Remove(holder);
105+
if (activeReferences.Count == 0)
106+
{
107+
activeReferences = null;
108+
}
109+
}
110+
}
111+
22112
/// <inheritdoc/>
23113
public T? RecursiveTarget
24114
{
25115
get
26116
{
27-
return ResolveRecursiveTarget(new HashSet<BaseOpenApiReferenceHolder<T, U, V>>());
117+
var visitedReferences = new HashSet<BaseOpenApiReferenceHolder<T, U, V>>(ReferenceHolderComparer.Instance);
118+
BaseOpenApiReferenceHolder<T, U, V> current = this;
119+
120+
while (visitedReferences.Add(current))
121+
{
122+
switch (current.Target)
123+
{
124+
case BaseOpenApiReferenceHolder<T, U, V> recursiveTarget:
125+
current = recursiveTarget;
126+
break;
127+
case T concrete:
128+
return concrete;
129+
default:
130+
return null;
131+
}
132+
}
133+
134+
throw new InvalidOperationException($"Circular reference detected while resolving reference: {current.Reference.ReferenceV3}");
28135
}
29136
}
30137

31-
private T? ResolveRecursiveTarget(ISet<BaseOpenApiReferenceHolder<T, U, V>> visitedReferences)
138+
private sealed class ReferenceHolderComparer : IEqualityComparer<BaseOpenApiReferenceHolder<T, U, V>>
32139
{
33-
if (!visitedReferences.Add(this))
140+
internal static ReferenceHolderComparer Instance { get; } = new();
141+
142+
public bool Equals(BaseOpenApiReferenceHolder<T, U, V>? x, BaseOpenApiReferenceHolder<T, U, V>? y)
34143
{
35-
throw new InvalidOperationException($"Circular reference detected while resolving reference: {Reference.ReferenceV3}");
144+
return ReferenceEquals(x, y);
36145
}
37146

38-
return Target switch
147+
public int GetHashCode(BaseOpenApiReferenceHolder<T, U, V> obj)
39148
{
40-
BaseOpenApiReferenceHolder<T, U, V> recursiveTarget => recursiveTarget.ResolveRecursiveTarget(visitedReferences),
41-
T concrete => concrete,
42-
_ => null
43-
};
149+
return RuntimeHelpers.GetHashCode(obj);
150+
}
44151
}
45152
/// <summary>
46153
/// Copy the reference as a target element with overrides.
@@ -147,9 +254,6 @@ private protected void SerializeInternal(IOpenApiWriter writer,
147254
Action<IOpenApiWriter, U> action)
148255
{
149256
Utils.CheckArgumentNull(writer);
150-
if (Target is not null)
151-
{
152-
action(writer, Target);
153-
}
257+
ApplyToTarget(element => action(writer, element));
154258
}
155259
}

‎src/Microsoft.OpenApi/Models/References/OpenApiCallbackReference.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ private OpenApiCallbackReference(OpenApiCallbackReference callback) : base(callb
3333
}
3434

3535
/// <inheritdoc/>
36-
public Dictionary<RuntimeExpression, IOpenApiPathItem>? PathItems { get => Target?.PathItems; }
36+
public Dictionary<RuntimeExpression, IOpenApiPathItem>? PathItems { get => GetFromTarget(static target => target.PathItems); }
3737

3838
/// <inheritdoc/>
39-
public IDictionary<string, IOpenApiExtension>? Extensions { get => Target?.Extensions; }
39+
public IDictionary<string, IOpenApiExtension>? Extensions { get => GetFromTarget(static target => target.Extensions); }
4040

4141
/// <inheritdoc/>
4242
public override IOpenApiCallback CopyReferenceAsTargetElementWithOverrides(IOpenApiCallback source)

‎src/Microsoft.OpenApi/Models/References/OpenApiExampleReference.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,25 @@ private OpenApiExampleReference(OpenApiExampleReference example) : base(example)
3535
/// <inheritdoc/>
3636
public string? Description
3737
{
38-
get => string.IsNullOrEmpty(Reference.Description) ? Target?.Description : Reference.Description;
38+
get => string.IsNullOrEmpty(Reference.Description) ? GetFromTarget(static target => target.Description) : Reference.Description;
3939
set => Reference.Description = value;
4040
}
4141

4242
/// <inheritdoc/>
4343
public string? Summary
4444
{
45-
get => string.IsNullOrEmpty(Reference.Summary) ? Target?.Summary : Reference.Summary;
45+
get => string.IsNullOrEmpty(Reference.Summary) ? GetFromTarget(static target => target.Summary) : Reference.Summary;
4646
set => Reference.Summary = value;
4747
}
4848

4949
/// <inheritdoc/>
50-
public IDictionary<string, IOpenApiExtension>? Extensions { get => Target?.Extensions; }
50+
public IDictionary<string, IOpenApiExtension>? Extensions { get => GetFromTarget(static target => target.Extensions); }
5151

5252
/// <inheritdoc/>
53-
public string? ExternalValue { get => Target?.ExternalValue; }
53+
public string? ExternalValue { get => GetFromTarget(static target => target.ExternalValue); }
5454

5555
/// <inheritdoc/>
56-
public JsonNode? Value { get => Target?.Value; }
56+
public JsonNode? Value { get => GetFromTarget(static target => target.Value); }
5757

5858
/// <inheritdoc/>
5959
public override IOpenApiExample CopyReferenceAsTargetElementWithOverrides(IOpenApiExample source)

‎src/Microsoft.OpenApi/Models/References/OpenApiHeaderReference.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,42 +36,42 @@ private OpenApiHeaderReference(OpenApiHeaderReference header) : base(header)
3636
/// <inheritdoc/>
3737
public string? Description
3838
{
39-
get => string.IsNullOrEmpty(Reference.Description) ? Target?.Description : Reference.Description;
39+
get => string.IsNullOrEmpty(Reference.Description) ? GetFromTarget(static target => target.Description) : Reference.Description;
4040
set => Reference.Description = value;
4141
}
4242

4343
/// <inheritdoc/>
44-
public bool Required { get => Target?.Required ?? default; }
44+
public bool Required { get => GetFromTarget(static target => target.Required); }
4545

4646
/// <inheritdoc/>
47-
public bool Deprecated { get => Target?.Deprecated ?? default; }
47+
public bool Deprecated { get => GetFromTarget(static target => target.Deprecated); }
4848

4949
/// <inheritdoc/>
50-
public bool AllowEmptyValue { get => Target?.AllowEmptyValue ?? default; }
50+
public bool AllowEmptyValue { get => GetFromTarget(static target => target.AllowEmptyValue); }
5151

5252
/// <inheritdoc/>
53-
public IOpenApiSchema? Schema { get => Target?.Schema; }
53+
public IOpenApiSchema? Schema { get => GetFromTarget(static target => target.Schema); }
5454

5555
/// <inheritdoc/>
56-
public ParameterStyle? Style { get => Target?.Style; }
56+
public ParameterStyle? Style { get => GetFromTarget(static target => target.Style); }
5757

5858
/// <inheritdoc/>
59-
public bool Explode { get => Target?.Explode ?? default; }
59+
public bool Explode { get => GetFromTarget(static target => target.Explode); }
6060

6161
/// <inheritdoc/>
62-
public bool AllowReserved { get => Target?.AllowReserved ?? default; }
62+
public bool AllowReserved { get => GetFromTarget(static target => target.AllowReserved); }
6363

6464
/// <inheritdoc/>
65-
public JsonNode? Example { get => Target?.Example; }
65+
public JsonNode? Example { get => GetFromTarget(static target => target.Example); }
6666

6767
/// <inheritdoc/>
68-
public IDictionary<string, IOpenApiExample>? Examples { get => Target?.Examples; }
68+
public IDictionary<string, IOpenApiExample>? Examples { get => GetFromTarget(static target => target.Examples); }
6969

7070
/// <inheritdoc/>
71-
public IDictionary<string, OpenApiMediaType>? Content { get => Target?.Content; }
71+
public IDictionary<string, OpenApiMediaType>? Content { get => GetFromTarget(static target => target.Content); }
7272

7373
/// <inheritdoc/>
74-
public IDictionary<string, IOpenApiExtension>? Extensions { get => Target?.Extensions; }
74+
public IDictionary<string, IOpenApiExtension>? Extensions { get => GetFromTarget(static target => target.Extensions); }
7575

7676
/// <inheritdoc/>
7777
public override IOpenApiHeader CopyReferenceAsTargetElementWithOverrides(IOpenApiHeader source)

‎src/Microsoft.OpenApi/Models/References/OpenApiLinkReference.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,27 @@ private OpenApiLinkReference(OpenApiLinkReference reference) : base(reference)
3434
/// <inheritdoc/>
3535
public string? Description
3636
{
37-
get => string.IsNullOrEmpty(Reference.Description) ? Target?.Description : Reference.Description;
37+
get => string.IsNullOrEmpty(Reference.Description) ? GetFromTarget(static target => target.Description) : Reference.Description;
3838
set => Reference.Description = value;
3939
}
4040

4141
/// <inheritdoc/>
42-
public string? OperationRef { get => Target?.OperationRef; }
42+
public string? OperationRef { get => GetFromTarget(static target => target.OperationRef); }
4343

4444
/// <inheritdoc/>
45-
public string? OperationId { get => Target?.OperationId; }
45+
public string? OperationId { get => GetFromTarget(static target => target.OperationId); }
4646

4747
/// <inheritdoc/>
48-
public OpenApiServer? Server { get => Target?.Server; }
48+
public OpenApiServer? Server { get => GetFromTarget(static target => target.Server); }
4949

5050
/// <inheritdoc/>
51-
public IDictionary<string, RuntimeExpressionAnyWrapper>? Parameters { get => Target?.Parameters; }
51+
public IDictionary<string, RuntimeExpressionAnyWrapper>? Parameters { get => GetFromTarget(static target => target.Parameters); }
5252

5353
/// <inheritdoc/>
54-
public RuntimeExpressionAnyWrapper? RequestBody { get => Target?.RequestBody; }
54+
public RuntimeExpressionAnyWrapper? RequestBody { get => GetFromTarget(static target => target.RequestBody); }
5555

5656
/// <inheritdoc/>
57-
public IDictionary<string, IOpenApiExtension>? Extensions { get => Target?.Extensions; }
57+
public IDictionary<string, IOpenApiExtension>? Extensions { get => GetFromTarget(static target => target.Extensions); }
5858

5959
/// <inheritdoc/>
6060
public override void SerializeAsV2(IOpenApiWriter writer)

‎src/Microsoft.OpenApi/Models/References/OpenApiParameterReference.cs‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,51 +36,51 @@ private OpenApiParameterReference(OpenApiParameterReference parameter):base(para
3636
}
3737

3838
/// <inheritdoc/>
39-
public string? Name { get => Target?.Name; }
39+
public string? Name { get => GetFromTarget(static target => target.Name); }
4040

4141
/// <inheritdoc/>
4242
public string? Description
4343
{
44-
get => string.IsNullOrEmpty(Reference.Description) ? Target?.Description : Reference.Description;
44+
get => string.IsNullOrEmpty(Reference.Description) ? GetFromTarget(static target => target.Description) : Reference.Description;
4545
set => Reference.Description = value;
4646
}
4747

4848
/// <inheritdoc/>
49-
public bool Required { get => Target?.Required ?? default; }
49+
public bool Required { get => GetFromTarget(static target => target.Required); }
5050

5151
/// <inheritdoc/>
52-
public bool Deprecated { get => Target?.Deprecated ?? default; }
52+
public bool Deprecated { get => GetFromTarget(static target => target.Deprecated); }
5353

5454
/// <inheritdoc/>
5555
[Obsolete("Use of AllowEmptyValue is not recommended and it is likely to be removed in a later revision.")]
56-
public bool AllowEmptyValue { get => Target?.AllowEmptyValue ?? default; }
56+
public bool AllowEmptyValue { get => GetFromTarget(static target => target.AllowEmptyValue); }
5757

5858
/// <inheritdoc/>
59-
public bool AllowReserved { get => Target?.AllowReserved ?? default; }
59+
public bool AllowReserved { get => GetFromTarget(static target => target.AllowReserved); }
6060

6161
/// <inheritdoc/>
62-
public IOpenApiSchema? Schema { get => Target?.Schema; }
62+
public IOpenApiSchema? Schema { get => GetFromTarget(static target => target.Schema); }
6363

6464
/// <inheritdoc/>
65-
public IDictionary<string, IOpenApiExample>? Examples { get => Target?.Examples; }
65+
public IDictionary<string, IOpenApiExample>? Examples { get => GetFromTarget(static target => target.Examples); }
6666

6767
/// <inheritdoc/>
68-
public JsonNode? Example { get => Target?.Example; }
68+
public JsonNode? Example { get => GetFromTarget(static target => target.Example); }
6969

7070
/// <inheritdoc/>
71-
public ParameterLocation? In { get => Target?.In; }
71+
public ParameterLocation? In { get => GetFromTarget(static target => target.In); }
7272

7373
/// <inheritdoc/>
74-
public ParameterStyle? Style { get => Target?.Style; }
74+
public ParameterStyle? Style { get => GetFromTarget(static target => target.Style); }
7575

7676
/// <inheritdoc/>
77-
public bool Explode { get => Target?.Explode ?? default; }
77+
public bool Explode { get => GetFromTarget(static target => target.Explode); }
7878

7979
/// <inheritdoc/>
80-
public IDictionary<string, OpenApiMediaType>? Content { get => Target?.Content; }
80+
public IDictionary<string, OpenApiMediaType>? Content { get => GetFromTarget(static target => target.Content); }
8181

8282
/// <inheritdoc/>
83-
public IDictionary<string, IOpenApiExtension>? Extensions { get => Target?.Extensions; }
83+
public IDictionary<string, IOpenApiExtension>? Extensions { get => GetFromTarget(static target => target.Extensions); }
8484

8585
/// <inheritdoc/>
8686
public override IOpenApiParameter CopyReferenceAsTargetElementWithOverrides(IOpenApiParameter source)

0 commit comments

Comments
 (0)