From 965eb2a02fd23f52d1d931b1684f56cc51553f7d Mon Sep 17 00:00:00 2001 From: William Cheng Date: Thu, 6 Jul 2023 11:45:31 +0800 Subject: [PATCH] Add logic to avoid stackoverflow (#16008) * add logic to avoid stackoverflow * add test file --- .../openapitools/codegen/DefaultCodegen.java | 39 ++++++--- .../codegen/utils/ModelUtils.java | 17 ++-- .../codegen/java/JavaClientCodegenTest.java | 23 ++++++ .../src/test/resources/3_0/issue_12929.yaml | 80 +++++++++++++++++++ 4 files changed, 142 insertions(+), 17 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_0/issue_12929.yaml diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 902dedf1e32..a1befd4e322 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3189,8 +3189,16 @@ public class DefaultCodegen implements CodegenConfig { * @param composedSchemaName The name of the sc Schema * @param sc The Schema that may contain the discriminator * @param discPropName The String that is the discriminator propertyName in the schema + * @param visitedSchemas A set of visited schema names + * */ - private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, OpenAPI openAPI) { + private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, Set visitedSchemas) { + if (visitedSchemas.contains(composedSchemaName)) { // recursive schema definition found + return null; + } else { + visitedSchemas.add(composedSchemaName); + } + Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); if (refSchema.getProperties() != null && refSchema.getProperties().get(discPropName) != null) { Schema discSchema = (Schema) refSchema.getProperties().get(discPropName); @@ -3209,7 +3217,7 @@ public class DefaultCodegen implements CodegenConfig { if (composedSchema.getAllOf() != null) { // If our discriminator is in one of the allOf schemas break when we find it for (Schema allOf : composedSchema.getAllOf()) { - CodegenProperty cp = discriminatorFound(composedSchemaName, allOf, discPropName, openAPI); + CodegenProperty cp = discriminatorFound(composedSchemaName, allOf, discPropName, visitedSchemas); if (cp != null) { return cp; } @@ -3220,7 +3228,7 @@ public class DefaultCodegen implements CodegenConfig { CodegenProperty cp = new CodegenProperty(); for (Schema oneOf : composedSchema.getOneOf()) { String modelName = ModelUtils.getSimpleRef(oneOf.get$ref()); - CodegenProperty thisCp = discriminatorFound(composedSchemaName, oneOf, discPropName, openAPI); + CodegenProperty thisCp = discriminatorFound(composedSchemaName, oneOf, discPropName, visitedSchemas); if (thisCp == null) { LOGGER.warn( "'{}' defines discriminator '{}', but the referenced OneOf schema '{}' is missing {}", @@ -3243,7 +3251,7 @@ public class DefaultCodegen implements CodegenConfig { CodegenProperty cp = new CodegenProperty(); for (Schema anyOf : composedSchema.getAnyOf()) { String modelName = ModelUtils.getSimpleRef(anyOf.get$ref()); - CodegenProperty thisCp = discriminatorFound(composedSchemaName, anyOf, discPropName, openAPI); + CodegenProperty thisCp = discriminatorFound(composedSchemaName, anyOf, discPropName, visitedSchemas); if (thisCp == null) { LOGGER.warn( "'{}' defines discriminator '{}', but the referenced AnyOf schema '{}' is missing {}", @@ -3268,12 +3276,11 @@ public class DefaultCodegen implements CodegenConfig { /** * Recursively look in Schema sc for the discriminator and return it - * Schema sc location - * OpenAPI openAPI the spec where we are searching for the discriminator * * @param sc The Schema that may contain the discriminator + * @param visitedSchemas An array list of visited schemas */ - private Discriminator recursiveGetDiscriminator(Schema sc, OpenAPI openAPI) { + private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList visitedSchemas) { Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); Discriminator foundDisc = refSchema.getDiscriminator(); if (foundDisc != null) { @@ -3283,13 +3290,21 @@ public class DefaultCodegen implements CodegenConfig { if (this.getLegacyDiscriminatorBehavior()) { return null; } + + for (Schema s : visitedSchemas) { + if (s == refSchema) { + return null; + } + } + visitedSchemas.add(refSchema); + Discriminator disc = new Discriminator(); if (ModelUtils.isComposedSchema(refSchema)) { ComposedSchema composedSchema = (ComposedSchema) refSchema; if (composedSchema.getAllOf() != null) { // If our discriminator is in one of the allOf schemas break when we find it for (Schema allOf : composedSchema.getAllOf()) { - foundDisc = recursiveGetDiscriminator(allOf, openAPI); + foundDisc = recursiveGetDiscriminator(allOf, visitedSchemas); if (foundDisc != null) { disc.setPropertyName(foundDisc.getPropertyName()); disc.setMapping(foundDisc.getMapping()); @@ -3308,7 +3323,7 @@ public class DefaultCodegen implements CodegenConfig { hasNullTypeCnt++; continue; } - foundDisc = recursiveGetDiscriminator(oneOf, openAPI); + foundDisc = recursiveGetDiscriminator(oneOf, visitedSchemas); if (foundDisc != null) { discriminatorsPropNames.add(foundDisc.getPropertyName()); hasDiscriminatorCnt++; @@ -3337,7 +3352,7 @@ public class DefaultCodegen implements CodegenConfig { hasNullTypeCnt++; continue; } - foundDisc = recursiveGetDiscriminator(anyOf, openAPI); + foundDisc = recursiveGetDiscriminator(anyOf, visitedSchemas); if (foundDisc != null) { discriminatorsPropNames.add(foundDisc.getPropertyName()); hasDiscriminatorCnt++; @@ -3398,7 +3413,7 @@ public class DefaultCodegen implements CodegenConfig { "Invalid inline schema defined in oneOf/anyOf in '{}'. Per the OpenApi spec, for this case when a composed schema defines a discriminator, the oneOf/anyOf schemas must use $ref. Change this inline definition to a $ref definition", composedSchemaName); } - CodegenProperty df = discriminatorFound(composedSchemaName, sc, discPropName, openAPI); + CodegenProperty df = discriminatorFound(composedSchemaName, sc, discPropName, new TreeSet()); String modelName = ModelUtils.getSimpleRef(ref); if (df == null || !df.isString || df.required != true) { String msgSuffix = ""; @@ -3494,7 +3509,7 @@ public class DefaultCodegen implements CodegenConfig { } protected CodegenDiscriminator createDiscriminator(String schemaName, Schema schema, OpenAPI openAPI) { - Discriminator sourceDiscriminator = recursiveGetDiscriminator(schema, openAPI); + Discriminator sourceDiscriminator = recursiveGetDiscriminator(schema, new ArrayList()); if (sourceDiscriminator == null) { return null; } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java index 289fea2e242..7252a1149a8 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java @@ -1341,7 +1341,7 @@ public class ModelUtils { if (s == null) { LOGGER.error("Failed to obtain schema from {}", parentName); return "UNKNOWN_PARENT_NAME"; - } else if (hasOrInheritsDiscriminator(s, allSchemas)) { + } else if (hasOrInheritsDiscriminator(s, allSchemas, new ArrayList())) { // discriminator.propertyName is used or x-parent is used return parentName; } else { @@ -1391,7 +1391,7 @@ public class ModelUtils { if (s == null) { LOGGER.error("Failed to obtain schema from {}", parentName); names.add("UNKNOWN_PARENT_NAME"); - } else if (hasOrInheritsDiscriminator(s, allSchemas)) { + } else if (hasOrInheritsDiscriminator(s, allSchemas, new ArrayList())) { // discriminator.propertyName is used or x-parent is used names.add(parentName); if (includeAncestors && s instanceof ComposedSchema) { @@ -1416,7 +1416,14 @@ public class ModelUtils { return names; } - private static boolean hasOrInheritsDiscriminator(Schema schema, Map allSchemas) { + private static boolean hasOrInheritsDiscriminator(Schema schema, Map allSchemas, ArrayList visitedSchemas) { + for (Schema s : visitedSchemas) { + if (s == schema) { + return false; + } + } + visitedSchemas.add(schema); + if ((schema.getDiscriminator() != null && StringUtils.isNotEmpty(schema.getDiscriminator().getPropertyName())) || (isExtensionParent(schema))) { // x-parent is used return true; @@ -1424,7 +1431,7 @@ public class ModelUtils { String parentName = getSimpleRef(schema.get$ref()); Schema s = allSchemas.get(parentName); if (s != null) { - return hasOrInheritsDiscriminator(s, allSchemas); + return hasOrInheritsDiscriminator(s, allSchemas, visitedSchemas); } else { LOGGER.error("Failed to obtain schema from {}", parentName); } @@ -1432,7 +1439,7 @@ public class ModelUtils { final ComposedSchema composed = (ComposedSchema) schema; final List interfaces = getInterfaces(composed); for (Schema i : interfaces) { - if (hasOrInheritsDiscriminator(i, allSchemas)) { + if (hasOrInheritsDiscriminator(i, allSchemas, visitedSchemas)) { return true; } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index 5ed5a272c8e..fd61211ef88 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2252,4 +2252,27 @@ public class JavaClientCodegenTest { TestUtils.assertFileContains(petApi, "@Component"); } + @Test + public void testLogicToAvoidStackOverflow() throws IOException { + Map properties = new HashMap<>(); + properties.put(CodegenConstants.API_PACKAGE, "xyz.abcdef.api"); + properties.put(JavaClientCodegen.GENERATE_CLIENT_AS_BEAN, true); + + File output = Files.createTempDirectory("test").toFile(); + output.deleteOnExit(); + + final CodegenConfigurator configurator = new CodegenConfigurator() + .setGeneratorName("java") + .setLibrary(JavaClientCodegen.RESTTEMPLATE) + .setAdditionalProperties(properties) + .setInputSpec("src/test/resources/3_0/issue_12929.yaml") + .setOutputDir(output.getAbsolutePath().replace("\\", "/")); + + + DefaultGenerator generator = new DefaultGenerator(); + List files = generator.opts(configurator.toClientOptInput()).generate(); + files.forEach(File::deleteOnExit); + + // shouldn't throw stackoverflow exception + } } diff --git a/modules/openapi-generator/src/test/resources/3_0/issue_12929.yaml b/modules/openapi-generator/src/test/resources/3_0/issue_12929.yaml new file mode 100644 index 00000000000..c905c79e613 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/issue_12929.yaml @@ -0,0 +1,80 @@ +openapi: 3.0.1 +info: + title: "test" + version: "1" +servers: +- {url: /} +paths: + /v1/program/{programId}/enrollment/{sourceAccountNumber}/fruits: + post: + operationId: addFruit + parameters: + - name: programId + in: path + required: true + schema: {pattern: '^[a-z0-9-]*$', type: string} + - name: sourceAccountNumber + in: path + required: true + schema: {pattern: '^[a-z0-9_-]*$', type: string} + requestBody: + content: + application/xml: + schema: {$ref: '#/components/schemas/FruitInfo'} + required: true + responses: + '401': + description: OK + content: + text/plain: + schema: + type: string +components: + schemas: + FruitInfo: + type: object + properties: + model: {maxLength: 255, minLength: 0, type: string } + installDate: {type: string, format: date-time} + manufacturer: {maxLength: 255, minLength: 0, type: string} + uuid: {maxLength: 36, minLength: 0,type: string } + fruitType: + type: string + enum: [CHERRY, APPLE, GENERIC] + fruitTypeSpecificFields: {$ref: '#/components/schemas/SpecificFields'} + xml: {name: fruitInfo} + SpecificFields: + type: object + discriminator: + propertyName: name + mapping: {AppleSpecificFields: '#/components/schemas/AppleSpecificFields', + CherrySpecificFields: '#/components/schemas/CherrySpecificFields', GenericFruitFields: '#/components/schemas/GenericFruitFields'} + oneOf: + - {$ref: '#/components/schemas/AppleSpecificFields'} + - {$ref: '#/components/schemas/CherrySpecificFields'} + - {$ref: '#/components/schemas/GenericFruitFields'} + CherrySpecificFields: + type: object + allOf: + - {$ref: '#/components/schemas/SpecificFields'} + - type: object + properties: + chargeCapacity: {type: number, format: double} + dischargeCapacity: {type: number, format: double} + storageCapacity: {type: number, format: double} + AppleSpecificFields: + type: object + allOf: + - {$ref: '#/components/schemas/SpecificFields'} + - type: object + properties: + evseType: + type: string + enum: [ONE, TWO, THREE] + GenericFruitFields: + type: object + allOf: + - {$ref: '#/components/schemas/SpecificFields'} + - type: object + properties: + category: {maxLength: 255, minLength: 0, type: string}