Skip to content

Commit 965eb2a

Browse files
wing328wing328
authored
Add logic to avoid stackoverflow (#16008)
* add logic to avoid stackoverflow * add test file
1 parent bf80ccb commit 965eb2a

4 files changed

Lines changed: 142 additions & 17 deletions

File tree

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3189,8 +3189,16 @@ protected void setAddProps(Schema schema, IJsonSchemaValidationProperties proper
31893189
* @param composedSchemaName The name of the sc Schema
31903190
* @param sc The Schema that may contain the discriminator
31913191
* @param discPropName The String that is the discriminator propertyName in the schema
3192+
* @param visitedSchemas A set of visited schema names
3193+
*
31923194
*/
3193-
private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, OpenAPI openAPI) {
3195+
private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, Set<String> visitedSchemas) {
3196+
if (visitedSchemas.contains(composedSchemaName)) { // recursive schema definition found
3197+
return null;
3198+
} else {
3199+
visitedSchemas.add(composedSchemaName);
3200+
}
3201+
31943202
Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc);
31953203
if (refSchema.getProperties() != null && refSchema.getProperties().get(discPropName) != null) {
31963204
Schema discSchema = (Schema) refSchema.getProperties().get(discPropName);
@@ -3209,7 +3217,7 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc,
32093217
if (composedSchema.getAllOf() != null) {
32103218
// If our discriminator is in one of the allOf schemas break when we find it
32113219
for (Schema allOf : composedSchema.getAllOf()) {
3212-
CodegenProperty cp = discriminatorFound(composedSchemaName, allOf, discPropName, openAPI);
3220+
CodegenProperty cp = discriminatorFound(composedSchemaName, allOf, discPropName, visitedSchemas);
32133221
if (cp != null) {
32143222
return cp;
32153223
}
@@ -3220,7 +3228,7 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc,
32203228
CodegenProperty cp = new CodegenProperty();
32213229
for (Schema oneOf : composedSchema.getOneOf()) {
32223230
String modelName = ModelUtils.getSimpleRef(oneOf.get$ref());
3223-
CodegenProperty thisCp = discriminatorFound(composedSchemaName, oneOf, discPropName, openAPI);
3231+
CodegenProperty thisCp = discriminatorFound(composedSchemaName, oneOf, discPropName, visitedSchemas);
32243232
if (thisCp == null) {
32253233
LOGGER.warn(
32263234
"'{}' defines discriminator '{}', but the referenced OneOf schema '{}' is missing {}",
@@ -3243,7 +3251,7 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc,
32433251
CodegenProperty cp = new CodegenProperty();
32443252
for (Schema anyOf : composedSchema.getAnyOf()) {
32453253
String modelName = ModelUtils.getSimpleRef(anyOf.get$ref());
3246-
CodegenProperty thisCp = discriminatorFound(composedSchemaName, anyOf, discPropName, openAPI);
3254+
CodegenProperty thisCp = discriminatorFound(composedSchemaName, anyOf, discPropName, visitedSchemas);
32473255
if (thisCp == null) {
32483256
LOGGER.warn(
32493257
"'{}' defines discriminator '{}', but the referenced AnyOf schema '{}' is missing {}",
@@ -3268,12 +3276,11 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc,
32683276

32693277
/**
32703278
* Recursively look in Schema sc for the discriminator and return it
3271-
* Schema sc location
3272-
* OpenAPI openAPI the spec where we are searching for the discriminator
32733279
*
32743280
* @param sc The Schema that may contain the discriminator
3281+
* @param visitedSchemas An array list of visited schemas
32753282
*/
3276-
private Discriminator recursiveGetDiscriminator(Schema sc, OpenAPI openAPI) {
3283+
private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList<Schema> visitedSchemas) {
32773284
Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc);
32783285
Discriminator foundDisc = refSchema.getDiscriminator();
32793286
if (foundDisc != null) {
@@ -3283,13 +3290,21 @@ private Discriminator recursiveGetDiscriminator(Schema sc, OpenAPI openAPI) {
32833290
if (this.getLegacyDiscriminatorBehavior()) {
32843291
return null;
32853292
}
3293+
3294+
for (Schema s : visitedSchemas) {
3295+
if (s == refSchema) {
3296+
return null;
3297+
}
3298+
}
3299+
visitedSchemas.add(refSchema);
3300+
32863301
Discriminator disc = new Discriminator();
32873302
if (ModelUtils.isComposedSchema(refSchema)) {
32883303
ComposedSchema composedSchema = (ComposedSchema) refSchema;
32893304
if (composedSchema.getAllOf() != null) {
32903305
// If our discriminator is in one of the allOf schemas break when we find it
32913306
for (Schema allOf : composedSchema.getAllOf()) {
3292-
foundDisc = recursiveGetDiscriminator(allOf, openAPI);
3307+
foundDisc = recursiveGetDiscriminator(allOf, visitedSchemas);
32933308
if (foundDisc != null) {
32943309
disc.setPropertyName(foundDisc.getPropertyName());
32953310
disc.setMapping(foundDisc.getMapping());
@@ -3308,7 +3323,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, OpenAPI openAPI) {
33083323
hasNullTypeCnt++;
33093324
continue;
33103325
}
3311-
foundDisc = recursiveGetDiscriminator(oneOf, openAPI);
3326+
foundDisc = recursiveGetDiscriminator(oneOf, visitedSchemas);
33123327
if (foundDisc != null) {
33133328
discriminatorsPropNames.add(foundDisc.getPropertyName());
33143329
hasDiscriminatorCnt++;
@@ -3337,7 +3352,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, OpenAPI openAPI) {
33373352
hasNullTypeCnt++;
33383353
continue;
33393354
}
3340-
foundDisc = recursiveGetDiscriminator(anyOf, openAPI);
3355+
foundDisc = recursiveGetDiscriminator(anyOf, visitedSchemas);
33413356
if (foundDisc != null) {
33423357
discriminatorsPropNames.add(foundDisc.getPropertyName());
33433358
hasDiscriminatorCnt++;
@@ -3398,7 +3413,7 @@ protected List<MappedModel> getOneOfAnyOfDescendants(String composedSchemaName,
33983413
"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",
33993414
composedSchemaName);
34003415
}
3401-
CodegenProperty df = discriminatorFound(composedSchemaName, sc, discPropName, openAPI);
3416+
CodegenProperty df = discriminatorFound(composedSchemaName, sc, discPropName, new TreeSet<String>());
34023417
String modelName = ModelUtils.getSimpleRef(ref);
34033418
if (df == null || !df.isString || df.required != true) {
34043419
String msgSuffix = "";
@@ -3494,7 +3509,7 @@ protected List<MappedModel> getAllOfDescendants(String thisSchemaName, OpenAPI o
34943509
}
34953510

34963511
protected CodegenDiscriminator createDiscriminator(String schemaName, Schema schema, OpenAPI openAPI) {
3497-
Discriminator sourceDiscriminator = recursiveGetDiscriminator(schema, openAPI);
3512+
Discriminator sourceDiscriminator = recursiveGetDiscriminator(schema, new ArrayList<Schema>());
34983513
if (sourceDiscriminator == null) {
34993514
return null;
35003515
}

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,7 @@ public static String getParentName(ComposedSchema composedSchema, Map<String, Sc
13411341
if (s == null) {
13421342
LOGGER.error("Failed to obtain schema from {}", parentName);
13431343
return "UNKNOWN_PARENT_NAME";
1344-
} else if (hasOrInheritsDiscriminator(s, allSchemas)) {
1344+
} else if (hasOrInheritsDiscriminator(s, allSchemas, new ArrayList<Schema>())) {
13451345
// discriminator.propertyName is used or x-parent is used
13461346
return parentName;
13471347
} else {
@@ -1391,7 +1391,7 @@ public static List<String> getAllParentsName(ComposedSchema composedSchema, Map<
13911391
if (s == null) {
13921392
LOGGER.error("Failed to obtain schema from {}", parentName);
13931393
names.add("UNKNOWN_PARENT_NAME");
1394-
} else if (hasOrInheritsDiscriminator(s, allSchemas)) {
1394+
} else if (hasOrInheritsDiscriminator(s, allSchemas, new ArrayList<Schema>())) {
13951395
// discriminator.propertyName is used or x-parent is used
13961396
names.add(parentName);
13971397
if (includeAncestors && s instanceof ComposedSchema) {
@@ -1416,23 +1416,30 @@ public static List<String> getAllParentsName(ComposedSchema composedSchema, Map<
14161416
return names;
14171417
}
14181418

1419-
private static boolean hasOrInheritsDiscriminator(Schema schema, Map<String, Schema> allSchemas) {
1419+
private static boolean hasOrInheritsDiscriminator(Schema schema, Map<String, Schema> allSchemas, ArrayList<Schema> visitedSchemas) {
1420+
for (Schema s : visitedSchemas) {
1421+
if (s == schema) {
1422+
return false;
1423+
}
1424+
}
1425+
visitedSchemas.add(schema);
1426+
14201427
if ((schema.getDiscriminator() != null && StringUtils.isNotEmpty(schema.getDiscriminator().getPropertyName()))
14211428
|| (isExtensionParent(schema))) { // x-parent is used
14221429
return true;
14231430
} else if (StringUtils.isNotEmpty(schema.get$ref())) {
14241431
String parentName = getSimpleRef(schema.get$ref());
14251432
Schema s = allSchemas.get(parentName);
14261433
if (s != null) {
1427-
return hasOrInheritsDiscriminator(s, allSchemas);
1434+
return hasOrInheritsDiscriminator(s, allSchemas, visitedSchemas);
14281435
} else {
14291436
LOGGER.error("Failed to obtain schema from {}", parentName);
14301437
}
14311438
} else if (schema instanceof ComposedSchema) {
14321439
final ComposedSchema composed = (ComposedSchema) schema;
14331440
final List<Schema> interfaces = getInterfaces(composed);
14341441
for (Schema i : interfaces) {
1435-
if (hasOrInheritsDiscriminator(i, allSchemas)) {
1442+
if (hasOrInheritsDiscriminator(i, allSchemas, visitedSchemas)) {
14361443
return true;
14371444
}
14381445
}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,4 +2252,27 @@ public void testRestTemplateWithGeneratedClientAsBeanEnabled() throws IOExceptio
22522252
TestUtils.assertFileContains(petApi, "@Component");
22532253
}
22542254

2255+
@Test
2256+
public void testLogicToAvoidStackOverflow() throws IOException {
2257+
Map<String, Object> properties = new HashMap<>();
2258+
properties.put(CodegenConstants.API_PACKAGE, "xyz.abcdef.api");
2259+
properties.put(JavaClientCodegen.GENERATE_CLIENT_AS_BEAN, true);
2260+
2261+
File output = Files.createTempDirectory("test").toFile();
2262+
output.deleteOnExit();
2263+
2264+
final CodegenConfigurator configurator = new CodegenConfigurator()
2265+
.setGeneratorName("java")
2266+
.setLibrary(JavaClientCodegen.RESTTEMPLATE)
2267+
.setAdditionalProperties(properties)
2268+
.setInputSpec("src/test/resources/3_0/issue_12929.yaml")
2269+
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));
2270+
2271+
2272+
DefaultGenerator generator = new DefaultGenerator();
2273+
List<File> files = generator.opts(configurator.toClientOptInput()).generate();
2274+
files.forEach(File::deleteOnExit);
2275+
2276+
// shouldn't throw stackoverflow exception
2277+
}
22552278
}

‎modules/openapi-generator/src/test/resources/3_0/issue_12929.yaml‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
openapi: 3.0.1
2+
info:
3+
title: "test"
4+
version: "1"
5+
servers:
6+
- {url: /}
7+
paths:
8+
/v1/program/{programId}/enrollment/{sourceAccountNumber}/fruits:
9+
post:
10+
operationId: addFruit
11+
parameters:
12+
- name: programId
13+
in: path
14+
required: true
15+
schema: {pattern: '^[a-z0-9-]*$', type: string}
16+
- name: sourceAccountNumber
17+
in: path
18+
required: true
19+
schema: {pattern: '^[a-z0-9_-]*$', type: string}
20+
requestBody:
21+
content:
22+
application/xml:
23+
schema: {$ref: '#/components/schemas/FruitInfo'}
24+
required: true
25+
responses:
26+
'401':
27+
description: OK
28+
content:
29+
text/plain:
30+
schema:
31+
type: string
32+
components:
33+
schemas:
34+
FruitInfo:
35+
type: object
36+
properties:
37+
model: {maxLength: 255, minLength: 0, type: string }
38+
installDate: {type: string, format: date-time}
39+
manufacturer: {maxLength: 255, minLength: 0, type: string}
40+
uuid: {maxLength: 36, minLength: 0,type: string }
41+
fruitType:
42+
type: string
43+
enum: [CHERRY, APPLE, GENERIC]
44+
fruitTypeSpecificFields: {$ref: '#/components/schemas/SpecificFields'}
45+
xml: {name: fruitInfo}
46+
SpecificFields:
47+
type: object
48+
discriminator:
49+
propertyName: name
50+
mapping: {AppleSpecificFields: '#/components/schemas/AppleSpecificFields',
51+
CherrySpecificFields: '#/components/schemas/CherrySpecificFields', GenericFruitFields: '#/components/schemas/GenericFruitFields'}
52+
oneOf:
53+
- {$ref: '#/components/schemas/AppleSpecificFields'}
54+
- {$ref: '#/components/schemas/CherrySpecificFields'}
55+
- {$ref: '#/components/schemas/GenericFruitFields'}
56+
CherrySpecificFields:
57+
type: object
58+
allOf:
59+
- {$ref: '#/components/schemas/SpecificFields'}
60+
- type: object
61+
properties:
62+
chargeCapacity: {type: number, format: double}
63+
dischargeCapacity: {type: number, format: double}
64+
storageCapacity: {type: number, format: double}
65+
AppleSpecificFields:
66+
type: object
67+
allOf:
68+
- {$ref: '#/components/schemas/SpecificFields'}
69+
- type: object
70+
properties:
71+
evseType:
72+
type: string
73+
enum: [ONE, TWO, THREE]
74+
GenericFruitFields:
75+
type: object
76+
allOf:
77+
- {$ref: '#/components/schemas/SpecificFields'}
78+
- type: object
79+
properties:
80+
category: {maxLength: 255, minLength: 0, type: string}

0 commit comments

Comments
 (0)