Skip to content

avoid stack overflow on cyclical references #2405

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.

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
merged 5 commits into from
Jun 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions src/Microsoft.OpenApi/Models/OpenApiDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@
/// <summary>
/// Serialize <see cref="OpenApiDocument"/> to OpenAPI object V2.0.
/// </summary>
public void SerializeAsV2(IOpenApiWriter writer)

Check warning on line 262 in src/Microsoft.OpenApi/Models/OpenApiDocument.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 31 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
Utils.CheckArgumentNull(writer);

Expand Down Expand Up @@ -408,7 +408,7 @@
return server.ReplaceServerUrlVariables([]);
}

private static void WriteHostInfoV2(IOpenApiWriter writer, IList<OpenApiServer>? servers)

Check warning on line 411 in src/Microsoft.OpenApi/Models/OpenApiDocument.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (servers == null || !servers.Any())
{
Expand Down Expand Up @@ -477,7 +477,7 @@
// schemes
writer.WriteOptionalCollection(OpenApiConstants.Schemes, schemes, (w, s) =>
{
if(!string.IsNullOrEmpty(s) && s is not null)

Check warning on line 480 in src/Microsoft.OpenApi/Models/OpenApiDocument.cs

View workflow job for this annotation

GitHub Actions / Build

Change this condition so that it does not always evaluate to 'True'. (https://rules.sonarsource.com/csharp/RSPEC-2589)
{
w.WriteValue(s);
}
Expand All @@ -498,10 +498,10 @@
/// <summary>
/// Load the referenced <see cref="IOpenApiReferenceable"/> object from a <see cref="BaseOpenApiReference"/> object
/// </summary>
internal T? ResolveReferenceTo<T>(BaseOpenApiReference reference) where T : IOpenApiReferenceable
internal T? ResolveReferenceTo<T>(BaseOpenApiReference reference, IOpenApiSchema? parentSchema) where T : IOpenApiReferenceable
{

if (ResolveReference(reference, reference.IsExternal) is T result)
if (ResolveReference(reference, reference.IsExternal, parentSchema) is T result)
{
return result;
}
Expand Down Expand Up @@ -538,7 +538,7 @@

return ConvertByteArrayToString(hash ?? []);

async Task WriteDocumentAsync(TextWriter writer, CancellationToken token)

Check warning on line 541 in src/Microsoft.OpenApi/Models/OpenApiDocument.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this unused method parameter 'token'. (https://rules.sonarsource.com/csharp/RSPEC-1172)
{
var openApiJsonWriter = new OpenApiJsonWriter(writer, new() { Terse = true });
SerializeAsV31(openApiJsonWriter);
Expand Down Expand Up @@ -566,7 +566,7 @@
/// <summary>
/// Load the referenced <see cref="IOpenApiReferenceable"/> object from a <see cref="BaseOpenApiReference"/> object
/// </summary>
internal IOpenApiReferenceable? ResolveReference(BaseOpenApiReference? reference, bool useExternal)
internal IOpenApiReferenceable? ResolveReference(BaseOpenApiReference? reference, bool useExternal, IOpenApiSchema? parentSchema)

Check warning on line 569 in src/Microsoft.OpenApi/Models/OpenApiDocument.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (reference == null)
{
Expand Down Expand Up @@ -621,9 +621,9 @@
false => new Uri(uriLocation).AbsoluteUri
};

if (reference.Type is ReferenceType.Schema && absoluteUri.Contains('#'))
if (reference.Type is ReferenceType.Schema && absoluteUri.Contains('#') && parentSchema is not null)
{
return Workspace?.ResolveJsonSchemaReference(absoluteUri);
return Workspace?.ResolveJsonSchemaReference(absoluteUri, parentSchema);
}

return Workspace?.ResolveReference<IOpenApiReferenceable>(absoluteUri);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
/// <typeparam name="T">The concrete class implementation type for the model.</typeparam>
/// <typeparam name="U">The interface type for the model.</typeparam>
/// <typeparam name="V">The type for the reference holding the additional fields and annotations</typeparam>
public abstract class BaseOpenApiReferenceHolder<T, U, V> : IOpenApiReferenceHolder<T, U, V> where T : class, IOpenApiReferenceable, U where U : IOpenApiReferenceable, IOpenApiSerializable where V : BaseOpenApiReference, new()

Check warning on line 10 in src/Microsoft.OpenApi/Models/References/BaseOpenApiReferenceHolder.cs

View workflow job for this annotation

GitHub Actions / Build

Reduce the number of generic parameters in the 'BaseOpenApiReferenceHolder' class to no more than the 2 authorized. (https://rules.sonarsource.com/csharp/RSPEC-2436)
{
/// <inheritdoc/>
public virtual U? Target
Expand All @@ -15,7 +15,7 @@
get
{
if (Reference.HostDocument is null) return default;
return Reference.HostDocument.ResolveReferenceTo<U>(Reference);
return Reference.HostDocument.ResolveReferenceTo<U>(Reference, this as IOpenApiSchema);
}
}
/// <inheritdoc/>
Expand Down Expand Up @@ -44,7 +44,7 @@
protected BaseOpenApiReferenceHolder(BaseOpenApiReferenceHolder<T, U, V> source)
{
Utils.CheckArgumentNull(source);
Reference = CopyReference(source.Reference);

Check warning on line 47 in src/Microsoft.OpenApi/Models/References/BaseOpenApiReferenceHolder.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this call from a constructor to the overridable 'CopyReference' method. (https://rules.sonarsource.com/csharp/RSPEC-1699)
//no need to copy summary and description as if they are not overridden, they will be fetched from the target
//if they are, the reference copy will handle it
}
Expand Down
23 changes: 16 additions & 7 deletions src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
/// Registers a document's components into the workspace
/// </summary>
/// <param name="document"></param>
public void RegisterComponents(OpenApiDocument document)

Check warning on line 84 in src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 31 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (document?.Components == null) return;

Expand Down Expand Up @@ -268,7 +268,7 @@
/// <param name="value"></param>
public void AddDocumentId(string? key, Uri? value)
{
if (!string.IsNullOrEmpty(key) && key is not null && value is not null && !_documentsIdRegistry.ContainsKey(key))

Check warning on line 271 in src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs

View workflow job for this annotation

GitHub Actions / Build

Change this condition so that it does not always evaluate to 'True'. (https://rules.sonarsource.com/csharp/RSPEC-2589)
{
_documentsIdRegistry[key] = value;
}
Expand Down Expand Up @@ -331,8 +331,9 @@
/// Recursively resolves a schema from a URI fragment.
/// </summary>
/// <param name="location"></param>
/// <param name="parentSchema">The parent schema to resolve against.</param>
/// <returns></returns>
internal IOpenApiSchema? ResolveJsonSchemaReference(string location)
internal IOpenApiSchema? ResolveJsonSchemaReference(string location, IOpenApiSchema parentSchema)
{
/* Enables resolving references for nested subschemas
* Examples:
Expand Down Expand Up @@ -362,14 +363,22 @@
{
// traverse remaining segments after fetching the base schema
var remainingSegments = pathSegments.Skip(4).ToArray();
return ResolveSubSchema(targetSchema, remainingSegments);
var stack = new Stack<IOpenApiSchema>();
stack.Push(parentSchema);
return ResolveSubSchema(targetSchema, remainingSegments, stack);
}

return default;
}

internal static IOpenApiSchema? ResolveSubSchema(IOpenApiSchema schema, string[] pathSegments)
internal static IOpenApiSchema? ResolveSubSchema(IOpenApiSchema schema, string[] pathSegments, Stack<IOpenApiSchema> visitedSchemas)

Check warning on line 374 in src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
// Prevent infinite recursion in case of circular references
if (visitedSchemas.Contains(schema))
{
return null;
}
visitedSchemas.Push(schema);
// Traverse schema object to resolve subschemas
if (pathSegments.Length == 0)
{
Expand All @@ -383,13 +392,13 @@
case OpenApiConstants.Properties:
var propName = pathSegments[0];
if (schema.Properties != null && schema.Properties.TryGetValue(propName, out var propSchema))
return ResolveSubSchema(propSchema, [.. pathSegments.Skip(1)]);
return ResolveSubSchema(propSchema, [.. pathSegments.Skip(1)], visitedSchemas);
break;
case OpenApiConstants.Items:
return schema.Items is OpenApiSchema itemsSchema ? ResolveSubSchema(itemsSchema, pathSegments) : null;
return schema.Items is OpenApiSchema itemsSchema ? ResolveSubSchema(itemsSchema, pathSegments, visitedSchemas) : null;

case OpenApiConstants.AdditionalProperties:
return schema.AdditionalProperties is OpenApiSchema additionalSchema ? ResolveSubSchema(additionalSchema, pathSegments) : null;
return schema.AdditionalProperties is OpenApiSchema additionalSchema ? ResolveSubSchema(additionalSchema, pathSegments, visitedSchemas) : null;
case OpenApiConstants.AllOf:
case OpenApiConstants.AnyOf:
case OpenApiConstants.OneOf:
Expand All @@ -405,7 +414,7 @@

// recurse into the indexed subschema if valid
if (list != null && index < list.Count)
return ResolveSubSchema(list[index], [.. pathSegments.Skip(1)]);
return ResolveSubSchema(list[index], [.. pathSegments.Skip(1)], visitedSchemas);
break;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public async Task ShouldAllowComponentsThatJustContainAReference()
var schema1 = actual.Components.Schemas["AllPets"];
var schema1Reference = Assert.IsType<OpenApiSchemaReference>(schema1);
Assert.False(schema1Reference.UnresolvedReference);
var schema2 = actual.ResolveReferenceTo<OpenApiSchema>(schema1Reference.Reference);
var schema2 = actual.ResolveReferenceTo<OpenApiSchema>(schema1Reference.Reference, schema1Reference);
Assert.IsType<OpenApiSchema>(schema2);
if (string.IsNullOrEmpty(schema1Reference.Reference.Id) || schema1Reference.UnresolvedReference)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public void ResolveSubSchema_ShouldTraverseKnownKeywords()

var path = new[] { "properties", "a", "properties", "b" };

var result = OpenApiWorkspace.ResolveSubSchema(schema, path);
var result = OpenApiWorkspace.ResolveSubSchema(schema, path, []);

Assert.NotNull(result);
Assert.Equal(JsonSchemaType.String, result!.Type);
Expand All @@ -250,7 +250,7 @@ public void ResolveSubSchema_ShouldHandleUserDefinedKeywordNamedProperty(string[
}
};

var result = OpenApiWorkspace.ResolveSubSchema(schema, pathSegments);
var result = OpenApiWorkspace.ResolveSubSchema(schema, pathSegments, []);

Assert.NotNull(result);
Assert.Equal(JsonSchemaType.String, result!.Type);
Expand All @@ -275,7 +275,7 @@ public void ResolveSubSchema_ShouldRecurseIntoAllOfComposition()

var path = new[] { "allOf", "0", "properties", "x" };

var result = OpenApiWorkspace.ResolveSubSchema(schema, path);
var result = OpenApiWorkspace.ResolveSubSchema(schema, path, []);

Assert.NotNull(result);
Assert.Equal(JsonSchemaType.Integer, result!.Type);
Expand Down Expand Up @@ -434,6 +434,81 @@ public async Task ShouldResolveReferencesInSchemasFromSystemTextJson()
var updatedTagsProperty = Assert.IsType<OpenApiSchemaReference>(schema.Properties["tags"]);
Assert.Equal(absoluteReferenceId, updatedTagsProperty.Reference.ReferenceV3);
Assert.Equal(JsonSchemaType.Array | JsonSchemaType.Null, updatedTagsProperty.Type);
Assert.Equal(JsonSchemaType.Object, updatedTagsProperty.Items.Type);


// doing the same for the parent property

var parentProperty = Assert.IsType<OpenApiSchema>(schema.Properties["parent"]);
var parentSubProperty = Assert.IsType<OpenApiSchemaReference>(parentProperty.Properties["parent"]);
Assert.Equal("#/properties/parent", parentSubProperty.Reference.ReferenceV3);
parentProperty.Properties["parent"] = new OpenApiSchemaReference($"#/components/schemas/Foo{parentSubProperty.Reference.ReferenceV3.Replace("#", string.Empty)}", document);
var updatedParentSubProperty = Assert.IsType<OpenApiSchemaReference>(parentProperty.Properties["parent"]);
Assert.Equal(JsonSchemaType.Object | JsonSchemaType.Null, updatedParentSubProperty.Type);

var pathItem = new OpenApiPathItem
{
Operations = new Dictionary<HttpMethod, OpenApiOperation>
{
[HttpMethod.Post] = new OpenApiOperation
{
Responses = new OpenApiResponses
{
["200"] = new OpenApiResponse
{
}
},
RequestBody = new OpenApiRequestBody
{
Content = new Dictionary<string, OpenApiMediaType>
{
["application/json"] = new OpenApiMediaType
{
Schema = new OpenApiSchemaReference("#/components/schemas/Foo", document)
}
}
}
}
}
};
document.Paths.Add("/", pathItem);

var requestBodySchema = pathItem.Operations[HttpMethod.Post].RequestBody.Content["application/json"].Schema;
Assert.NotNull(requestBodySchema);
var requestBodyTagsProperty = Assert.IsType<OpenApiSchemaReference>(requestBodySchema.Properties["tags"]);
Assert.Equal(JsonSchemaType.Object, requestBodyTagsProperty.Items.Type);
}

[Fact]
public void ExitsEarlyOnCyclicalReferences()
{
var document = new OpenApiDocument
{
Info = new OpenApiInfo { Title = "Test API", Version = "1.0.0" },
};
var categorySchema = new OpenApiSchema
{
Type = JsonSchemaType.Object,
Properties = new Dictionary<string, IOpenApiSchema>
{
["name"] = new OpenApiSchema { Type = JsonSchemaType.String },
["parent"] = new OpenApiSchemaReference("#/components/schemas/Category", document),
// this is intentionally wrong and cyclical reference
// it tests whether we're going in an infinite resolution loop
["tags"] = new OpenApiSchemaReference("#/components/schemas/Category/properties/parent/properties/tags", document)
}
};
document.AddComponent("Category", categorySchema);
document.RegisterComponents();

var tagsSchemaRef = Assert.IsType<OpenApiSchemaReference>(categorySchema.Properties["tags"]);
Assert.Null(tagsSchemaRef.Items);
Assert.Equal("#/components/schemas/Category/properties/parent/properties/tags", tagsSchemaRef.Reference.ReferenceV3);
Assert.Null(tagsSchemaRef.Target);

var parentSchemaRef = Assert.IsType<OpenApiSchemaReference>(categorySchema.Properties["parent"]);
Assert.Equal("#/components/schemas/Category", parentSchemaRef.Reference.ReferenceV3);
Assert.NotNull(parentSchemaRef.Target);
}
}
}