[#4650] Fix StackOverflowError for recursive composite schemas (#11620)

The generator ran into a loop when a composite schema recursively added itself. This change provides a reproducing example and fixes the issue by extending DefaultCodegen#addProperties() by an additional circuit breaker parameter.

When additionalProperties() is called with a schema instance for which the properties have already been added, the method directly returns and does not run into the loop.
This commit is contained in:
Karsten Thoms 2022-03-01 18:35:47 +01:00 committed by GitHub
parent 21f649e087
commit 6a7fc38fb9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 105 additions and 17 deletions

View File

@ -2535,14 +2535,14 @@ public class DefaultCodegen implements CodegenConfig {
if (allDefinitions != null && refSchema != null) { if (allDefinitions != null && refSchema != null) {
if (allParents.contains(ref) && supportsMultipleInheritance) { if (allParents.contains(ref) && supportsMultipleInheritance) {
// multiple inheritance // multiple inheritance
addProperties(allProperties, allRequired, refSchema); addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} else if (parentName != null && parentName.equals(ref) && supportsInheritance) { } else if (parentName != null && parentName.equals(ref) && supportsInheritance) {
// single inheritance // single inheritance
addProperties(allProperties, allRequired, refSchema); addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} else { } else {
// composition // composition
addProperties(properties, required, refSchema); addProperties(properties, required, refSchema, new HashSet<>());
addProperties(allProperties, allRequired, refSchema); addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} }
} }
@ -2579,10 +2579,10 @@ public class DefaultCodegen implements CodegenConfig {
if (component.get$ref() == null) { if (component.get$ref() == null) {
if (component != null) { if (component != null) {
// component is the child schema // component is the child schema
addProperties(properties, required, component); addProperties(properties, required, component, new HashSet<>());
// includes child's properties (all, required) in allProperties, allRequired // includes child's properties (all, required) in allProperties, allRequired
addProperties(allProperties, allRequired, component); addProperties(allProperties, allRequired, component, new HashSet<>());
} }
break; // at most one child only break; // at most one child only
} }
@ -3263,17 +3263,21 @@ public class DefaultCodegen implements CodegenConfig {
/** /**
* Add schema's properties to "properties" and "required" list * Add schema's properties to "properties" and "required" list
* *
* @param properties all properties * @param properties all properties
* @param required required property only * @param required required property only
* @param schema schema in which the properties will be added to the lists * @param schema schema in which the properties will be added to the lists
* @param visitedSchemas circuit-breaker - the schemas with which the method was called before for recursive structures
*/ */
protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema) { protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema, Set<Schema> visitedSchemas) {
if (!visitedSchemas.add(schema)) {
return;
}
if (schema instanceof ComposedSchema) { if (schema instanceof ComposedSchema) {
ComposedSchema composedSchema = (ComposedSchema) schema; ComposedSchema composedSchema = (ComposedSchema) schema;
if (composedSchema.getAllOf() != null) { if (composedSchema.getAllOf() != null) {
for (Schema component : composedSchema.getAllOf()) { for (Schema component : composedSchema.getAllOf()) {
addProperties(properties, required, component); addProperties(properties, required, component, visitedSchemas);
} }
} }
@ -3283,13 +3287,13 @@ public class DefaultCodegen implements CodegenConfig {
if (composedSchema.getOneOf() != null) { if (composedSchema.getOneOf() != null) {
for (Schema component : composedSchema.getOneOf()) { for (Schema component : composedSchema.getOneOf()) {
addProperties(properties, required, component); addProperties(properties, required, component, visitedSchemas);
} }
} }
if (composedSchema.getAnyOf() != null) { if (composedSchema.getAnyOf() != null) {
for (Schema component : composedSchema.getAnyOf()) { for (Schema component : composedSchema.getAnyOf()) {
addProperties(properties, required, component); addProperties(properties, required, component, visitedSchemas);
} }
} }
@ -3298,7 +3302,7 @@ public class DefaultCodegen implements CodegenConfig {
if (StringUtils.isNotBlank(schema.get$ref())) { if (StringUtils.isNotBlank(schema.get$ref())) {
Schema interfaceSchema = ModelUtils.getReferencedSchema(this.openAPI, schema); Schema interfaceSchema = ModelUtils.getReferencedSchema(this.openAPI, schema);
addProperties(properties, required, interfaceSchema); addProperties(properties, required, interfaceSchema, visitedSchemas);
return; return;
} }
if (schema.getProperties() != null) { if (schema.getProperties() != null) {
@ -6247,7 +6251,7 @@ public class DefaultCodegen implements CodegenConfig {
// TODO in the future have this return one codegenParameter of type object or composed which includes all definition // TODO in the future have this return one codegenParameter of type object or composed which includes all definition
// that will be needed for complex composition use cases // that will be needed for complex composition use cases
// https://github.com/OpenAPITools/openapi-generator/issues/10415 // https://github.com/OpenAPITools/openapi-generator/issues/10415
addProperties(properties, allRequired, schema); addProperties(properties, allRequired, schema, new HashSet<>());
if (!properties.isEmpty()) { if (!properties.isEmpty()) {
for (Map.Entry<String, Schema> entry : properties.entrySet()) { for (Map.Entry<String, Schema> entry : properties.entrySet()) {

View File

@ -629,14 +629,14 @@ public class PythonClientCodegen extends PythonLegacyClientCodegen {
} }
for (Schema sc : oneOfanyOfSchemas) { for (Schema sc : oneOfanyOfSchemas) {
Schema refSchema = ModelUtils.getReferencedSchema(this.openAPI, sc); Schema refSchema = ModelUtils.getReferencedSchema(this.openAPI, sc);
addProperties(otherProperties, otherRequired, refSchema); addProperties(otherProperties, otherRequired, refSchema, new HashSet<>());
} }
Set<String> otherRequiredSet = new HashSet<>(otherRequired); Set<String> otherRequiredSet = new HashSet<>(otherRequired);
List<Schema> allOf = cs.getAllOf(); List<Schema> allOf = cs.getAllOf();
if ((schema.getProperties() != null && !schema.getProperties().isEmpty()) || allOf != null) { if ((schema.getProperties() != null && !schema.getProperties().isEmpty()) || allOf != null) {
// NOTE: this function also adds the allOf properties inside schema // NOTE: this function also adds the allOf properties inside schema
addProperties(selfProperties, selfRequired, schema); addProperties(selfProperties, selfRequired, schema, new HashSet<>());
} }
if (result.discriminator != null) { if (result.discriminator != null) {
selfRequired.add(result.discriminator.getPropertyBaseName()); selfRequired.add(result.discriminator.getPropertyBaseName());

View File

@ -744,4 +744,24 @@ public class DefaultGeneratorTest {
templates.toFile().delete(); templates.toFile().delete();
} }
} }
@Test
public void testRecursionBug4650() {
OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/bugs/recursion-bug-4650.yaml");
ClientOptInput opts = new ClientOptInput();
opts.openAPI(openAPI);
DefaultCodegen config = new DefaultCodegen();
config.setStrictSpecBehavior(false);
opts.config(config);
final DefaultGenerator generator = new DefaultGenerator();
generator.opts(opts);
generator.configureGeneratorProperties();
List<File> files = new ArrayList<>();
List<String> filteredSchemas = ModelUtils.getSchemasUsedOnlyInFormParam(openAPI);
List<Object> allModels = new ArrayList<>();
// The bug causes a StackOverflowError when calling generateModels
generator.generateModels(files, allModels, filteredSchemas);
// all fine, we have passed
}
} }

View File

@ -0,0 +1,64 @@
openapi: 3.0.1
info:
title: Test
version: v1
paths:
/api/v1/filters:
get:
responses:
'200':
description: default response
content:
'*/*':
schema:
$ref: '#/components/schemas/Filter'
components:
schemas:
AndFilter:
type: object
allOf:
- $ref: '#/components/schemas/Filter'
- type: object
properties:
operator:
type: string
default: and
enum:
- and
- or
filters:
type: array
items:
$ref: '#/components/schemas/Filter'
Filter:
type: object
properties:
operator:
type: string
enum:
- and
- or
example:
operator: eq
field: name
value: john
discriminator:
propertyName: operator
mapping:
and: '#/components/schemas/AndFilter'
or: '#/components/schemas/OrFilter'
oneOf:
- $ref: '#/components/schemas/AndFilter'
- $ref: '#/components/schemas/OrFilter'
OrFilter:
type: object
allOf:
- $ref: '#/components/schemas/Filter'
- type: object
properties:
operator:
type: string
default: or
enum:
- and
- or