Skip to content

Commit b99101f

Browse files
authored
fix(normalizer): prevent nullable state leaking across shared schema refs (OpenAPITools#24356)
1 parent dfb3742 commit b99101f

12 files changed

Lines changed: 155 additions & 35 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.swagger.v3.oas.models.OpenAPI;
2424
import io.swagger.v3.oas.models.Operation;
2525
import io.swagger.v3.oas.models.PathItem;
26+
import io.swagger.v3.oas.models.SpecVersion;
2627
import io.swagger.v3.oas.models.callbacks.Callback;
2728
import io.swagger.v3.oas.models.headers.Header;
2829
import io.swagger.v3.oas.models.media.*;
@@ -2481,6 +2482,13 @@ public static boolean isParent(Schema schema) {
24812482
return false;
24822483
}
24832484

2485+
/**
2486+
* Creates a deep copy of a schema.
2487+
*
2488+
* @param schema schema to clone
2489+
* @param openapi31 true when cloning an OpenAPI 3.1 schema
2490+
* @return a deep copy of the schema
2491+
*/
24842492
public static Schema cloneSchema(Schema schema, boolean openapi31) {
24852493
if (openapi31) {
24862494
return AnnotationsUtils.clone(schema, openapi31);
@@ -2513,6 +2521,12 @@ public static Schema simplifyOneOfAnyOfWithOnlyOneNonNullSubSchema(OpenAPI openA
25132521
// if only one element left, simplify to just the element (schema)
25142522
if (subSchemas.size() == 1) {
25152523
Schema<?> subSchema = subSchemas.get(0);
2524+
// The parser may reuse a $ref schema instance in multiple locations. Clone the
2525+
// remaining $ref before applying parent metadata so those locations stay isolated.
2526+
if (subSchema.get$ref() != null) {
2527+
subSchema = cloneSchema(subSchema,
2528+
openAPI != null && SpecVersion.V31.equals(openAPI.getSpecVersion()));
2529+
}
25162530
// Preserve parent-level docs when nullable anyOf/oneOf collapses to a single child schema.
25172531
if (subSchema.getDescription() == null && schema.getDescription() != null) {
25182532
subSchema.setDescription(schema.getDescription());

modules/openapi-generator/src/test/java/org/openapitools/codegen/typescript/axios/TypeScriptAxiosClientCodegenTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,27 @@ public void testDeprecatedArrayAttribute() throws Exception {
179179
TestUtils.assertFileContains(file, "'nicknames'?: Array<string>");
180180
}
181181

182+
@Test(description = "Nullable oneOf references do not make other uses of the referenced schema nullable")
183+
public void testNullableReferenceDoesNotLeakIntoModelProperty() throws Exception {
184+
final File output = Files.createTempDirectory("typescript_axios_nullable_reference_").toFile();
185+
output.deleteOnExit();
186+
187+
final CodegenConfigurator configurator = new CodegenConfigurator()
188+
.setGeneratorName("typescript-axios")
189+
.setInputSpec("src/test/resources/3_1/issue_23340.yaml")
190+
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));
191+
192+
final List<File> files = new DefaultGenerator().opts(configurator.toClientOptInput()).generate();
193+
files.forEach(File::deleteOnExit);
194+
195+
Path api = Paths.get(output + "/api.ts");
196+
TestUtils.assertFileContains(api,
197+
"export interface TitleGroupHierarchyLite {",
198+
"'content_type': ContentType;");
199+
TestUtils.assertFileNotContains(api, "'content_type': ContentType | null;");
200+
TestUtils.assertFileContains(api, "search: async (contentType?: ContentType | null");
201+
}
202+
182203
@Test(description = "Verify multipart file arrays use repeated form fields")
183204
public void testMultipartFileArrayUsesRepeatedFormFields() throws Exception {
184205
final File output = Files.createTempDirectory("typescript_axios_multipart_file_array_").toFile();

modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.openapitools.codegen.utils;
1919

2020
import io.swagger.v3.oas.models.OpenAPI;
21+
import io.swagger.v3.oas.models.SpecVersion;
2122
import io.swagger.v3.oas.models.media.*;
2223
import io.swagger.v3.oas.models.parameters.Parameter;
2324
import io.swagger.v3.oas.models.parameters.RequestBody;
@@ -555,6 +556,23 @@ public void simplifyOneOfWithOnlyOneNonNullSubSchemaKeepsReadOnlyWriteOnlyAttrib
555556
assertEquals(schema.get$ref(), "#/components/schemas/IntegerRef");
556557
}
557558

559+
@Test
560+
public void simplifyOneOfWithOnlyOneNonNullSubSchemaDoesNotMutateSharedSchema() {
561+
OpenAPI openAPI = new OpenAPI().specVersion(SpecVersion.V31);
562+
Schema sharedRef = new Schema<>().$ref("#/components/schemas/ContentType");
563+
Schema nullableParent = new ComposedSchema().oneOf(new ArrayList<>(Arrays.asList(
564+
new Schema<>().type("null"),
565+
sharedRef)));
566+
567+
Schema simplified = ModelUtils.simplifyOneOfAnyOfWithOnlyOneNonNullSubSchema(
568+
openAPI, nullableParent, nullableParent.getOneOf());
569+
570+
assertNotSame(simplified, sharedRef);
571+
assertEquals(simplified.get$ref(), sharedRef.get$ref());
572+
assertTrue(simplified.getNullable());
573+
assertNull(sharedRef.getNullable());
574+
}
575+
558576
@Test
559577
public void simplifyAnyOfWithOnlyOneNonNullSubSchemaKeepsReadOnlyWriteOnlyAttribute() {
560578
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml");
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
{
2+
"openapi": "3.1.0",
3+
"info": { "title": "repro-23340", "version": "1.0.0" },
4+
"paths": {
5+
"/home": {
6+
"get": {
7+
"operationId": "getHome",
8+
"responses": {
9+
"200": {
10+
"description": "ok",
11+
"content": {
12+
"application/json": {
13+
"schema": {
14+
"type": "object",
15+
"required": ["data"],
16+
"properties": { "data": { "$ref": "#/components/schemas/HomePage" } }
17+
}
18+
}
19+
}
20+
}
21+
}
22+
}
23+
},
24+
"/search": {
25+
"get": {
26+
"operationId": "search",
27+
"parameters": [
28+
{
29+
"name": "content_type",
30+
"in": "query",
31+
"required": false,
32+
"schema": { "oneOf": [ { "type": "null" }, { "$ref": "#/components/schemas/ContentType" } ] }
33+
}
34+
],
35+
"responses": { "200": { "description": "ok" } }
36+
}
37+
}
38+
},
39+
"components": {
40+
"schemas": {
41+
"ContentType": { "type": "string", "enum": ["movie"] },
42+
"TitleGroupLite": {
43+
"type": "object",
44+
"required": ["content_type"],
45+
"properties": { "content_type": { "$ref": "#/components/schemas/ContentType" } }
46+
},
47+
"HomePage": {
48+
"type": "object",
49+
"required": ["latest_uploads"],
50+
"properties": {
51+
"latest_uploads": { "type": "array", "items": { "$ref": "#/components/schemas/TitleGroupLite" } }
52+
}
53+
},
54+
"TitleGroupHierarchyLite": {
55+
"type": "object",
56+
"required": ["id", "content_type"],
57+
"properties": {
58+
"id": { "type": "integer", "format": "int32" },
59+
"content_type": { "$ref": "#/components/schemas/ContentType" }
60+
}
61+
}
62+
}
63+
}
64+
}

samples/client/others/ruby-nextgen-qdrant/lib/qdrant/models/batch_payloads_inner.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ module Models
1111
# anyOf wrapper. Returns the first candidate that validates.
1212
module BatchPayloadsInner
1313
CANDIDATES = [
14-
'Hash',
14+
'Hash<String, Object>',
1515
].freeze
1616

1717
def self.build(data)

samples/client/others/typescript-node/encode-decode/build/api/defaultApi.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ export class DefaultApi {
517517
/**
518518
*
519519
*/
520-
public async testDecodeMapOfObjectsGet (options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: { [key: string]: ComplexObject | undefined | null; }; }> {
520+
public async testDecodeMapOfObjectsGet (options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: { [key: string]: ComplexObject | undefined; }; }> {
521521
const localVarPath = this.basePath + '/test/decode/map-of/objects';
522522
let localVarQueryParameters: any = {};
523523
let localVarHeaderParams: any = (<any>Object).assign({}, this._defaultHeaders);
@@ -559,13 +559,13 @@ export class DefaultApi {
559559
localVarRequestOptions.form = localVarFormParams;
560560
}
561561
}
562-
return new Promise<{ response: http.IncomingMessage; body: { [key: string]: ComplexObject | undefined | null; }; }>((resolve, reject) => {
562+
return new Promise<{ response: http.IncomingMessage; body: { [key: string]: ComplexObject | undefined; }; }>((resolve, reject) => {
563563
localVarRequest(localVarRequestOptions, (error, response, body) => {
564564
if (error) {
565565
reject(error);
566566
} else {
567567
if (response.statusCode && response.statusCode >= 200 && response.statusCode <= 299) {
568-
body = ObjectSerializer.deserialize(body, "{ [key: string]: ComplexObject | undefined | null; }");
568+
body = ObjectSerializer.deserialize(body, "{ [key: string]: ComplexObject | undefined; }");
569569
resolve({ response: response, body: body });
570570
} else {
571571
reject(new HttpError(response, body, response.statusCode));
@@ -1487,7 +1487,7 @@ export class DefaultApi {
14871487
*
14881488
* @param requestBody
14891489
*/
1490-
public async testEncodeMapOfObjectsPost (requestBody: { [key: string]: ComplexObject | undefined | null; }, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body?: any; }> {
1490+
public async testEncodeMapOfObjectsPost (requestBody: { [key: string]: ComplexObject | undefined; }, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body?: any; }> {
14911491
const localVarPath = this.basePath + '/test/encode/map-of/objects';
14921492
let localVarQueryParameters: any = {};
14931493
let localVarHeaderParams: any = (<any>Object).assign({}, this._defaultHeaders);
@@ -1509,7 +1509,7 @@ export class DefaultApi {
15091509
uri: localVarPath,
15101510
useQuerystring: this._useQuerystring,
15111511
json: true,
1512-
body: ObjectSerializer.serialize(requestBody, "{ [key: string]: ComplexObject | undefined | null; }")
1512+
body: ObjectSerializer.serialize(requestBody, "{ [key: string]: ComplexObject | undefined; }")
15131513
};
15141514

15151515
let authenticationPromise = Promise.resolve();

samples/client/others/typescript/encode-decode/build/apis/DefaultApi.ts

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

samples/client/others/typescript/encode-decode/build/docs/DefaultApi.md

Lines changed: 9 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

samples/client/others/typescript/encode-decode/build/types/ObjectParamAPI.ts

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

samples/client/others/typescript/encode-decode/build/types/ObservableAPI.ts

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)