diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 13ec2cc24a..affd06d4ed 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1893,6 +1893,20 @@ private CodeTypeBase CreateModelDeclarations(OpenApiUrlTreeNode currentNode, IOp typeNameForInlineSchema = string.Empty; } + // Unwrap single-entry anyOf/oneOf wrappers that contain only a $ref and no discriminator or own properties. + // This covers the OpenAPI 3.1 nullable reference pattern (e.g. { type: [object, null], anyOf: [$ref: X] }) + // as well as degenerate single-entry unions. Without unwrapping, the wrapper gets treated as an inline + // object, and self-referential properties like "innerError" or "parent" cause infinite recursion because + // each iteration creates a uniquely-named inline class. + if (string.IsNullOrEmpty(schema.GetDiscriminatorPropertyName()) + && !schema.HasAnyProperty() + && ((schema.AnyOf is { Count: 1 } && schema.AnyOf[0].IsReferencedSchema()) + || (schema.OneOf is { Count: 1 } && schema.OneOf[0].IsReferencedSchema()))) + { + var innerRef = schema.AnyOf is { Count: 1 } ? schema.AnyOf[0] : schema.OneOf![0]; + return CreateModelDeclarations(currentNode, innerRef, operation, parentElement, suffixForInlineSchema, response, typeNameForInlineSchema, isRequestBody, isViaDiscriminator); + } + if (schema.IsInherited()) { // Pass isViaDiscriminator so that we can handle the special case where this model was referenced by a discriminator and we always want to generate a base class. @@ -1972,6 +1986,7 @@ private CodeNamespace GetSearchNamespace(OpenApiUrlTreeNode currentNode, CodeNam return currentNamespace; } private ConcurrentDictionary classLifecycles = new(StringComparer.OrdinalIgnoreCase); + private static readonly ThreadLocal> schemasBeingProcessedForDiscriminators = new(() => new(StringComparer.OrdinalIgnoreCase)); private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentNode, OpenApiOperation? currentOperation, IOpenApiSchema schema, string declarationName, CodeNamespace currentNamespace, CodeClass? inheritsFrom = null) { if (GetExistingDeclaration(currentNamespace, currentNode, declarationName) is not CodeElement existingDeclaration) // we can find it in the components @@ -2102,7 +2117,7 @@ private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, IOpenApiSchema s var newClass = currentNamespace.AddClass(newClassStub).First(); var lifecycle = classLifecycles.GetOrAdd(currentNamespace.Name + "." + declarationName, static n => new()); - if (!lifecycle.IsPropertiesBuilt()) + if (!lifecycle.IsPropertiesBuilt() && !lifecycle.IsPropertiesBuildingInProgress()) { try { @@ -2122,16 +2137,37 @@ private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, IOpenApiSchema s lifecycle.PropertiesBuildingDone(); } } + else if (lifecycle.IsPropertiesBuildingInProgress()) + { + LogCircularPropertyReference(declarationName, currentNamespace.Name); + } var lookupSchema = schema.GetMergedSchemaOriginalReferenceId() is string originalName ? new OpenApiSchemaReference(originalName, openApiDocument) : schema; - // Recurse into the discriminator & generate its referenced types - var mappings = GetDiscriminatorMappings(currentNode, lookupSchema, currentNamespace, newClass, currentOperation) - .Where(x => x.Value is { TypeDefinition: CodeClass definition } && - definition.DerivesFrom(newClass)); // only the mappings that derive from the current class + // Recurse into the discriminator & generate its referenced types, guarding against same-thread circular + // references for schemas with a reference ID. Inline schemas (no ref ID) bypass this guard but are + // covered by the depth guard in CreatePropertiesForModelClass. + var schemaRefId = lookupSchema.GetReferenceId(); + var discriminatorVisited = schemasBeingProcessedForDiscriminators.Value!; + if (!string.IsNullOrEmpty(schemaRefId) && !discriminatorVisited.Add(schemaRefId)) + { + // This schema's discriminators are already being resolved up the call stack — skip to break the cycle + return newClass; + } + try + { + var mappings = GetDiscriminatorMappings(currentNode, lookupSchema, currentNamespace, newClass, currentOperation) + .Where(x => x.Value is { TypeDefinition: CodeClass definition } && + definition.DerivesFrom(newClass)); // only the mappings that derive from the current class - AddDiscriminatorMethod(newClass, schema.GetDiscriminatorPropertyName(), mappings, static s => s); + AddDiscriminatorMethod(newClass, schema.GetDiscriminatorPropertyName(), mappings, static s => s); + } + finally + { + if (!string.IsNullOrEmpty(schemaRefId)) + discriminatorVisited.Remove(schemaRefId); + } return newClass; } /// @@ -2350,28 +2386,46 @@ internal static void AddDiscriminatorMethod(CodeClass newClass, string discrimin } return result; } + private static readonly ThreadLocal modelCreationDepth = new(() => 0); + // Parallel.ForEach may inline tasks on the calling thread, so this depth counter can be + // inflated across unrelated URL tree nodes sharing a thread. 50 is conservative enough + // to avoid false positives — legitimate model nesting rarely exceeds 5-10 levels. + private const int MaxModelCreationDepth = 50; private void CreatePropertiesForModelClass(OpenApiUrlTreeNode currentNode, IOpenApiSchema schema, CodeNamespace ns, CodeClass model) { - var propertiesToAdd = schema.Properties - ?.Select(x => - { - var propertySchema = x.Value; - var className = $"{model.Name}_{x.Key.CleanupSymbolName()}"; - var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(propertySchema.GetReferenceId()); - var targetNamespace = string.IsNullOrEmpty(shortestNamespaceName) ? ns : - rootNamespace?.FindOrAddNamespace(shortestNamespaceName) ?? ns; - var definition = CreateModelDeclarations(currentNode, propertySchema, default, targetNamespace, string.Empty, typeNameForInlineSchema: className); - if (definition == null) + if (modelCreationDepth.Value >= MaxModelCreationDepth) + { + logger.LogWarning("Maximum model creation depth ({MaxDepth}) exceeded for model {ModelName} in namespace {Namespace}. Skipping property creation to prevent stack overflow.", MaxModelCreationDepth, model.Name, ns.Name); + return; + } + modelCreationDepth.Value++; + try + { + var propertiesToAdd = schema.Properties + ?.Select(x => { - LogOmittedPropertyInvalidSchema(x.Key, model.Name, currentNode.Path); - return null; - } - return CreateProperty(x.Key, definition.Name, propertySchema: propertySchema, existingType: definition); - }) - .OfType() - .ToArray() ?? []; - if (propertiesToAdd.Length != 0) - model.AddProperty(propertiesToAdd); + var propertySchema = x.Value; + var className = $"{model.Name}_{x.Key.CleanupSymbolName()}"; + var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(propertySchema.GetReferenceId()); + var targetNamespace = string.IsNullOrEmpty(shortestNamespaceName) ? ns : + rootNamespace?.FindOrAddNamespace(shortestNamespaceName) ?? ns; + var definition = CreateModelDeclarations(currentNode, propertySchema, default, targetNamespace, string.Empty, typeNameForInlineSchema: className); + if (definition == null) + { + LogOmittedPropertyInvalidSchema(x.Key, model.Name, currentNode.Path); + return null; + } + return CreateProperty(x.Key, definition.Name, propertySchema: propertySchema, existingType: definition); + }) + .OfType() + .ToArray() ?? []; + if (propertiesToAdd.Length != 0) + model.AddProperty(propertiesToAdd); + } + finally + { + modelCreationDepth.Value--; + } } private const string FieldDeserializersMethodName = "GetFieldDeserializers"; private const string SerializeMethodName = "Serialize"; @@ -2680,4 +2734,6 @@ private void CleanUpInternalState() private partial void LogOmittedPropertyInvalidSchema(string propertyName, string modelName, string apiPath); [LoggerMessage(Level = LogLevel.Warning, Message = "Ignoring duplicate parameter {Name}")] private partial void LogIgnoringDuplicateParameter(string name); + [LoggerMessage(Level = LogLevel.Warning, Message = "Circular property reference detected for model {ModelName} in namespace {Namespace}. Skipping property creation to prevent infinite recursion.")] + private partial void LogCircularPropertyReference(string modelName, string @namespace); } diff --git a/src/Kiota.Builder/ModelClassBuildLifecycle.cs b/src/Kiota.Builder/ModelClassBuildLifecycle.cs index 0a5400bcee..f1d805c8d9 100644 --- a/src/Kiota.Builder/ModelClassBuildLifecycle.cs +++ b/src/Kiota.Builder/ModelClassBuildLifecycle.cs @@ -13,6 +13,15 @@ public Boolean IsPropertiesBuilt() { return propertiesBuilt.IsSet; } + /// + /// Returns true when the current thread is already building properties for this class, + /// indicating a circular reference in the schema. Monitor.Enter is reentrant, so without + /// this check the same thread could re-enter property building and cause a stack overflow. + /// + public bool IsPropertiesBuildingInProgress() + { + return Monitor.IsEntered(propertiesBuilt); + } public void WaitForPropertiesBuilt() { if (!Monitor.IsEntered(propertiesBuilt)) diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index c59b0618c8..89c635b8de 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -8874,6 +8874,119 @@ public async Task InclusiveUnionSingleEntriesMergingAsync() Assert.NotNull(twoProperty); } + [Fact] + public async Task NullableReferenceWrapperUnwrapsToReferencedSchemaAsync() + { + var tempFilePath = Path.GetTempFileName(); + await using var fs = await GetDocumentStreamAsync( + """ +openapi: 3.1.0 +info: + title: "Nullable reference wrapper unwrap test" + version: "1.0.0" +servers: + - url: https://example.doesnotexist/ +paths: + /items: + get: + description: Return something + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: "#/components/schemas/WorkbookOperationError" +components: + schemas: + WorkbookOperationError: + type: object + properties: + code: + type: string + message: + type: string + innerError: + type: + - 'null' + - object + anyOf: + - $ref: "#/components/schemas/WorkbookOperationError" +"""); + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient); + var document = await builder.CreateOpenApiDocumentAsync(fs, cancellationToken: TestContext.Current.CancellationToken); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + + // The self-referential schema should produce exactly one class, not infinite inline classes + var errorClass = codeModel.FindChildByName("WorkbookOperationError"); + Assert.NotNull(errorClass); + var codeProperty = errorClass.FindChildByName("innerError", false); + Assert.NotNull(codeProperty); + // innerError should reference the same WorkbookOperationError type, not a unique inline class + Assert.Equal("WorkbookOperationError", codeProperty.Type.Name); + } + + [Fact] + public async Task NullableReferenceWrapperWithDiscriminatorIsNotUnwrappedAsync() + { + var tempFilePath = Path.GetTempFileName(); + await using var fs = await GetDocumentStreamAsync( + """ +openapi: 3.0.0 +info: + title: "Nullable reference wrapper with discriminator should not unwrap" + version: "1.0.0" +servers: + - url: https://example.doesnotexist/ +paths: + /items: + get: + description: Return something + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: "#/components/schemas/UsesWrapper" +components: + schemas: + UsesWrapper: + type: object + properties: + value: + $ref: "#/components/schemas/WrapperWithDiscriminator" + WrapperWithDiscriminator: + type: object + anyOf: + - $ref: "#/components/schemas/Component1" + discriminator: + propertyName: objectType + Component1: + type: object + required: + - objectType + properties: + objectType: + type: string + name: + type: string +"""); + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient); + var document = await builder.CreateOpenApiDocumentAsync(fs, cancellationToken: TestContext.Current.CancellationToken); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + + // With a discriminator, the wrapper should NOT be unwrapped — it should keep its own class with merged properties + var wrapperClass = codeModel.FindChildByName("WrapperWithDiscriminator"); + Assert.NotNull(wrapperClass); + var nameProperty = wrapperClass.FindChildByName("name", false); + Assert.NotNull(nameProperty); + } + [Fact] public async Task InclusiveUnionInheritanceEntriesMergingAsync() {