Skip to content
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

fix/stack overflow #5370

Merged
merged 3 commits into from
Sep 10, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Fixed a stack overflow in the core generator caused by circular comparisons. [#5369](https://github.com/microsoft/kiota/issues/5369)
- Fixed a bug where collection/array of primitive types members for union/intersection types would be ignored. [#5283](https://github.com/microsoft/kiota/issues/5283)
- Fixed a when generating a plugin when only an operation is selected in the root node in the extension. [#5300](https://github.com/microsoft/kiota/issues/5300)

Expand Down
6 changes: 4 additions & 2 deletions src/Kiota.Builder/Manifest/ApiDependencyComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ public bool Equals(ApiDependency? x, ApiDependency? y)

string? xExtensions = GetDependencyExtensionsValue(x), yExtensions = GetDependencyExtensionsValue(y);
// Assume requests are equal if we aren't comparing them.
var requestsEqual = !_compareRequests || x.Requests.SequenceEqual(y.Requests, _requestInfoComparer);
var requestsEqual = !_compareRequests || GetOrderedRequests(x.Requests).SequenceEqual(GetOrderedRequests(y.Requests), _requestInfoComparer);
return _stringComparer.Equals(xExtensions, yExtensions)
&& requestsEqual;
}
private static IOrderedEnumerable<RequestInfo> GetOrderedRequests(IList<RequestInfo> requests) =>
requests.OrderBy(x => x.UriTemplate, StringComparer.Ordinal).ThenBy(x => x.Method, StringComparer.Ordinal);
/// <inheritdoc/>
public int GetHashCode([DisallowNull] ApiDependency obj)
{
Expand All @@ -56,7 +58,7 @@ public int GetHashCode([DisallowNull] ApiDependency obj)
hash.Add(GetDependencyExtensionsValue(obj) ?? string.Empty, _stringComparer);
if (!_compareRequests) return hash.ToHashCode();

foreach (var request in obj.Requests)
foreach (var request in GetOrderedRequests(obj.Requests))
{
hash.Add(request, _requestInfoComparer);
}
Expand Down
29 changes: 7 additions & 22 deletions src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
{
private readonly OpenApiDiscriminatorComparer discriminatorComparer;
private readonly OpenApiAnyComparer openApiAnyComparer;
private readonly KeyValueComparer<string, OpenApiSchema> schemaMapComparer;

Check warning on line 15 in src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this unread private field 'schemaMapComparer' or refactor the code to use its value. (https://rules.sonarsource.com/csharp/RSPEC-4487)

Check warning on line 15 in src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this unread private field 'schemaMapComparer' or refactor the code to use its value. (https://rules.sonarsource.com/csharp/RSPEC-4487)

public OpenApiSchemaComparer(
OpenApiDiscriminatorComparer? discriminatorComparer = null,
Expand All @@ -27,7 +27,9 @@
/// <inheritdoc/>
public bool Equals(OpenApiSchema? x, OpenApiSchema? y)
{
return EqualsInternal(x, y);
// this workaround might result in collisions, however so far this has not been a problem
// implemented this way to avoid stack overflow caused by schemas referencing themselves
return x == null && y == null || x != null && y != null && GetHashCode(x) == GetHashCode(y);
baywet marked this conversation as resolved.
Show resolved Hide resolved
}
/// <inheritdoc/>
public int GetHashCode([DisallowNull] OpenApiSchema obj)
Expand All @@ -37,25 +39,6 @@
return hash.ToHashCode();
}

private bool EqualsInternal(OpenApiSchema? x, OpenApiSchema? y)
{
if (x is null || y is null) return object.Equals(x, y);
return x.Deprecated == y.Deprecated
&& x.Nullable == y.Nullable
&& x.AdditionalPropertiesAllowed == y.AdditionalPropertiesAllowed
&& discriminatorComparer.Equals(x.Discriminator, y.Discriminator)
&& string.Equals(x.Format, y.Format, StringComparison.OrdinalIgnoreCase)
&& string.Equals(x.Type, y.Type, StringComparison.OrdinalIgnoreCase)
&& string.Equals(x.Title, y.Title, StringComparison.Ordinal)
&& openApiAnyComparer.Equals(x.Default, y.Default)
&& EqualsInternal(x.AdditionalProperties, y.AdditionalProperties)
&& EqualsInternal(x.Items, y.Items)
&& Enumerable.SequenceEqual(x.Properties, y.Properties, schemaMapComparer)
&& Enumerable.SequenceEqual(x.AnyOf, y.AnyOf, this)
&& Enumerable.SequenceEqual(x.AllOf, y.AllOf, this)
&& Enumerable.SequenceEqual(x.OneOf, y.OneOf, this);
}

private void GetHashCodeInternal([DisallowNull] OpenApiSchema obj, HashSet<OpenApiSchema> visitedSchemas, ref HashCode hash)
{
if (obj is null) return;
Expand Down Expand Up @@ -143,15 +126,17 @@
public bool Equals(OpenApiDiscriminator? x, OpenApiDiscriminator? y)
{
if (x is null || y is null) return object.Equals(x, y);
return x.PropertyName.EqualsIgnoreCase(y.PropertyName) && x.Mapping.SequenceEqual(y.Mapping, mappingComparer);
return x.PropertyName.EqualsIgnoreCase(y.PropertyName) && GetOrderedRequests(x.Mapping).SequenceEqual(GetOrderedRequests(y.Mapping), mappingComparer);
}
private static IOrderedEnumerable<KeyValuePair<string, string>> GetOrderedRequests(IDictionary<string, string> mappings) =>
mappings.OrderBy(x => x.Key, StringComparer.Ordinal);
/// <inheritdoc/>
public int GetHashCode([DisallowNull] OpenApiDiscriminator obj)
{
var hash = new HashCode();
if (obj == null) return hash.ToHashCode();
hash.Add(obj.PropertyName);
foreach (var mapping in obj.Mapping)
foreach (var mapping in GetOrderedRequests(obj.Mapping))
{
hash.Add(mapping, mappingComparer);
}
Expand All @@ -164,7 +149,7 @@
public bool Equals(IOpenApiAny? x, IOpenApiAny? y)
{
if (x is null || y is null) return object.Equals(x, y);
// TODO: Can we use the OpenAPI.NET implementation of Equals?

Check warning on line 152 in src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)

Check warning on line 152 in src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
return x.AnyType == y.AnyType && string.Equals(x.ToString(), y.ToString(), StringComparison.OrdinalIgnoreCase);
}
/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public ApiPluginConfigurationComparer(StringIEnumerableDeepComparer? stringIEnum
public override bool Equals(ApiPluginConfiguration? x, ApiPluginConfiguration? y)
{
if (x is null || y is null) return object.Equals(x, y);
return x.Types.SequenceEqual(y.Types, _stringComparer) && base.Equals(x, y);
return _stringIEnumerableDeepComparer.Equals(x.Types, y.Types) && base.Equals(x, y);
}

/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
private readonly StringIEnumerableDeepComparer _stringIEnumerableDeepComparer;
private readonly StringComparer _stringComparer;

internal BaseApiConsumerConfigurationComparer(StringIEnumerableDeepComparer? stringIEnumerableDeepComparer = null,

Check warning on line 13 in src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfigurationComparer.cs

View workflow job for this annotation

GitHub Actions / Build

Change the visibility of this constructor to 'private protected'. (https://rules.sonarsource.com/csharp/RSPEC-3442)

Check warning on line 13 in src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfigurationComparer.cs

View workflow job for this annotation

GitHub Actions / Build

Change the visibility of this constructor to 'private protected'. (https://rules.sonarsource.com/csharp/RSPEC-3442)
StringComparer? stringComparer = null)
{
_stringComparer = stringComparer ?? StringComparer.OrdinalIgnoreCase;
Expand All @@ -23,8 +23,7 @@
if (x is null || y is null) return object.Equals(x, y);
return _stringComparer.Equals(x.DescriptionLocation, y.DescriptionLocation)
&& _stringComparer.Equals(x.OutputPath, y.OutputPath)
&& x.IncludePatterns.SequenceEqual(y.IncludePatterns, _stringComparer)
&& x.ExcludePatterns.SequenceEqual(y.ExcludePatterns, _stringComparer);
&& _stringIEnumerableDeepComparer.Equals(x.IncludePatterns, y.IncludePatterns);
}

public virtual int GetHashCode([DisallowNull] T obj)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using Kiota.Builder.Validation;
using Microsoft.OpenApi.Models;
using Xunit;

namespace Kiota.Builder.Tests.Validation;
Expand All @@ -21,4 +22,21 @@ public void TestEquals()
{
Assert.True(_comparer.Equals(new(), new()));
}
[Fact]
public void DoesNotStackOverFlowOnCircularReferencesForEquals()
{
var schema = new OpenApiSchema
{

};
schema.Properties.Add("test", schema);
schema.AnyOf.Add(schema);
var schema2 = new OpenApiSchema
{

};
schema2.Properties.Add("test", schema2);
schema2.AnyOf.Add(schema2);
Assert.True(_comparer.Equals(schema, schema2));
}
}
Loading