From db9e70bdd482b4771faa7029416938c0798bc02a Mon Sep 17 00:00:00 2001 From: Paul King Date: Tue, 18 Aug 2026 18:51:29 +1000 Subject: [PATCH] GROOVY-12272: Validate JSON string escapes where they are read JsonLexer's string branch appended one character at a time and, at every unescaped quote, re-validated the whole accumulated token by copying it to a String and running two regular expressions over it. For a well formed string the closing quote is the first unescaped one, so that happened once. For a string holding an invalid escape the validation could never succeed, so the loop consumed the rest of the document and paid the cost again at every quote it passed, giving O(n^2) behaviour on input an author controls. Measured on a document of the shape {"k":"\q" followed by n quotes, parsed with JsonSlurperClassic: quotes before after 2,000 42 ms 4 ms 4,000 67 ms 0 ms 8,000 220 ms 0 ms 16,000 874 ms 0 ms Read and check each escape sequence where the backslash is found instead. The scan becomes linear, the accepted language is unchanged, and a bad escape is now reported at its own position rather than after the document has been consumed to its end. Escape-state tracking is no longer needed, since consuming the sequence in place is what distinguishes an escaped quote from a closing one. Note the reach is wider than the parser: JsonOutput.prettyPrint(String) lexes through the same class, so it shared the behaviour. --- .../src/main/java/groovy/json/JsonLexer.java | 46 +++++++++++++++++-- .../JsonSlurperMalformedStringTest.groovy | 44 ++++++++++++++++++ 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/subprojects/groovy-json/src/main/java/groovy/json/JsonLexer.java b/subprojects/groovy-json/src/main/java/groovy/json/JsonLexer.java index 4ec3fa5d2be..11b60e429ae 100644 --- a/subprojects/groovy-json/src/main/java/groovy/json/JsonLexer.java +++ b/subprojects/groovy-json/src/main/java/groovy/json/JsonLexer.java @@ -45,6 +45,10 @@ public class JsonLexer implements Iterator { private static final char UPPER_E = 'E'; private static final char ZERO = '0'; private static final char NINE = '9'; + /** The characters which may follow a backslash in a JSON string, other than {@code u}. */ + private static final String SIMPLE_ESCAPES = "\"\\/bfnrt"; + /** The digits a {@code \\u} escape may carry; deliberately ASCII only. */ + private static final String HEX_DIGITS = "0123456789abcdefABCDEF"; private final LineColumnReader reader; @@ -107,18 +111,19 @@ public JsonToken nextToken() { StringBuilder currentContent = new StringBuilder("\""); // consume the first double quote starting the string reader.read(); - boolean isEscaped = false; for (;;) { int read = reader.read(); if (read == -1) return null; - isEscaped = (!isEscaped && currentContent.charAt(currentContent.length() - 1) == '\\'); - char charRead = (char) read; currentContent.append(charRead); - if (charRead == '"' && !isEscaped && - possibleTokenType.matching(currentContent.toString())) { + if (charRead == '\\') { + // Consume and check the escape sequence where it is read. Validating + // here keeps the scan linear in the length of the token, and reports + // the position of the offending escape rather than of the token end. + readEscapeSequence(currentContent, possibleTokenType); + } else if (charRead == '"') { token.setEndLine(reader.getLine()); token.setEndColumn(reader.getColumn()); token.setText(unescape(currentContent.toString())); @@ -163,6 +168,37 @@ public JsonToken nextToken() { } } + /** + * Reads the remainder of an escape sequence whose backslash has already been consumed, + * appending it to the token content and rejecting anything RFC 8259 does not allow to + * follow a backslash. + * + * @param currentContent the token content read so far, ending in the backslash + * @param type the token type being matched, for the error message + * @throws IOException if reading fails + */ + private void readEscapeSequence(StringBuilder currentContent, JsonTokenType type) throws IOException { + int escapeRead = reader.read(); + if (escapeRead == -1) return; // unterminated; the caller stops at end of input + char escapeChar = (char) escapeRead; + currentContent.append(escapeChar); + + if (escapeChar == 'u') { + for (int i = 0; i < 4; i += 1) { + int hexRead = reader.read(); + if (hexRead == -1) return; + char hexChar = (char) hexRead; + currentContent.append(hexChar); + // Character.digit would accept non-ASCII digits, which JSON does not. + if (HEX_DIGITS.indexOf(hexChar) == -1) { + throwJsonException(currentContent.toString(), type); + } + } + } else if (SIMPLE_ESCAPES.indexOf(escapeChar) == -1) { + throwJsonException(currentContent.toString(), type); + } + } + private void throwJsonException(String content, JsonTokenType type) { throw new JsonException( "Lexing failed on line: " + diff --git a/subprojects/groovy-json/src/test/groovy/groovy/json/JsonSlurperMalformedStringTest.groovy b/subprojects/groovy-json/src/test/groovy/groovy/json/JsonSlurperMalformedStringTest.groovy index 5dc44817b46..56f6811a7a2 100644 --- a/subprojects/groovy-json/src/test/groovy/groovy/json/JsonSlurperMalformedStringTest.groovy +++ b/subprojects/groovy-json/src/test/groovy/groovy/json/JsonSlurperMalformedStringTest.groovy @@ -133,6 +133,50 @@ class JsonSlurperMalformedStringTest { assertValidUnicodeEscapes('JsonSlurperClassic') { String doc -> slurper.parseText(doc).k } } + /** + * GROOVY-12272: the lexer used to re-validate the whole accumulated token at every unescaped + * quote, so a string holding an invalid escape followed by many quotes cost O(n^2). The + * escape is now checked where it is read, which both bounds the work and reports the + * position of the offending escape rather than of the end of the document. + */ + @Test + void testInvalidEscapeFollowedByManyQuotesStaysLinear() { + def document = { int quotes -> '{"k":"\\q' + ('"' * quotes) + '"}' } + + // Warm up so the measurement is not dominated by class loading and JIT. + 3.times { parseIgnoringFailure(document(2000)) } + + long small = timeParse(document(4000)) + long large = timeParse(document(16000)) + + // Four times the input. Quadratic cost would be about sixteen times the work; allow a + // wide margin so the test is about the growth curve, not the absolute speed. + assert large < Math.max(small, 5) * 8, + "parse time grew from ${small}ms to ${large}ms for a 4x larger document" + } + + @Test + void testInvalidEscapeIsReportedWhereItOccurs() { + def e = shouldFail(JsonException) { + new JsonSlurperClassic().parseText('{"k":"a\\qb"}') + } + // The report names the escape, not the whole remaining document. + assert e.message.contains('\\q') + } + + private static void parseIgnoringFailure(String text) { + try { + new JsonSlurperClassic().parseText(text) + } catch (JsonException ignored) { + } + } + + private static long timeParse(String text) { + long start = System.nanoTime() + parseIgnoringFailure(text) + (System.nanoTime() - start) / 1_000_000L + } + private static void assertRejected(String context, String description, Closure parse) { def thrown = null try {