Skip to content

Commit

Permalink
Merge pull request #5370 from microsoft/fix/stack-overflow
Browse files Browse the repository at this point in the history
fix/stack overflow
  • Loading branch information
baywet authored Sep 10, 2024
2 parents bb90c48 + 6eecf1a commit 7a39ecf
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 27 deletions.
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)
- Fixed a bug where function descriptions in plugin manifest defaults to path summary instead of description[#5301](https://github.com/microsoft/kiota/issues/5301)
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 @@ -27,7 +27,9 @@ public OpenApiSchemaComparer(
/// <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);
}
/// <inheritdoc/>
public int GetHashCode([DisallowNull] OpenApiSchema obj)
Expand All @@ -37,25 +39,6 @@ public int GetHashCode([DisallowNull] OpenApiSchema obj)
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 @@ internal class OpenApiDiscriminatorComparer : IEqualityComparer<OpenApiDiscrimin
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 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 @@ -23,8 +23,7 @@ public virtual bool Equals(T? x, T? y)
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
18 changes: 18 additions & 0 deletions tests/Kiota.Builder.Tests/Validation/OpenApiSchemaComparerTests.cs
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));
}
}

0 comments on commit 7a39ecf

Please sign in to comment.