Escape JavaDoc default values safely - #24890
Conversation
Prevent generated Java clients from becoming invalid when OpenAPI default values contain a JavaDoc comment terminator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/Java/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/api.mustache:48">
P2: The `*/` escape is only applied to the base `api.mustache` and okhttp-gson; 12 other Java library templates (apache-httpclient, feign, jersey2/jersey3, microprofile, native, rest-assured, resteasy, resttemplate, retrofit2 and its play variants) still interpolate `default to {{.}}` into a JavaDoc comment unescaped. Generators using those libraries will still fail to compile when a default value contains `*/`. Apply `{{#escapeJavaDoc}}{{.}}{{/escapeJavaDoc}}` at every Javadoc default-value site for consistency.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * {{notes}} | ||
| {{#allParams}} | ||
| * @param {{paramName}} {{description}}{{#required}} (required){{/required}}{{^required}} (optional{{^isContainer}}{{#defaultValue}}, default to {{.}}{{/defaultValue}}{{/isContainer}}){{/required}} | ||
| * @param {{paramName}} {{description}}{{#required}} (required){{/required}}{{^required}} (optional{{^isContainer}}{{#defaultValue}}, default to {{#escapeJavaDoc}}{{.}}{{/escapeJavaDoc}}{{/defaultValue}}{{/isContainer}}){{/required}} |
There was a problem hiding this comment.
P2: The */ escape is only applied to the base api.mustache and okhttp-gson; 12 other Java library templates (apache-httpclient, feign, jersey2/jersey3, microprofile, native, rest-assured, resteasy, resttemplate, retrofit2 and its play variants) still interpolate default to {{.}} into a JavaDoc comment unescaped. Generators using those libraries will still fail to compile when a default value contains */. Apply {{#escapeJavaDoc}}{{.}}{{/escapeJavaDoc}} at every Javadoc default-value site for consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/api.mustache, line 48:
<comment>The `*/` escape is only applied to the base `api.mustache` and okhttp-gson; 12 other Java library templates (apache-httpclient, feign, jersey2/jersey3, microprofile, native, rest-assured, resteasy, resttemplate, retrofit2 and its play variants) still interpolate `default to {{.}}` into a JavaDoc comment unescaped. Generators using those libraries will still fail to compile when a default value contains `*/`. Apply `{{#escapeJavaDoc}}{{.}}{{/escapeJavaDoc}}` at every Javadoc default-value site for consistency.</comment>
<file context>
@@ -45,7 +45,7 @@ public class {{classname}} {
* {{notes}}
{{#allParams}}
- * @param {{paramName}} {{description}}{{#required}} (required){{/required}}{{^required}} (optional{{^isContainer}}{{#defaultValue}}, default to {{.}}{{/defaultValue}}{{/isContainer}}){{/required}}
+ * @param {{paramName}} {{description}}{{#required}} (required){{/required}}{{^required}} (optional{{^isContainer}}{{#defaultValue}}, default to {{#escapeJavaDoc}}{{.}}{{/escapeJavaDoc}}{{/defaultValue}}{{/isContainer}}){{/required}}
{{/allParams}}
{{#returnType}}
</file context>
There was a problem hiding this comment.
Fixed in commit 7d8795b. All remaining JavaDoc default-value interpolations across the Java library templates are now wrapped with escapeJavaDoc, including the four okhttp-gson sites.\n\n---\nDrafted-by: Copilot (no human review before posting)
Ensure every Java library template protects generated comments from default values containing a comment terminator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 16 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Avoid applying JavaDoc escaping to a regular source comment where it changes the displayed default value unnecessarily. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Fixed the latest review issue in commit 00f801c. The native template line comment no longer uses JavaDoc escaping; all actual JavaDoc default-value sites remain protected.\n\n---\nDrafted-by: Copilot (no human review before posting) |
|
Do we see any particular benefit with doing this as a mustache-lamba rather than doing it in the generator itself? Given that comments is an attack vector, and the it most likely something that should be heavily sanitized, I believe having it in code might make more sense. |
|
Thanks for raising this. I considered moving the escaping into the generator, but kept it as a Mustache lambda intentionally because the same The lambda is registered once in If you prefer a generator-side implementation, I can introduce a separate |
|
I think the best approach would be some sort of "sanitizedDefaultValue" that was shared for all generators, and then that each language is expected to override it. But as you say it is a very large change given how coupled the current |
|
@Mattias-Sehlstedt That makes sense. I agree that introducing a shared sanitized default value would expand the scope significantly given the current coupling. Let's keep this PR focused and handle the broader approach in a separate issue. |
The escapeJavaDoc lambda is registered in the codegen lambda map, so
templates must invoke it as {{#lambda.escapeJavaDoc}}. The bare
{{#escapeJavaDoc}} reference never resolves in the render context,
silently dropping every default value from @PARAM Javadoc lines and
breaking the "Samples up-to-date" check.
Add a minimal wildcard-accept-header spec (Accept header defaulting to
*/*) and its generated okhttp-gson sample so the escaping is visible:
the value now renders as *&OpenAPITools#47;* instead of prematurely closing the
Javadoc block.
Also tighten EscapeJavaDocLambdaTest to exercise the lambda through the
lambda map in a nested defaultValue section, the real codegen pattern.
There was a problem hiding this comment.
1 issue found across 60 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/wildcard-accept-header/java/.travis.yml">
<violation number="1" location="samples/client/wildcard-accept-header/java/.travis.yml:20">
P2: The script chmods `./gradlew` in before_install but then runs `gradle test` (system Gradle) instead of the wrapper. This bypasses the pinned Gradle 8.14.5 in gradle/wrapper/gradle-wrapper.properties, and the build's spotless 6.11.0 requires Gradle 7+, so an older system Gradle on Travis will fail. Use `./gradlew test`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| # test using maven | ||
| #- mvn test | ||
| # test using gradle | ||
| - gradle test |
There was a problem hiding this comment.
P2: The script chmods ./gradlew in before_install but then runs gradle test (system Gradle) instead of the wrapper. This bypasses the pinned Gradle 8.14.5 in gradle/wrapper/gradle-wrapper.properties, and the build's spotless 6.11.0 requires Gradle 7+, so an older system Gradle on Travis will fail. Use ./gradlew test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/.travis.yml, line 20:
<comment>The script chmods `./gradlew` in before_install but then runs `gradle test` (system Gradle) instead of the wrapper. This bypasses the pinned Gradle 8.14.5 in gradle/wrapper/gradle-wrapper.properties, and the build's spotless 6.11.0 requires Gradle 7+, so an older system Gradle on Travis will fail. Use `./gradlew test`.</comment>
<file context>
@@ -0,0 +1,22 @@
+ # test using maven
+ #- mvn test
+ # test using gradle
+ - gradle test
+ # test using sbt
+ # - sbt test
</file context>
| - gradle test | |
| - ./gradlew test |
String example values are escaped for use inside a double-quoted Java string literal. Avoid mangling '*/' and '/*' (e.g. the '*/*' media type) by not applying escapeUnsafeCharacters, which is only needed for javadoc comment contexts. Also remove the unused HashMap import in EscapeJavaDocLambdaTest.
|
Addressed the latest review items: #2 — unused #3 — corrupted example #1 — |
Description
Generated Java clients fail to compile when an OpenAPI parameter default value contains
*/, such as*/*. The value is currently interpolated directly into a JavaDoc comment, prematurely closing the comment block.Register a Java-specific Mustache lambda and apply it at the JavaDoc interpolation site. The lambda replaces the slash in
*/with the HTML entity/, preserving the rendered documentation value without changing the generated Java semantics.A regression test covers comment terminators and leaves ordinary values unchanged.
Fixes #24852
Validation
./mvnw -B -pl modules/openapi-generator -am -Dtest=org.openapitools.codegen.templating.mustache.EscapeJavaDocLambdaTest -Dsurefire.failIfNoSpecifiedTests=false testWas generative AI tooling used to co-author this PR?
Generated-by: Copilot following the guidelines in the project contribution documentation.
Drafted-by: Copilot (no human review before posting)
Summary by cubic
Fixes generated Java clients failing to compile when a default value contains
*/(for example*/*), which prematurely closed the JavaDoc comment. Default values in JavaDoc are now escaped to*/, preserving rendered text and generated behavior, and string example values like*/*are no longer mangled in generated source.escapeJavaDoclambda in the codegen lambda map and invokes it as{{#lambda.escapeJavaDoc}}in templates; the bare{{#escapeJavaDoc}}reference never resolves and silently drops default values.escapeUnsafeCharacters, so wildcard media types stay intact.wildcard-accept-headersample covering comment terminators and ordinary values.Written for commit f9a9903. Summary will update on new commits.