Skip to content

Commit

Permalink
[OpenAPITools#4650] Fix StackOverflowError for recursive composite sc…
Browse files Browse the repository at this point in the history
…hemas

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.
  • Loading branch information
Karsten Thoms committed Feb 23, 2022
1 parent 266cd5d commit 46c489a
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2532,14 +2532,14 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
if (allDefinitions != null && refSchema != null) {
if (allParents.contains(ref) && supportsMultipleInheritance) {
// multiple inheritance
addProperties(allProperties, allRequired, refSchema);
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} else if (parentName != null && parentName.equals(ref) && supportsInheritance) {
// single inheritance
addProperties(allProperties, allRequired, refSchema);
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} else {
// composition
addProperties(properties, required, refSchema);
addProperties(allProperties, allRequired, refSchema);
addProperties(properties, required, refSchema, new HashSet<>());
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
}
}

Expand Down Expand Up @@ -2576,10 +2576,10 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
if (component.get$ref() == null) {
if (component != null) {
// component is the child schema
addProperties(properties, required, component);
addProperties(properties, required, component, new HashSet<>());

// 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
}
Expand Down Expand Up @@ -3260,17 +3260,21 @@ protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Sc
/**
* Add schema's properties to "properties" and "required" list
*
* @param properties all properties
* @param required required property only
* @param schema schema in which the properties will be added to the lists
* @param properties all properties
* @param required required property only
* @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) {
ComposedSchema composedSchema = (ComposedSchema) schema;

if (composedSchema.getAllOf() != null) {
for (Schema component : composedSchema.getAllOf()) {
addProperties(properties, required, component);
addProperties(properties, required, component, visitedSchemas);
}
}

Expand All @@ -3280,13 +3284,13 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir

if (composedSchema.getOneOf() != null) {
for (Schema component : composedSchema.getOneOf()) {
addProperties(properties, required, component);
addProperties(properties, required, component, visitedSchemas);
}
}

if (composedSchema.getAnyOf() != null) {
for (Schema component : composedSchema.getAnyOf()) {
addProperties(properties, required, component);
addProperties(properties, required, component, visitedSchemas);
}
}

Expand All @@ -3295,7 +3299,7 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir

if (StringUtils.isNotBlank(schema.get$ref())) {
Schema interfaceSchema = ModelUtils.getReferencedSchema(this.openAPI, schema);
addProperties(properties, required, interfaceSchema);
addProperties(properties, required, interfaceSchema, visitedSchemas);
return;
}
if (schema.getProperties() != null) {
Expand Down Expand Up @@ -6244,7 +6248,7 @@ public List<CodegenParameter> fromRequestBodyToFormParameters(RequestBody body,
// 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
// https://github.com/OpenAPITools/openapi-generator/issues/10415
addProperties(properties, allRequired, schema);
addProperties(properties, allRequired, schema, new HashSet<>());

if (!properties.isEmpty()) {
for (Map.Entry<String, Schema> entry : properties.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,14 +629,14 @@ private void fixComposedSchemaRequiredVars(Schema schema, CodegenModel result) {
}
for (Schema sc : oneOfanyOfSchemas) {
Schema refSchema = ModelUtils.getReferencedSchema(this.openAPI, sc);
addProperties(otherProperties, otherRequired, refSchema);
addProperties(otherProperties, otherRequired, refSchema, new HashSet<>());
}
Set<String> otherRequiredSet = new HashSet<>(otherRequired);

List<Schema> allOf = cs.getAllOf();
if ((schema.getProperties() != null && !schema.getProperties().isEmpty()) || allOf != null) {
// NOTE: this function also adds the allOf properties inside schema
addProperties(selfProperties, selfRequired, schema);
addProperties(selfProperties, selfRequired, schema, new HashSet<>());
}
if (result.discriminator != null) {
selfRequired.add(result.discriminator.getPropertyBaseName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,4 +744,24 @@ public void testProcessUserDefinedTemplatesWithConfig() throws IOException {
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
}
}
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 46c489a

Please sign in to comment.