From c7f84acac4bab6d54d738c14412a344301b83e3a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 9 Sep 2024 15:05:27 -0400 Subject: [PATCH 1/3] fix: stack overflow in comparer Signed-off-by: Vincent Biret --- CHANGELOG.md | 1 + .../Validation/OpenApiSchemaComparer.cs | 21 +------------------ .../Validation/OpenApiSchemaComparerTests.cs | 18 ++++++++++++++++ 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a5948d2a4..0cc6e99f61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs b/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs index 4a2bd33e90..c6bef7f56e 100644 --- a/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs +++ b/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs @@ -27,7 +27,7 @@ public OpenApiSchemaComparer( /// public bool Equals(OpenApiSchema? x, OpenApiSchema? y) { - return EqualsInternal(x, y); + return x == null && y == null || x != null && y != null && GetHashCode(x) == GetHashCode(y); } /// public int GetHashCode([DisallowNull] OpenApiSchema obj) @@ -37,25 +37,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 visitedSchemas, ref HashCode hash) { if (obj is null) return; diff --git a/tests/Kiota.Builder.Tests/Validation/OpenApiSchemaComparerTests.cs b/tests/Kiota.Builder.Tests/Validation/OpenApiSchemaComparerTests.cs index e89c849234..e794eb3f3d 100644 --- a/tests/Kiota.Builder.Tests/Validation/OpenApiSchemaComparerTests.cs +++ b/tests/Kiota.Builder.Tests/Validation/OpenApiSchemaComparerTests.cs @@ -1,5 +1,6 @@ using System; using Kiota.Builder.Validation; +using Microsoft.OpenApi.Models; using Xunit; namespace Kiota.Builder.Tests.Validation; @@ -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)); + } } From 30d9c346e1170e950ec7703768df008a8362183f Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 9 Sep 2024 15:30:58 -0400 Subject: [PATCH 2/3] fix: order does not matter in comparison Signed-off-by: Vincent Biret --- src/Kiota.Builder/Manifest/ApiDependencyComparer.cs | 6 ++++-- src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs | 6 ++++-- .../WorkspaceManagement/ApiPluginConfigurationComparer.cs | 2 +- .../BaseApiConsumerConfigurationComparer.cs | 3 +-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Kiota.Builder/Manifest/ApiDependencyComparer.cs b/src/Kiota.Builder/Manifest/ApiDependencyComparer.cs index 7158778755..c9fe09588d 100644 --- a/src/Kiota.Builder/Manifest/ApiDependencyComparer.cs +++ b/src/Kiota.Builder/Manifest/ApiDependencyComparer.cs @@ -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 GetOrderedRequests(IList requests) => + requests.OrderBy(x => x.UriTemplate, StringComparer.Ordinal).ThenBy(x => x.Method, StringComparer.Ordinal); /// public int GetHashCode([DisallowNull] ApiDependency obj) { @@ -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); } diff --git a/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs b/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs index c6bef7f56e..e3bcf76491 100644 --- a/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs +++ b/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs @@ -124,15 +124,17 @@ internal class OpenApiDiscriminatorComparer : IEqualityComparer> GetOrderedRequests(IDictionary mappings) => + mappings.OrderBy(x => x.Key, StringComparer.Ordinal); /// 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); } diff --git a/src/Kiota.Builder/WorkspaceManagement/ApiPluginConfigurationComparer.cs b/src/Kiota.Builder/WorkspaceManagement/ApiPluginConfigurationComparer.cs index afbbf16568..1c00d9bc5f 100644 --- a/src/Kiota.Builder/WorkspaceManagement/ApiPluginConfigurationComparer.cs +++ b/src/Kiota.Builder/WorkspaceManagement/ApiPluginConfigurationComparer.cs @@ -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); } /// diff --git a/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfigurationComparer.cs b/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfigurationComparer.cs index cd1a77f688..aa59e320e9 100644 --- a/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfigurationComparer.cs +++ b/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfigurationComparer.cs @@ -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) From 6eecf1ad840c967199aba537a3041db5efa3874a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Sep 2024 07:07:53 -0400 Subject: [PATCH 3/3] fix: adds comment to contextualize the workaround --- src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs b/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs index e3bcf76491..5b145e9daa 100644 --- a/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs +++ b/src/Kiota.Builder/Validation/OpenApiSchemaComparer.cs @@ -27,6 +27,8 @@ public OpenApiSchemaComparer( /// public bool Equals(OpenApiSchema? x, OpenApiSchema? 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); } ///