Copilot commented on code in PR #3880:
URL: https://github.com/apache/avro/pull/3880#discussion_r3610813067
##########
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<SpecificCompiler.OutputFile> 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);
+ }
+ }
Review Comment:
The end-to-end assertion only checks lines whose trimmed text starts with
"/**" or "*". But the classic templates emit docs as a single-line Javadoc
(`/** $doc */`), so if a schema doc contains newlines (already exercised in
docsAreEscaped_avro4053), the middle lines of that Javadoc block won’t start
with `*` and won’t be checked. This makes the test miss potential regressions
where raw backslashes appear on subsequent lines within the same Javadoc
comment block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
