Skip to content

Commit 06d84df

Browse files
authored
[kotlin-server][Java][JAX-RS] Fix path shadowing (#23414) (#23871)
* Add tests to demonstrate the bug * Fix cross-tag path shadowing in JAX-RS common path extraction * Regenerate samples
1 parent d655b45 commit 06d84df

9 files changed

Lines changed: 530 additions & 124 deletions

File tree

modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaJAXRSServerCodegen.java

Lines changed: 160 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,24 @@
2121
import io.swagger.v3.oas.models.OpenAPI;
2222
import io.swagger.v3.oas.models.Operation;
2323
import io.swagger.v3.oas.models.PathItem;
24+
import java.io.File;
25+
import java.net.URL;
26+
import java.util.ArrayList;
27+
import java.util.Arrays;
28+
import java.util.Collections;
29+
import java.util.HashMap;
30+
import java.util.HashSet;
31+
import java.util.List;
32+
import java.util.Map;
33+
import java.util.Set;
2434
import lombok.Setter;
2535
import org.apache.commons.lang3.StringUtils;
26-
import org.openapitools.codegen.*;
36+
import org.openapitools.codegen.CliOption;
37+
import org.openapitools.codegen.CodegenConstants;
38+
import org.openapitools.codegen.CodegenOperation;
39+
import org.openapitools.codegen.CodegenParameter;
40+
import org.openapitools.codegen.CodegenResponse;
41+
import org.openapitools.codegen.CodegenType;
2742
import org.openapitools.codegen.languages.features.BeanValidationFeatures;
2843
import org.openapitools.codegen.model.ModelMap;
2944
import org.openapitools.codegen.model.OperationMap;
@@ -32,10 +47,6 @@
3247
import org.slf4j.Logger;
3348
import org.slf4j.LoggerFactory;
3449

35-
import java.io.File;
36-
import java.net.URL;
37-
import java.util.*;
38-
3950
public abstract class AbstractJavaJAXRSServerCodegen extends AbstractJavaCodegen implements BeanValidationFeatures {
4051
public static final String SERVER_PORT = "serverPort";
4152
public static final String USE_TAGS = "useTags";
@@ -70,6 +81,15 @@ public abstract class AbstractJavaJAXRSServerCodegen extends AbstractJavaCodegen
7081

7182
protected boolean useTags = false;
7283

84+
/**
85+
* All resource paths seen across every tag, collected during the first pass
86+
* ({@link #addOperationToGroup}). Used in the second pass
87+
* ({@link #postProcessOperationsWithModels}) to detect cross-tag path shadowing: a
88+
* candidate {@code commonPath} is only safe if no <em>other</em> tag owns a resource
89+
* path that starts with that prefix.
90+
*/
91+
private final Set<String> allResourcePaths = new HashSet<>();
92+
7393
private final Logger LOGGER = LoggerFactory.getLogger(AbstractJavaJAXRSServerCodegen.class);
7494

7595
public AbstractJavaJAXRSServerCodegen() {
@@ -138,6 +158,8 @@ public void addOperationToGroup(String tag, String resourcePath, Operation opera
138158
final List<CodegenOperation> opList = operations.computeIfAbsent(co.baseName, k -> new ArrayList<>());
139159
opList.add(co);
140160
}
161+
162+
allResourcePaths.add(resourcePath);
141163
}
142164

143165
@Override
@@ -182,7 +204,7 @@ public void preprocessOpenAPI(OpenAPI openAPI) {
182204

183205
@Override
184206
public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<ModelMap> allModels) {
185-
OperationsMap updatedObjs = jaxrsPostProcessOperations(objs);
207+
OperationsMap updatedObjs = jaxrsPostProcessOperations(objs, allResourcePaths);
186208
OperationMap operations = updatedObjs.getOperations();
187209
if (operations != null) {
188210
List<CodegenOperation> ops = operations.getOperation();
@@ -193,99 +215,164 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
193215
return updatedObjs;
194216
}
195217

218+
/** Delegates to {@link #jaxrsPostProcessOperations(OperationsMap, Set)} with no shadow check. */
196219
static OperationsMap jaxrsPostProcessOperations(OperationsMap objs) {
220+
return jaxrsPostProcessOperations(objs, null);
221+
}
222+
223+
/** Post-processes operations: normalizes metadata, computes common path, applies shadowing check. */
224+
private static OperationsMap jaxrsPostProcessOperations(OperationsMap objs, Set<String> allResourcePaths) {
197225
OperationMap operations = objs.getOperations();
198-
String commonPath = null;
199-
if (operations != null) {
200-
List<CodegenOperation> ops = operations.getOperation();
201-
for (CodegenOperation operation : ops) {
202-
if (operation.hasConsumes == Boolean.TRUE) {
203-
Map<String, String> firstType = operation.consumes.get(0);
204-
if (firstType != null) {
205-
if ("multipart/form-data".equals(firstType.get("mediaType"))) {
206-
operation.isMultipart = Boolean.TRUE;
207-
}
226+
if (operations == null) {
227+
return objs;
228+
}
229+
List<CodegenOperation> ops = operations.getOperation();
230+
231+
processOperationMetadata(ops);
232+
233+
String commonPath = computeCommonPath(ops);
234+
235+
if (commonPath != null && !commonPath.isEmpty() && !"/".equals(commonPath)
236+
&& wouldShadowOtherTags(commonPath, ops, allResourcePaths)) {
237+
commonPath = null;
238+
}
239+
240+
applyCommonPath(ops, commonPath, objs);
241+
242+
return objs;
243+
}
244+
245+
/** Normalizes consumes, responses, and return types for all operations. */
246+
private static void processOperationMetadata(List<CodegenOperation> ops) {
247+
for (CodegenOperation operation : ops) {
248+
if (operation.hasConsumes == Boolean.TRUE) {
249+
Map<String, String> firstType = operation.consumes.get(0);
250+
if (firstType != null) {
251+
if ("multipart/form-data".equals(firstType.get("mediaType"))) {
252+
operation.isMultipart = Boolean.TRUE;
208253
}
209254
}
255+
}
210256

211-
boolean isMultipartPost = false;
212-
List<Map<String, String>> consumes = operation.consumes;
213-
if (consumes != null) {
214-
for (Map<String, String> consume : consumes) {
215-
String mt = consume.get("mediaType");
216-
if (mt != null) {
217-
if (mt.startsWith("multipart/form-data")) {
218-
isMultipartPost = true;
219-
}
257+
boolean isMultipartPost = false;
258+
List<Map<String, String>> consumes = operation.consumes;
259+
if (consumes != null) {
260+
for (Map<String, String> consume : consumes) {
261+
String mt = consume.get("mediaType");
262+
if (mt != null) {
263+
if (mt.startsWith("multipart/form-data")) {
264+
isMultipartPost = true;
220265
}
221266
}
222267
}
268+
}
223269

224-
for (CodegenParameter parameter : operation.allParams) {
225-
if (isMultipartPost) {
226-
parameter.vendorExtensions.put("x-multipart", "true");
227-
}
270+
for (CodegenParameter parameter : operation.allParams) {
271+
if (isMultipartPost) {
272+
parameter.vendorExtensions.put("x-multipart", "true");
228273
}
274+
}
229275

230-
List<CodegenResponse> responses = operation.responses;
231-
if (responses != null) {
232-
for (CodegenResponse resp : responses) {
233-
if ("0".equals(resp.code)) {
234-
resp.code = "200";
235-
}
276+
List<CodegenResponse> responses = operation.responses;
277+
if (responses != null) {
278+
for (CodegenResponse resp : responses) {
279+
if ("0".equals(resp.code)) {
280+
resp.code = "200";
281+
}
236282

237-
if (resp.baseType == null) {
238-
resp.dataType = "void";
239-
resp.baseType = "Void";
240-
// set vendorExtensions.x-java-is-response-void to true as baseType is set to "Void"
241-
resp.vendorExtensions.put("x-java-is-response-void", true);
242-
}
283+
if (resp.baseType == null) {
284+
resp.dataType = "void";
285+
resp.baseType = "Void";
286+
// set vendorExtensions.x-java-is-response-void to true as baseType is set to "Void"
287+
resp.vendorExtensions.put("x-java-is-response-void", true);
288+
}
243289

244-
if ("array".equals(resp.containerType)) {
245-
resp.containerType = "List";
246-
resp.vendorExtensions.put(X_MICROPROFILE_OPEN_API_RETURN_SCHEMA_CONTAINER, SCHEMA_TYPE_ARRAY);
247-
} else if ("set".equals(resp.containerType)) {
248-
resp.containerType = "Set";
249-
resp.vendorExtensions.put(X_MICROPROFILE_OPEN_API_RETURN_SCHEMA_CONTAINER, SCHEMA_TYPE_ARRAY);
250-
resp.vendorExtensions.put(X_MICROPROFILE_OPEN_API_RETURN_UNIQUE_ITEMS, true);
251-
} else if ("map".equals(resp.containerType)) {
252-
resp.containerType = "Map";
253-
}
290+
if ("array".equals(resp.containerType)) {
291+
resp.containerType = "List";
292+
resp.vendorExtensions.put(X_MICROPROFILE_OPEN_API_RETURN_SCHEMA_CONTAINER, SCHEMA_TYPE_ARRAY);
293+
} else if ("set".equals(resp.containerType)) {
294+
resp.containerType = "Set";
295+
resp.vendorExtensions.put(X_MICROPROFILE_OPEN_API_RETURN_SCHEMA_CONTAINER, SCHEMA_TYPE_ARRAY);
296+
resp.vendorExtensions.put(X_MICROPROFILE_OPEN_API_RETURN_UNIQUE_ITEMS, true);
297+
} else if ("map".equals(resp.containerType)) {
298+
resp.containerType = "Map";
299+
}
254300

255-
if (resp.getResponseHeaders() != null) {
256-
handleHeaders(resp.getResponseHeaders());
257-
}
301+
if (resp.getResponseHeaders() != null) {
302+
handleHeaders(resp.getResponseHeaders());
258303
}
259304
}
305+
}
260306

261-
if (operation.returnBaseType == null) {
262-
operation.returnType = "void";
263-
operation.returnBaseType = "Void";
264-
// set vendorExtensions.x-java-is-response-void to true as returnBaseType is set to "Void"
265-
operation.vendorExtensions.put("x-java-is-response-void", true);
266-
}
307+
if (operation.returnBaseType == null) {
308+
operation.returnType = "void";
309+
operation.returnBaseType = "Void";
310+
// set vendorExtensions.x-java-is-response-void to true as returnBaseType is set to "Void"
311+
operation.vendorExtensions.put("x-java-is-response-void", true);
312+
}
267313

268-
if ("array".equals(operation.returnContainer)) {
269-
operation.returnContainer = "List";
270-
} else if ("set".equals(operation.returnContainer)) {
271-
operation.returnContainer = "Set";
272-
} else if ("map".equals(operation.returnContainer)) {
273-
operation.returnContainer = "Map";
274-
}
314+
if ("array".equals(operation.returnContainer)) {
315+
operation.returnContainer = "List";
316+
} else if ("set".equals(operation.returnContainer)) {
317+
operation.returnContainer = "Set";
318+
} else if ("map".equals(operation.returnContainer)) {
319+
operation.returnContainer = "Map";
320+
}
321+
}
322+
}
275323

276-
if (commonPath == null) {
277-
commonPath = operation.path;
278-
} else {
279-
commonPath = getCommonPath(commonPath, operation.path);
280-
}
324+
/** Computes the longest common path prefix shared by all operations. */
325+
private static String computeCommonPath(List<CodegenOperation> ops) {
326+
String commonPath = null;
327+
for (CodegenOperation operation : ops) {
328+
if (commonPath == null) {
329+
commonPath = operation.path;
330+
} else {
331+
commonPath = getCommonPath(commonPath, operation.path);
281332
}
333+
}
334+
return commonPath;
335+
}
336+
337+
/** Strips {@code commonPath} from operation paths and writes it to {@code objs}; null means shadowing was detected. */
338+
private static void applyCommonPath(List<CodegenOperation> ops, String commonPath, OperationsMap objs) {
339+
if (commonPath == null) {
340+
// Shadowing detected or no operations — keep full paths, set empty class-level prefix.
341+
for (CodegenOperation co : ops) {
342+
co.subresourceOperation = co.path.length() > 1;
343+
}
344+
objs.put("commonPath", StringUtils.EMPTY);
345+
} else {
282346
for (CodegenOperation co : ops) {
283347
co.path = StringUtils.removeStart(co.path, commonPath);
284348
co.subresourceOperation = co.path.length() > 1;
285349
}
286350
objs.put("commonPath", "/".equals(commonPath) ? StringUtils.EMPTY : commonPath);
287351
}
288-
return objs;
352+
}
353+
354+
/** Returns {@code true} if using {@code commonPath} as the class-level {@code @Path} would shadow routes of another tag. */
355+
private static boolean wouldShadowOtherTags(String commonPath, List<CodegenOperation> ops, Set<String> allResourcePaths) {
356+
if (allResourcePaths == null || allResourcePaths.isEmpty()) {
357+
return false;
358+
}
359+
360+
// Build the set of full paths owned by the current tag.
361+
Set<String> currentTagPaths = new HashSet<>();
362+
for (CodegenOperation co : ops) {
363+
currentTagPaths.add(co.path);
364+
}
365+
366+
// Check whether any path from a different tag would be shadowed by commonPath.
367+
for (String path : allResourcePaths) {
368+
if (currentTagPaths.contains(path)) {
369+
continue; // this path belongs to the current tag — not a shadow
370+
}
371+
if (path.startsWith(commonPath + "/") || path.equals(commonPath)) {
372+
return true;
373+
}
374+
}
375+
return false;
289376
}
290377

291378
private static void handleHeaders(List<CodegenParameter> headers) {

0 commit comments

Comments
 (0)