From b53e435804fc37b75c8a9926eb419265c22b13f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Quenaudon?= Date: Fri, 15 May 2026 12:21:11 +0100 Subject: [PATCH] Safer sanitising in Java/Kotlin generators --- .../com/squareup/wire/java/JavaGenerator.java | 9 ++-- .../squareup/wire/java/JavaGeneratorTest.java | 50 ++++++++++++++++++- .../squareup/wire/kotlin/KotlinGenerator.kt | 7 ++- .../wire/kotlin/KotlinGeneratorTest.kt | 23 +++++++++ .../com/squareup/wire/swift/SwiftGenerator.kt | 4 +- 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java b/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java index 0ce3131a14..505835c77f 100644 --- a/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java +++ b/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java @@ -623,6 +623,7 @@ static String sanitizeJavadoc(String documentation) { documentation = documentation.replaceAll("[^\\S\n]+\n", "\n"); documentation = documentation.replaceAll("\\s+$", ""); documentation = documentation.replaceAll("\\*/", "*/"); + documentation = documentation.replaceAll("/\\*", "/*"); // Rewrite '@see ' to use an html anchor tag documentation = documentation.replaceAll("@see (http:" + URL_CHARS + "+)", "@see $1"); @@ -829,7 +830,9 @@ private TypeSpec generateMessage(MessageType type) { } fieldBuilder.addAnnotation(wireFieldAnnotation(nameAllocator, field, type)); if (field.isExtension()) { - fieldBuilder.addJavadoc("Extension source: $L\n", field.getLocation().withPathOnly()); + fieldBuilder.addJavadoc( + "Extension source: $L\n", + sanitizeJavadoc(field.getLocation().withPathOnly().toString())); } if (field.isDeprecated()) { fieldBuilder.addAnnotation(Deprecated.class); @@ -900,7 +903,7 @@ private TypeSpec generateEnclosingType(EnclosingType type) { documentation += "NOTE: This type only exists to maintain class structure" + " for its nested types and is not an actual message."; - builder.addJavadoc("$L\n", documentation); + builder.addJavadoc("$L\n", sanitizeJavadoc(documentation)); builder.addMethod( MethodSpec.constructorBuilder() @@ -2392,7 +2395,7 @@ public ClassName generatedTypeName(ProtoMember member) { .build()); if (!field.getDocumentation().isEmpty()) { - builder.addJavadoc("$L\n", field.getDocumentation()); + builder.addJavadoc("$L\n", sanitizeJavadoc(field.getDocumentation())); } builder.addMethod( diff --git a/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java b/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java index 671fae6b0e..d9d870f748 100644 --- a/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java +++ b/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java @@ -46,10 +46,58 @@ public void sanitizeJavadocWrapsSeeLinks() { @Test public void sanitizeJavadocStarSlash() { String input = "/* comment inside comment. */"; - String expected = "/* comment inside comment. */"; + String expected = "/* comment inside comment. */"; assertThat(JavaGenerator.sanitizeJavadoc(input)).isEqualTo(expected); } + @Test + public void generatedOptionTypeSanitizesJavadoc() throws Exception { + Schema schema = + new SchemaBuilder() + .add( + Path.get("message.proto"), + "" + + "syntax = \"proto2\";\n" + + "import \"google/protobuf/descriptor.proto\";\n" + + "message Message {\n" + + " extend google.protobuf.MessageOptions {\n" + + " // */ class Cheeky { } /*\n" + + " optional string owner = 55682;\n" + + " }\n" + + "}\n") + .build(); + + String javaOutput = + new JavaWithProfilesGenerator(schema).generateJava("Message", null, false, true); + + assertThat(javaOutput).contains("*/ class Cheeky { } /*"); + assertThat(javaOutput).doesNotContain("*/ class Cheeky"); + } + + @Test + public void enclosingTypeSanitizesJavadoc() throws Exception { + Schema schema = + new SchemaBuilder() + .add( + Path.get("message.proto"), + "" + + "// */ class Cheeky { } /*\n" + + "message A {\n" + + " message B {\n" + + " }\n" + + " optional B b = 1;\n" + + "}\n") + .build(); + + Schema pruned = schema.prune(new PruningRules.Builder().addRoot("A.B").build()); + JavaGenerator javaGenerator = JavaGenerator.get(schema); + TypeSpec typeSpec = javaGenerator.generateType(pruned.getType("A")); + String javaOutput = JavaFile.builder("", typeSpec).build().toString(); + + assertThat(javaOutput).contains("*/ class Cheeky { } /*"); + assertThat(javaOutput).doesNotContain("*/ class Cheeky"); + } + @Test public void generateTypeUsesNameAllocatorInMessageBuilderBuild() throws Exception { Schema schema = diff --git a/wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt b/wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt index 14954122c4..c772fe07de 100644 --- a/wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt +++ b/wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt @@ -1349,7 +1349,10 @@ class KotlinGenerator private constructor( addKdoc("%L\n", field.documentation.sanitizeKdoc()) } if (field.isExtension) { - addKdoc("Extension source: %L\n", field.location.withPathOnly()) + addKdoc( + "Extension source: %L\n", + field.location.withPathOnly().toString().sanitizeKdoc(), + ) } } return parameterSpec.build() to propertySpec.build() @@ -2877,7 +2880,7 @@ class KotlinGenerator private constructor( .build(), ) if (field.documentation.isNotEmpty()) { - builder.addKdoc("%L\n", field.documentation) + builder.addKdoc("%L\n", field.documentation.sanitizeKdoc()) } builder.primaryConstructor( FunSpec.constructorBuilder() diff --git a/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinGeneratorTest.kt b/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinGeneratorTest.kt index 5de4a7e3ce..2e10c81437 100644 --- a/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinGeneratorTest.kt +++ b/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinGeneratorTest.kt @@ -1483,6 +1483,29 @@ class KotlinGeneratorTest { assertThat(input.sanitizeKdoc()).isEqualTo(expected) } + @Test fun generatedOptionTypeSanitizesKdoc() { + val schema = buildSchema { + add( + "message.proto".toPath(), + """ + |syntax = "proto2"; + |import "google/protobuf/descriptor.proto"; + |message Message { + | extend google.protobuf.MessageOptions { + | // */ class Cheeky { } /* + | optional string owner = 55682; + | } + |} + """.trimMargin(), + ) + } + + val code = KotlinWithProfilesGenerator(schema).generateKotlin("Message") + + assertThat(code).contains("*/ class Cheeky { } /*") + assertThat(code).doesNotContain("*/ class Cheeky") + } + @Test fun handleLongIdentifiers() { val longType = "MessageWithNameLongerThan100Chars00000000000000000000000000000000000000000000000000000000000000000000" diff --git a/wire-swift-generator/src/main/java/com/squareup/wire/swift/SwiftGenerator.kt b/wire-swift-generator/src/main/java/com/squareup/wire/swift/SwiftGenerator.kt index 5c6d62843b..1245ee5e32 100644 --- a/wire-swift-generator/src/main/java/com/squareup/wire/swift/SwiftGenerator.kt +++ b/wire-swift-generator/src/main/java/com/squareup/wire/swift/SwiftGenerator.kt @@ -1200,7 +1200,7 @@ class SwiftGenerator private constructor( if (field.documentation.isNotBlank()) { addDoc("\n%L\n", field.documentation.sanitizeDoc()) } - addDoc("\nSource: %L\n", field.location.withPathOnly()) + addDoc("\nSource: %L\n", field.location.withPathOnly().toString().sanitizeDoc()) if (field.isDeprecated) { addAttribute(deprecated) @@ -1266,7 +1266,7 @@ class SwiftGenerator private constructor( addDoc("\n%L\n", field.documentation.sanitizeDoc()) } if (!forStorageType) { - addDoc("\nSource: %L\n", field.location.withPathOnly()) + addDoc("\nSource: %L\n", field.location.withPathOnly().toString().sanitizeDoc()) } if (field.isDeprecated) {