From 5510072d95212741400bd290a211b98faf12cb7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 19 Jul 2026 17:04:56 +0200 Subject: [PATCH 1/2] AVRO-4311: [java] Escape backslashes in generated Javadoc The code generator writes schema documentation strings into the Javadoc comments of generated Java sources. escapeForJavadoc escaped the comment terminator and HTML metacharacters but left backslashes untouched. The Java compiler translates Unicode escapes (\uXXXX) across the whole source file, including inside comments, before comments are recognized (JLS 3.3). A documentation string containing backslash sequences could therefore be reinterpreted by the compiler and change the generated source in unintended ways. Neutralize backslashes in escapeForJavadoc by encoding them as the HTML entity \, so documentation content is always emitted as inert text. Add a regression test covering several escape forms. The string-literal path (escapeForJavaString) already doubles backslashes and is unaffected. --- .../compiler/specific/SpecificCompiler.java | 14 +++++- .../specific/TestSpecificCompiler.java | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java index ea1e1a11b55..3d4e4263239 100644 --- a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java +++ b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java @@ -1109,10 +1109,20 @@ public static String javaEscape(String o) { } /** - * Utility for template use. Escapes comment end with HTML entities. + * Utility for template use. Escapes content emitted into a Javadoc comment. + * + *

+ * As well as escaping the comment terminator ({@code *}{@code /}) and HTML + * metacharacters, this neutralizes backslashes. This is required because the + * Java compiler translates Unicode escapes (of the form {@code \}{@code uXXXX}) + * across the whole source file, including inside comments, as its first lexical + * step (JLS §3.3). Without this, a schema doc value such as + * {@code \}{@code u002a\}{@code u002f} would be decoded by the compiler to + * {@code *}{@code /}, prematurely closing the comment and allowing arbitrary + * code to be injected into the generated source. */ public static String escapeForJavadoc(String s) { - return s.replace("*/", "*/").replace("<", "<").replace(">", ">"); + return s.replace("\\", "\").replace("*/", "*/").replace("<", "<").replace(">", ">"); } /** diff --git a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java index 178f2fa2100..ea9abbb17c3 100644 --- a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java +++ b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java @@ -1031,6 +1031,52 @@ void docsAreEscaped_avro4053() { } } + @Test + void unicodeEscapesInDocsAreNeutralized() { + // The Java compiler decodes Unicode escapes (\ uXXXX) across the whole source + // file, including inside comments, before comments are recognized (JLS 3.3). + // A doc value carrying the literal text "\ u002a\ u002f" therefore decodes to + // "*/" at compile time and could close the generated Javadoc comment early, + // enabling arbitrary code injection. Since a Unicode escape always requires a + // literal backslash, escapeForJavadoc neutralizes every backslash, which + // covers all escape variants at once. + String[] maliciousDocs = { // + "\\u002a\\u002f static { System.exit(1); } \\u002f\\u002a", // basic form + "\\uuuu002a\\uuuu002f System.exit(1);", // multiple 'u's are legal (JLS 3.3) + "\\u005cu002a\\u005cu002f System.exit(1);", // escape that would decode to a backslash + "\\U002A\\u002F", // uppercase hex / uppercase-U decoy + "prefix\\\\u002a\\\\u002f even-backslash-run", // even run of backslashes + "literal */ static { System.exit(1); } /* comment close" // no escape at all + }; + + for (String maliciousDoc : maliciousDocs) { + // Unit-level check on the escaping utility itself. + String escaped = SpecificCompiler.escapeForJavadoc(maliciousDoc); + assertFalse(escaped.contains("\\"), "Backslashes must be neutralized: " + escaped); + assertFalse(escaped.contains("*/"), "Comment terminator must be neutralized: " + escaped); + + // End-to-end check: no raw backslash may reach the generated source's doc + // comments. A Java Unicode escape always requires a literal backslash, so the + // absence of backslashes in every comment line proves no \ uXXXX sequence can + // be reconstituted by the compiler to close the comment. + Schema schema = SchemaBuilder.record("EvilRecord").namespace("org.apache.avro.codegentest.testdata") + .doc(maliciousDoc).fields().name("field").doc(maliciousDoc).type().stringType().noDefault().endRecord(); + Collection outputs = new SpecificCompiler(schema).compile(); + assertEquals(1, outputs.size()); + for (SpecificCompiler.OutputFile outputFile : outputs) { + // Inspect only Javadoc/comment lines. The schema string literal (emitted via + // escapeForJavaString, which doubles backslashes and is therefore immune) is + // on a code line and is intentionally excluded. + for (String line : outputFile.contents.split("\n")) { + String trimmed = line.trim(); + if (trimmed.startsWith("/**") || trimmed.startsWith("*")) { + assertFalse(line.contains("\\"), "Raw backslash reached doc comment: " + line); + } + } + } + } + } + private int countOccurrences(Pattern pattern, String textToSearch) { int count = 0; for (Matcher matcher = pattern.matcher(textToSearch); matcher.find();) { From 9b6b2f1fec8a448d94c9311b52b88db75d8299fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 20 Jul 2026 09:25:18 +0200 Subject: [PATCH 2/2] AVRO-4311: [java] Strengthen Javadoc escaping regression test Address review feedback: the end-to-end assertion only inspected lines starting with "/**" or "*", so the middle physical lines of a multi-line Javadoc block (a doc containing newlines) were not checked. Replace the line-based scan with one that removes all Java string literals (the embedded schema, which legitimately contains doubled backslashes) and then asserts no backslash remains in the surrounding code and comments. Add a doc vector that spans multiple physical lines. --- .../specific/TestSpecificCompiler.java | 50 ++++++++++++++----- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java index ea9abbb17c3..62d199376ba 100644 --- a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java +++ b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java @@ -1046,7 +1046,8 @@ void unicodeEscapesInDocsAreNeutralized() { "\\u005cu002a\\u005cu002f System.exit(1);", // escape that would decode to a backslash "\\U002A\\u002F", // uppercase hex / uppercase-U decoy "prefix\\\\u002a\\\\u002f even-backslash-run", // even run of backslashes - "literal */ static { System.exit(1); } /* comment close" // no escape at all + "literal */ static { System.exit(1); } /* comment close", // no escape at all + "first line\\u002a\\u002f\nsecond line \\u002f\\u002a end" // spans multiple physical lines }; for (String maliciousDoc : maliciousDocs) { @@ -1055,26 +1056,49 @@ void unicodeEscapesInDocsAreNeutralized() { assertFalse(escaped.contains("\\"), "Backslashes must be neutralized: " + escaped); assertFalse(escaped.contains("*/"), "Comment terminator must be neutralized: " + escaped); - // End-to-end check: no raw backslash may reach the generated source's doc - // comments. A Java Unicode escape always requires a literal backslash, so the - // absence of backslashes in every comment line proves no \ uXXXX sequence can - // be reconstituted by the compiler to close the comment. + // End-to-end check: no raw backslash may reach the generated source outside of + // string literals. A Java Unicode escape always requires a literal backslash, + // so the absence of backslashes everywhere except string literals proves no + // \ uXXXX sequence can be reconstituted by the compiler to close a comment. Schema schema = SchemaBuilder.record("EvilRecord").namespace("org.apache.avro.codegentest.testdata") .doc(maliciousDoc).fields().name("field").doc(maliciousDoc).type().stringType().noDefault().endRecord(); Collection outputs = new SpecificCompiler(schema).compile(); assertEquals(1, outputs.size()); for (SpecificCompiler.OutputFile outputFile : outputs) { - // Inspect only Javadoc/comment lines. The schema string literal (emitted via - // escapeForJavaString, which doubles backslashes and is therefore immune) is - // on a code line and is intentionally excluded. - for (String line : outputFile.contents.split("\n")) { - String trimmed = line.trim(); - if (trimmed.startsWith("/**") || trimmed.startsWith("*")) { - assertFalse(line.contains("\\"), "Raw backslash reached doc comment: " + line); - } + // Remove Java string literals (the schema is embedded via escapeForJavaString, + // which doubles backslashes and is therefore immune) so that the remaining + // text is code and comments only. This checks every line of every Javadoc + // block, including the middle lines of a multi-line doc comment. + String withoutStringLiterals = removeJavaStringLiterals(outputFile.contents); + assertFalse(withoutStringLiterals.contains("\\"), + "Raw backslash reached generated code/comments: " + outputFile.path); + } + } + } + + /** + * Returns the given Java source with the content of all double-quoted string + * literals removed, so tests can assert on code and comments without matching + * the (legitimately backslash-containing) embedded schema string literal. + */ + private String removeJavaStringLiterals(String source) { + StringBuilder out = new StringBuilder(source.length()); + boolean inString = false; + for (int i = 0; i < source.length(); i++) { + char c = source.charAt(i); + if (inString) { + if (c == '\\') { + i++; // skip the escaped character (e.g. \" or \\) + } else if (c == '"') { + inString = false; } + } else if (c == '"') { + inString = true; + } else { + out.append(c); } } + return out.toString(); } private int countOccurrences(Pattern pattern, String textToSearch) {