From 2e3d02a2d8fccae3f08e6990c1e1f4f875a392fa Mon Sep 17 00:00:00 2001 From: Elankumaran Srinivasan <5340827+elang2@users.noreply.github.com> Date: Sat, 5 Sep 2026 23:21:25 -0700 Subject: [PATCH 1/9] Fix #1683: reject JSON-escaped lone surrogates in ReaderBasedJsonParser Mirror of #1541 for the Reader-based parser: the \uXXXX escape decoder now rejects lone / reversed surrogates in field-name and string-value positions, plus _skipString for consistency with skipChildren() callers. Error messages match #1541's wording. Ships strict-default with no JsonReadFeature gate, matching #1542/#1583. --- release-notes/CREDITS-2.x | 6 + release-notes/VERSION-2.x | 4 + .../core/json/ReaderBasedJsonParser.java | 149 ++++++- ...EscapedSurrogateInStringValue1683Test.java | 411 ++++++++++++++++++ 4 files changed, 569 insertions(+), 1 deletion(-) create mode 100644 src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 910b174c16..0ca07d1411 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -554,3 +554,9 @@ Burak KALAYCI (@kalayciburak) * Contributed #1668: `WriterBasedJsonGenerator` AIOOBE when a zero-length custom escape lands at the output buffer boundary (2.21.7) + +Elankumaran Srinivasan (@elang2) + * Contributed #1683: `ReaderBasedJsonParser` should reject JSON-escaped lone + surrogates in field names and string values (mirror of #1541 fix in + `UTF8StreamJsonParser`) + (2.23.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 604d157df3..29f27c766d 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -20,6 +20,10 @@ a pure JSON library. based on supplied length (requested by @kilink) (contributed by @seonwooj0810) +#1683: `ReaderBasedJsonParser` should reject JSON-escaped lone + surrogates in field names and string values (mirror of #1541 + fix in `UTF8StreamJsonParser`) + (contributed by @elang2) 2.22.3 (not yet released) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java index c9e13cee66..81dac5c2ee 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java @@ -1866,6 +1866,46 @@ private String _parseName2(int startPtr, int hash, int endChar) throws IOExcepti * For now let's assume it does not. */ c = _decodeEscaped(); + // [jackson-core#1683]: Validate JSON-escaped surrogates in + // field name. Mirror of [jackson-core#1541] fix in + // UTF8StreamJsonParser. + if (c >= 0xD800 && c <= 0xDFFF) { + if (c < 0xDC00) { // high surrogate: must be followed by low surrogate escape + char hi = c; + if (_inputPtr >= _inputEnd) { + if (!_loadMore()) { + _reportInvalidEOF(" in field name", JsonToken.FIELD_NAME); + } + } + if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "field name"); + } + ++_inputPtr; + char lo = _decodeEscaped(); + if (lo < 0xDC00 || lo > 0xDFFF) { + _reportBrokenSurrogatePair(lo, "field name"); + } + // Store as two UTF-16 code units. Hash includes the low + // surrogate below; add high surrogate here. + hash = (hash * CharsToNameCanonicalizer.HASH_MULT) + hi; + if (outPtr >= outBuf.length) { + totalLen += outBuf.length; + _streamReadConstraints.validateNameLength(totalLen); + outBuf = _textBuffer.finishCurrentSegment(); + outPtr = 0; + } + outBuf[outPtr++] = hi; + if (outPtr >= outBuf.length) { + totalLen += outBuf.length; + _streamReadConstraints.validateNameLength(totalLen); + outBuf = _textBuffer.finishCurrentSegment(); + outPtr = 0; + } + c = lo; + } else { // lone low surrogate + _reportUnexpectedLowSurrogate(c, "field name"); + } + } } else if (i <= endChar) { if (i == endChar) { break; @@ -2086,6 +2126,36 @@ protected JsonToken _handleApos() throws IOException // an UTF-16 surrogate pair, does that affect decoding? // For now let's assume it does not. c = _decodeEscaped(); + // [jackson-core#1683]: Validate JSON-escaped surrogates in + // apostrophe-quoted string value. + if (c >= 0xD800 && c <= 0xDFFF) { + if (c < 0xDC00) { // high surrogate + char hi = c; + if (_inputPtr >= _inputEnd) { + if (!_loadMore()) { + _reportInvalidEOF( + ": was expecting closing quote for a string value", + JsonToken.VALUE_STRING); + } + } + if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "string value"); + } + ++_inputPtr; + char lo = _decodeEscaped(); + if (lo < 0xDC00 || lo > 0xDFFF) { + _reportBrokenSurrogatePair(lo, "string value"); + } + if (outPtr >= outBuf.length) { + outBuf = _textBuffer.finishCurrentSegment(); + outPtr = 0; + } + outBuf[outPtr++] = hi; + c = lo; + } else { // lone low surrogate + _reportUnexpectedLowSurrogate(c, "string value"); + } + } } else if (i <= '\'') { if (i == '\'') { break; @@ -2214,6 +2284,39 @@ protected void _finishString2() throws IOException * For now let's assume it does not. */ c = _decodeEscaped(); + // [jackson-core#1683]: Validate JSON-escaped surrogates in + // string value. Mirror of [jackson-core#1541] fix in + // UTF8StreamJsonParser, applied to the Reader-based path. + if (c >= 0xD800 && c <= 0xDFFF) { + if (c < 0xDC00) { // high surrogate: must be followed by low surrogate escape + char hi = c; + if (_inputPtr >= _inputEnd) { + if (!_loadMore()) { + _reportInvalidEOF( + ": was expecting closing quote for a string value", + JsonToken.VALUE_STRING); + } + } + if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "string value"); + } + ++_inputPtr; + char lo = _decodeEscaped(); + if (lo < 0xDC00 || lo > 0xDFFF) { + _reportBrokenSurrogatePair(lo, "string value"); + } + // Emit high surrogate first, then fall through so the + // low surrogate is appended by the normal path below. + if (outPtr >= outBuf.length) { + outBuf = _textBuffer.finishCurrentSegment(); + outPtr = 0; + } + outBuf[outPtr++] = hi; + c = lo; + } else { // lone low surrogate + _reportUnexpectedLowSurrogate(c, "string value"); + } + } } else if (i < INT_SPACE) { _throwUnquotedSpace(i, "string value"); } // anything else? @@ -2262,7 +2365,33 @@ protected final void _skipString() throws IOException // Although chars outside of BMP are to be escaped as an UTF-16 surrogate pair, // does that affect decoding? For now let's assume it does not. _inputPtr = inPtr; - /*c = */ _decodeEscaped(); + char decoded = _decodeEscaped(); + // [jackson-core#1683]: Validate JSON-escaped surrogates even when + // the string content is being skipped, so callers that stream + // over content with `skipChildren()` still see malformed input. + if (decoded >= 0xD800 && decoded <= 0xDFFF) { + if (decoded < 0xDC00) { // high surrogate: must be followed by low surrogate escape + char hi = decoded; + if (_inputPtr >= _inputEnd) { + if (!_loadMore()) { + _reportInvalidEOF( + ": was expecting closing quote for a string value", + JsonToken.VALUE_STRING); + } + } + if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "string value"); + } + ++_inputPtr; + char lo = _decodeEscaped(); + if (lo < 0xDC00 || lo > 0xDFFF) { + _reportBrokenSurrogatePair(lo, "string value"); + } + } else { // lone low surrogate + _reportUnexpectedLowSurrogate(decoded, "string value"); + } + } + inBuf = _inputBuffer; inPtr = _inputPtr; inLen = _inputEnd; } else if (i <= INT_QUOTE) { @@ -3041,6 +3170,24 @@ protected void _reportInvalidToken(String matchedPart, String msg) throws IOExce throw _constructReadException(fullMsg, loc); } + // [jackson-core#1683]: helpers for reporting malformed JSON-escaped surrogate + // sequences. Wording mirrors the messages introduced for UTF8StreamJsonParser + // by the [jackson-core#1541] fix, extended to name the offending context. + private void _reportUnexpectedLowSurrogate(int lo, String ctx) throws IOException { + _reportError("Unexpected low surrogate in " + ctx + ": 0x" + Integer.toHexString(lo)); + } + + private void _reportUnexpectedCharAfterHighSurrogate(int next, String ctx) + throws IOException { + _reportError("Broken surrogate pair in " + ctx + + ": expected '\\' to start low surrogate, got 0x" + Integer.toHexString(next)); + } + + private void _reportBrokenSurrogatePair(int lo, String ctx) throws IOException { + _reportError(String.format( + "Broken surrogate pair in %s: expected low surrogate, got 0x%04X", ctx, lo)); + } + /* /********************************************************** /* Internal methods, other diff --git a/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java b/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java new file mode 100644 index 0000000000..f508a7a8b7 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java @@ -0,0 +1,411 @@ +package com.fasterxml.jackson.core.read; + +import java.io.StringReader; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.core.JUnit5TestBase; +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.core.json.JsonReadFeature; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Tests for [jackson-core#1683]: JSON-escaped lone surrogates in string values + * (and field names) must be rejected by the Reader-based parser, matching the + * fix applied for the UTF-8 stream parser in [jackson-core#1541]. + * + *

Only exercises {@link com.fasterxml.jackson.core.json.ReaderBasedJsonParser} + * paths; UTF-8 / async parsers are covered separately. + */ +class EscapedSurrogateInStringValue1683Test extends JUnit5TestBase +{ + private final JsonFactory FACTORY = new JsonFactory(); + + // JSON documents. Each backslash-u escape is written using an explicit + // '\\' + 'u' + hex prefix so the Java pre-lexer doesn't fold it into a + // single UTF-16 code unit before it reaches the parser. + private static final String LONE_LEADING_VALUE = "{\"k\":\"\\uD800\"}"; + private static final String LONE_TRAILING_VALUE = "{\"k\":\"\\uDC00\"}"; + private static final String REVERSED_PAIR_VALUE = "{\"k\":\"\\uDC00\\uD800\"}"; + private static final String VALID_PAIR_VALUE = "{\"k\":\"\\uD800\\uDC00\"}"; + private static final String LONE_TRAILING_NAME = "{\"\\uDC00\":1}"; + private static final String LONE_LEADING_NAME = "{\"\\uD800\":1}"; + private static final String REVERSED_PAIR_NAME = "{\"\\uDC00\\uD800\":1}"; + + // ---- string-value coverage -------------------------------------------- + + @Test + void loneLeadingSurrogateInStringValue_stringInput() throws Exception { + assertRejects(LONE_LEADING_VALUE, /*fromString=*/true); + } + + @Test + void loneLeadingSurrogateInStringValue_readerInput() throws Exception { + assertRejects(LONE_LEADING_VALUE, /*fromString=*/false); + } + + @Test + void loneTrailingSurrogateInStringValue_stringInput() throws Exception { + assertRejects(LONE_TRAILING_VALUE, /*fromString=*/true); + } + + @Test + void loneTrailingSurrogateInStringValue_readerInput() throws Exception { + assertRejects(LONE_TRAILING_VALUE, /*fromString=*/false); + } + + @Test + void reversedSurrogatePairInStringValue_stringInput() throws Exception { + assertRejects(REVERSED_PAIR_VALUE, /*fromString=*/true); + } + + @Test + void reversedSurrogatePairInStringValue_readerInput() throws Exception { + assertRejects(REVERSED_PAIR_VALUE, /*fromString=*/false); + } + + @Test + void validSurrogatePairInStringValue_stringInput() throws Exception { + assertAcceptsValidPair(VALID_PAIR_VALUE, /*fromString=*/true); + } + + @Test + void validSurrogatePairInStringValue_readerInput() throws Exception { + assertAcceptsValidPair(VALID_PAIR_VALUE, /*fromString=*/false); + } + + // ---- field-name coverage ---------------------------------------------- + + @Test + void loneTrailingSurrogateInFieldName_stringInput() throws Exception { + assertRejects(LONE_TRAILING_NAME, /*fromString=*/true); + } + + @Test + void loneTrailingSurrogateInFieldName_readerInput() throws Exception { + assertRejects(LONE_TRAILING_NAME, /*fromString=*/false); + } + + @Test + void loneLeadingSurrogateInFieldName_stringInput() throws Exception { + assertRejects(LONE_LEADING_NAME, /*fromString=*/true); + } + + @Test + void loneLeadingSurrogateInFieldName_readerInput() throws Exception { + assertRejects(LONE_LEADING_NAME, /*fromString=*/false); + } + + @Test + void reversedSurrogatePairInFieldName_stringInput() throws Exception { + assertRejects(REVERSED_PAIR_NAME, /*fromString=*/true); + } + + @Test + void reversedSurrogatePairInFieldName_readerInput() throws Exception { + assertRejects(REVERSED_PAIR_NAME, /*fromString=*/false); + } + + // ---- single-quoted string coverage (exercises _handleApos) ------------ + + private static final String APOS_LONE_LEADING = "{'k':'\\uD800'}"; + private static final String APOS_LONE_TRAILING = "{'k':'\\uDC00'}"; + private static final String APOS_REVERSED_PAIR = "{'k':'\\uDC00\\uD800'}"; + private static final String APOS_VALID_PAIR = "{'k':'\\uD800\\uDC00'}"; + + private JsonFactory factoryWithSingleQuotes() { + return JsonFactory.builder() + .enable(JsonReadFeature.ALLOW_SINGLE_QUOTES) + .build(); + } + + @Test + void singleQuotedStringWithLoneLeadingSurrogate_stringInput() throws Exception { + assertRejects(factoryWithSingleQuotes(), APOS_LONE_LEADING, /*fromString=*/true); + } + + @Test + void singleQuotedStringWithLoneLeadingSurrogate_readerInput() throws Exception { + assertRejects(factoryWithSingleQuotes(), APOS_LONE_LEADING, /*fromString=*/false); + } + + @Test + void singleQuotedStringWithLoneTrailingSurrogate_stringInput() throws Exception { + assertRejects(factoryWithSingleQuotes(), APOS_LONE_TRAILING, /*fromString=*/true); + } + + @Test + void singleQuotedStringWithLoneTrailingSurrogate_readerInput() throws Exception { + assertRejects(factoryWithSingleQuotes(), APOS_LONE_TRAILING, /*fromString=*/false); + } + + @Test + void singleQuotedStringWithReversedSurrogatePair_stringInput() throws Exception { + assertRejects(factoryWithSingleQuotes(), APOS_REVERSED_PAIR, /*fromString=*/true); + } + + @Test + void singleQuotedStringWithReversedSurrogatePair_readerInput() throws Exception { + assertRejects(factoryWithSingleQuotes(), APOS_REVERSED_PAIR, /*fromString=*/false); + } + + @Test + void singleQuotedStringWithValidSurrogatePair_stringInput() throws Exception { + assertAcceptsValidPair(factoryWithSingleQuotes(), APOS_VALID_PAIR, /*fromString=*/true); + } + + @Test + void singleQuotedStringWithValidSurrogatePair_readerInput() throws Exception { + assertAcceptsValidPair(factoryWithSingleQuotes(), APOS_VALID_PAIR, /*fromString=*/false); + } + + // ---- skipChildren coverage (exercises _skipString) -------------------- + + @Test + void skipChildrenPastMalformedString_fromString() throws Exception { + String doc = "{\"outer\":{\"k\":\"\\uDC00\"}}"; + try (JsonParser p = FACTORY.createParser(doc)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + try { + p.skipChildren(); + fail("expected JsonParseException"); + } catch (JsonParseException e) { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue(msg.contains("surrogate"), + "expected surrogate error, got: " + e.getMessage()); + } + } + } + + @Test + void skipChildrenPastMalformedString_fromReader() throws Exception { + String doc = "{\"outer\":{\"k\":\"\\uDC00\"}}"; + try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + try { + p.skipChildren(); + fail("expected JsonParseException"); + } catch (JsonParseException e) { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue(msg.contains("surrogate"), + "expected surrogate error, got: " + e.getMessage()); + } + } + } + + @Test + void skipChildrenPastMalformedString_highSurrogateFollowedByNonEscape() throws Exception { + // High surrogate followed by a plain character rather than another escape + String doc = "{\"outer\":{\"k\":\"\\uD800x\"}}"; + try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + try { + p.skipChildren(); + fail("expected JsonParseException"); + } catch (JsonParseException e) { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue(msg.contains("surrogate"), + "expected surrogate error, got: " + e.getMessage()); + } + } + } + + // ---- segment-boundary coverage (exercises _parseName2 bounds check) --- + + /** + * Regression test for the buffer overflow that would occur if the high + * surrogate write in {@code _parseName2} landed on the last slot of the + * current text-buffer segment. Without the bounds check between the hi + * and lo writes, the fall-through path would then write the low + * surrogate at index {@code outBuf.length}, throwing + * {@link ArrayIndexOutOfBoundsException} instead of returning a valid + * astral field name. + * + *

The default first-segment size in {@code TextBuffer} is 200 UTF-16 + * code units; we sweep several padding sizes so at least one lands the + * high-surrogate write on {@code outBuf.length - 1}. + */ + @Test + void longFieldNameWithSurrogatePairAtSegmentBoundary_readerInput() throws Exception { + // Sweep sizes so that at least one lands the hi write on the segment boundary + int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; + for (int pad : padSizes) { + StringBuilder sb = new StringBuilder("{\""); + for (int i = 0; i < pad; i++) sb.append('a'); + sb.append("\\uD83D\\uDE00\":1}"); + String doc = sb.toString(); + try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + String name = p.currentName(); + int expectedCodePoints = pad + 1; + assertEquals(expectedCodePoints, name.codePointCount(0, name.length()), + "codepoint count for pad=" + pad); + assertEquals(0x1F600, name.codePointAt(pad), + "astral cp at index " + pad + " for pad=" + pad); + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } + } + } + + @Test + void longFieldNameWithLoneTrailingSurrogateAtSegmentBoundary_readerInput() throws Exception { + // Same padding sweep, but lone-trailing-surrogate escape — must still + // reject cleanly at the boundary rather than overflowing. + int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; + for (int pad : padSizes) { + StringBuilder sb = new StringBuilder("{\""); + for (int i = 0; i < pad; i++) sb.append('a'); + sb.append("\\uDC00\":1}"); + String doc = sb.toString(); + try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { + try { + while (p.nextToken() != null) { + if (p.currentToken() == JsonToken.FIELD_NAME) { + p.getText(); + } + } + fail("expected JsonParseException for lone trailing surrogate at pad=" + pad); + } catch (JsonParseException e) { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue(msg.contains("surrogate"), + "expected surrogate error at pad=" + pad + ", got: " + e.getMessage()); + } + } + } + } + + // ---- mixed escape / literal-surrogate coverage (locks §5.6 behavior) -- + + /** + * The Reader path can accept a Java {@code String} input that contains a + * literal (unescaped) surrogate {@code char}. Verify the deliberate + * strict-rejection behavior when an escape surrogate is followed by, or + * preceded by, a literal surrogate char: both cases MUST reject with + * {@code JsonParseException}, even though a lenient reader might treat + * the escape+literal combination as a valid UTF-16 pair. + */ + @Test + void escapedHighFollowedByLiteralLowSurrogate_isRejected() throws Exception { + String doc = "{\"k\":\"\\uD800" + '\uDC00' + "\"}"; + for (boolean fromString : new boolean[]{true, false}) { + try (JsonParser p = open(doc, fromString)) { + try { + while (p.nextToken() != null) { p.getText(); } + fail("expected JsonParseException (fromString=" + fromString + ")"); + } catch (JsonParseException e) { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue(msg.contains("surrogate"), + "expected surrogate error, got: " + e.getMessage()); + } + } + } + } + + @Test + void literalHighFollowedByEscapedLowSurrogate_isRejected() throws Exception { + String doc = "{\"k\":\"" + '\uD800' + "\\uDC00\"}"; + for (boolean fromString : new boolean[]{true, false}) { + try (JsonParser p = open(doc, fromString)) { + try { + while (p.nextToken() != null) { p.getText(); } + fail("expected JsonParseException (fromString=" + fromString + ")"); + } catch (JsonParseException e) { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue(msg.contains("surrogate"), + "expected surrogate error, got: " + e.getMessage()); + } + } + } + } + + @Test + void bothLiteralSurrogatesFormingValidPair_isAccepted() throws Exception { + // Literal char pair — parser does not decode escapes, so no surrogate + // guard fires. This exercises the pre-existing accept path for + // literal chars in the input stream and locks that we do not + // regress it while validating the escape paths. + String doc = "{\"k\":\"" + '\uD800' + '\uDC00' + "\"}"; + for (boolean fromString : new boolean[]{true, false}) { + try (JsonParser p = open(doc, fromString)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + String v = p.getText(); + assertEquals(0x10000, v.codePointAt(0), + "expected U+10000 from literal surrogate pair (fromString=" + fromString + ")"); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } + } + } + + // ---- helpers ---------------------------------------------------------- + + private JsonParser open(String doc, boolean fromString) throws Exception { + return open(FACTORY, doc, fromString); + } + + private JsonParser open(JsonFactory factory, String doc, boolean fromString) throws Exception { + return fromString + ? factory.createParser(doc) + : factory.createParser(new StringReader(doc)); + } + + private void assertRejects(String doc, boolean fromString) throws Exception { + assertRejects(FACTORY, doc, fromString); + } + + private void assertRejects(JsonFactory factory, String doc, boolean fromString) throws Exception { + try (JsonParser p = open(factory, doc, fromString)) { + try { + // Drive tokens until either the parser throws or the doc ends. + while (p.nextToken() != null) { + if (p.currentToken() == JsonToken.VALUE_STRING + || p.currentToken() == JsonToken.FIELD_NAME) { + // Force decode; some paths defer surrogate work until + // the caller pulls the text. + p.getText(); + } + } + fail("Expected JsonParseException for malformed surrogate escape in: " + doc); + } catch (JsonParseException e) { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue(msg.contains("surrogate"), + "Expected surrogate error message, got: " + e.getMessage()); + } + } + } + + private void assertAcceptsValidPair(String doc, boolean fromString) throws Exception { + assertAcceptsValidPair(FACTORY, doc, fromString); + } + + private void assertAcceptsValidPair(JsonFactory factory, String doc, boolean fromString) throws Exception { + try (JsonParser p = open(factory, doc, fromString)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("k", p.currentName()); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + String text = p.getText(); + // U+10000 encodes as the surrogate pair D800 DC00 in Java strings. + assertEquals(2, text.length(), "expected two UTF-16 code units"); + int cp = text.codePointAt(0); + assertEquals(0x10000, cp, + "expected code point U+10000, got U+" + Integer.toHexString(cp)); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } + } +} From 17be547a531d5b15644174e758b493df78797450 Mon Sep 17 00:00:00 2001 From: Elankumaran Srinivasan <5340827+elang2@users.noreply.github.com> Date: Sun, 6 Sep 2026 18:47:44 -0700 Subject: [PATCH 2/9] Trigger CI re-run (full local verify passes with CI JAVA_OPTS) From 596a0c890b1074371677d924051912210c93ed8c Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 1 Oct 2026 20:19:12 -0700 Subject: [PATCH 3/9] Streamlining --- .../jackson/core/json/ReaderBasedJsonParser.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java index 7085c8066d..244126f382 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java @@ -1878,7 +1878,7 @@ private String _parseName2(int startPtr, int hash, int endChar) throws IOExcepti } } if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { - _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "field name"); + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr], "field name"); } ++_inputPtr; char lo = _decodeEscaped(); @@ -2139,7 +2139,7 @@ protected JsonToken _handleApos() throws IOException } } if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { - _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "string value"); + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr], "string value"); } ++_inputPtr; char lo = _decodeEscaped(); @@ -2298,7 +2298,7 @@ protected void _finishString2() throws IOException } } if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { - _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "string value"); + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr], "string value"); } ++_inputPtr; char lo = _decodeEscaped(); @@ -2371,7 +2371,6 @@ protected final void _skipString() throws IOException // over content with `skipChildren()` still see malformed input. if (decoded >= 0xD800 && decoded <= 0xDFFF) { if (decoded < 0xDC00) { // high surrogate: must be followed by low surrogate escape - char hi = decoded; if (_inputPtr >= _inputEnd) { if (!_loadMore()) { _reportInvalidEOF( @@ -2380,7 +2379,7 @@ protected final void _skipString() throws IOException } } if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { - _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr] & 0xFFFF, "string value"); + _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr], "string value"); } ++_inputPtr; char lo = _decodeEscaped(); From ee8d5d61c1330c018bb516da1fac29524dfdc672 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 1 Oct 2026 20:20:02 -0700 Subject: [PATCH 4/9] Remove dead code block --- .../jackson/core/json/ReaderBasedJsonParser.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java index 244126f382..90552cadf2 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java @@ -1888,13 +1888,7 @@ private String _parseName2(int startPtr, int hash, int endChar) throws IOExcepti // Store as two UTF-16 code units. Hash includes the low // surrogate below; add high surrogate here. hash = (hash * CharsToNameCanonicalizer.HASH_MULT) + hi; - if (outPtr >= outBuf.length) { - totalLen += outBuf.length; - _streamReadConstraints.validateNameLength(totalLen); - outBuf = _textBuffer.finishCurrentSegment(); - outPtr = 0; - } - outBuf[outPtr++] = hi; + // Room for one char is guaranteed at loop start. if (outPtr >= outBuf.length) { totalLen += outBuf.length; _streamReadConstraints.validateNameLength(totalLen); From aa746a14935628468ab8b4d6aa9cef78d4510d9b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 1 Oct 2026 20:25:31 -0700 Subject: [PATCH 5/9] Add back accidentally removed line --- .../com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java index 90552cadf2..0764380067 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java @@ -1889,6 +1889,7 @@ private String _parseName2(int startPtr, int hash, int endChar) throws IOExcepti // surrogate below; add high surrogate here. hash = (hash * CharsToNameCanonicalizer.HASH_MULT) + hi; // Room for one char is guaranteed at loop start. + outBuf[outPtr++] = hi; if (outPtr >= outBuf.length) { totalLen += outBuf.length; _streamReadConstraints.validateNameLength(totalLen); From 7342f59451d63b46eb1318e09e36a506684e84f7 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 1 Oct 2026 20:27:36 -0700 Subject: [PATCH 6/9] Test refactoring --- ...EscapedSurrogateInStringValue1683Test.java | 434 ++++++++---------- 1 file changed, 196 insertions(+), 238 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java b/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java index f508a7a8b7..4f055eb297 100644 --- a/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java +++ b/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java @@ -1,7 +1,5 @@ package com.fasterxml.jackson.core.read; -import java.io.StringReader; - import org.junit.jupiter.api.Test; import com.fasterxml.jackson.core.JUnit5TestBase; @@ -21,12 +19,35 @@ * fix applied for the UTF-8 stream parser in [jackson-core#1541]. * *

Only exercises {@link com.fasterxml.jackson.core.json.ReaderBasedJsonParser} - * paths; UTF-8 / async parsers are covered separately. + * paths; UTF-8 / async parsers are covered separately. Every test runs over + * {@link #MODES}: {@code String} input (whole content in one buffer), plain + * {@code Reader} and 1-char-at-a-time throttled {@code Reader}; the latter + * forces every escape to span input-buffer boundaries. */ class EscapedSurrogateInStringValue1683Test extends JUnit5TestBase { + // Local pseudo-mode for `JsonFactory.createParser(String)`, in addition + // to Reader-backed ALL_TEXT_MODES + private final static int MODE_STRING = -1; + + private final static int[] MODES = new int[] { + MODE_STRING, MODE_READER, MODE_READER_THROTTLED + }; + + // Size of char input buffer `ReaderBasedJsonParser` reads into + // (BufferRecycler.CHAR_TOKEN_BUFFER) + private final static int INPUT_BUFFER_LEN = 4000; + + // 6-char escape for high surrogate of U+1F600 + private final static String HI_ESCAPE = "\\uD83D"; + private final static String LO_ESCAPE = "\\uDE00"; + private final JsonFactory FACTORY = new JsonFactory(); + private final JsonFactory APOS_FACTORY = JsonFactory.builder() + .enable(JsonReadFeature.ALLOW_SINGLE_QUOTES) + .build(); + // JSON documents. Each backslash-u escape is written using an explicit // '\\' + 'u' + hex prefix so the Java pre-lexer doesn't fold it into a // single UTF-16 code unit before it reaches the parser. @@ -38,191 +59,86 @@ class EscapedSurrogateInStringValue1683Test extends JUnit5TestBase private static final String LONE_LEADING_NAME = "{\"\\uD800\":1}"; private static final String REVERSED_PAIR_NAME = "{\"\\uDC00\\uD800\":1}"; - // ---- string-value coverage -------------------------------------------- - - @Test - void loneLeadingSurrogateInStringValue_stringInput() throws Exception { - assertRejects(LONE_LEADING_VALUE, /*fromString=*/true); - } - - @Test - void loneLeadingSurrogateInStringValue_readerInput() throws Exception { - assertRejects(LONE_LEADING_VALUE, /*fromString=*/false); - } - - @Test - void loneTrailingSurrogateInStringValue_stringInput() throws Exception { - assertRejects(LONE_TRAILING_VALUE, /*fromString=*/true); - } + private static final String APOS_LONE_LEADING = "{'k':'\\uD800'}"; + private static final String APOS_LONE_TRAILING = "{'k':'\\uDC00'}"; + private static final String APOS_REVERSED_PAIR = "{'k':'\\uDC00\\uD800'}"; + private static final String APOS_VALID_PAIR = "{'k':'\\uD800\\uDC00'}"; - @Test - void loneTrailingSurrogateInStringValue_readerInput() throws Exception { - assertRejects(LONE_TRAILING_VALUE, /*fromString=*/false); - } + // ---- string-value coverage -------------------------------------------- @Test - void reversedSurrogatePairInStringValue_stringInput() throws Exception { - assertRejects(REVERSED_PAIR_VALUE, /*fromString=*/true); + void loneLeadingSurrogateInStringValue() throws Exception { + assertRejects(FACTORY, LONE_LEADING_VALUE); } @Test - void reversedSurrogatePairInStringValue_readerInput() throws Exception { - assertRejects(REVERSED_PAIR_VALUE, /*fromString=*/false); + void loneTrailingSurrogateInStringValue() throws Exception { + assertRejects(FACTORY, LONE_TRAILING_VALUE); } @Test - void validSurrogatePairInStringValue_stringInput() throws Exception { - assertAcceptsValidPair(VALID_PAIR_VALUE, /*fromString=*/true); + void reversedSurrogatePairInStringValue() throws Exception { + assertRejects(FACTORY, REVERSED_PAIR_VALUE); } @Test - void validSurrogatePairInStringValue_readerInput() throws Exception { - assertAcceptsValidPair(VALID_PAIR_VALUE, /*fromString=*/false); + void validSurrogatePairInStringValue() throws Exception { + assertAcceptsValidPair(FACTORY, VALID_PAIR_VALUE); } // ---- field-name coverage ---------------------------------------------- @Test - void loneTrailingSurrogateInFieldName_stringInput() throws Exception { - assertRejects(LONE_TRAILING_NAME, /*fromString=*/true); + void loneTrailingSurrogateInFieldName() throws Exception { + assertRejects(FACTORY, LONE_TRAILING_NAME); } @Test - void loneTrailingSurrogateInFieldName_readerInput() throws Exception { - assertRejects(LONE_TRAILING_NAME, /*fromString=*/false); + void loneLeadingSurrogateInFieldName() throws Exception { + assertRejects(FACTORY, LONE_LEADING_NAME); } @Test - void loneLeadingSurrogateInFieldName_stringInput() throws Exception { - assertRejects(LONE_LEADING_NAME, /*fromString=*/true); - } - - @Test - void loneLeadingSurrogateInFieldName_readerInput() throws Exception { - assertRejects(LONE_LEADING_NAME, /*fromString=*/false); - } - - @Test - void reversedSurrogatePairInFieldName_stringInput() throws Exception { - assertRejects(REVERSED_PAIR_NAME, /*fromString=*/true); - } - - @Test - void reversedSurrogatePairInFieldName_readerInput() throws Exception { - assertRejects(REVERSED_PAIR_NAME, /*fromString=*/false); + void reversedSurrogatePairInFieldName() throws Exception { + assertRejects(FACTORY, REVERSED_PAIR_NAME); } // ---- single-quoted string coverage (exercises _handleApos) ------------ - private static final String APOS_LONE_LEADING = "{'k':'\\uD800'}"; - private static final String APOS_LONE_TRAILING = "{'k':'\\uDC00'}"; - private static final String APOS_REVERSED_PAIR = "{'k':'\\uDC00\\uD800'}"; - private static final String APOS_VALID_PAIR = "{'k':'\\uD800\\uDC00'}"; - - private JsonFactory factoryWithSingleQuotes() { - return JsonFactory.builder() - .enable(JsonReadFeature.ALLOW_SINGLE_QUOTES) - .build(); - } - - @Test - void singleQuotedStringWithLoneLeadingSurrogate_stringInput() throws Exception { - assertRejects(factoryWithSingleQuotes(), APOS_LONE_LEADING, /*fromString=*/true); - } - - @Test - void singleQuotedStringWithLoneLeadingSurrogate_readerInput() throws Exception { - assertRejects(factoryWithSingleQuotes(), APOS_LONE_LEADING, /*fromString=*/false); - } - - @Test - void singleQuotedStringWithLoneTrailingSurrogate_stringInput() throws Exception { - assertRejects(factoryWithSingleQuotes(), APOS_LONE_TRAILING, /*fromString=*/true); - } - - @Test - void singleQuotedStringWithLoneTrailingSurrogate_readerInput() throws Exception { - assertRejects(factoryWithSingleQuotes(), APOS_LONE_TRAILING, /*fromString=*/false); - } - @Test - void singleQuotedStringWithReversedSurrogatePair_stringInput() throws Exception { - assertRejects(factoryWithSingleQuotes(), APOS_REVERSED_PAIR, /*fromString=*/true); + void singleQuotedStringWithLoneLeadingSurrogate() throws Exception { + assertRejects(APOS_FACTORY, APOS_LONE_LEADING); } @Test - void singleQuotedStringWithReversedSurrogatePair_readerInput() throws Exception { - assertRejects(factoryWithSingleQuotes(), APOS_REVERSED_PAIR, /*fromString=*/false); + void singleQuotedStringWithLoneTrailingSurrogate() throws Exception { + assertRejects(APOS_FACTORY, APOS_LONE_TRAILING); } @Test - void singleQuotedStringWithValidSurrogatePair_stringInput() throws Exception { - assertAcceptsValidPair(factoryWithSingleQuotes(), APOS_VALID_PAIR, /*fromString=*/true); + void singleQuotedStringWithReversedSurrogatePair() throws Exception { + assertRejects(APOS_FACTORY, APOS_REVERSED_PAIR); } @Test - void singleQuotedStringWithValidSurrogatePair_readerInput() throws Exception { - assertAcceptsValidPair(factoryWithSingleQuotes(), APOS_VALID_PAIR, /*fromString=*/false); + void singleQuotedStringWithValidSurrogatePair() throws Exception { + assertAcceptsValidPair(APOS_FACTORY, APOS_VALID_PAIR); } // ---- skipChildren coverage (exercises _skipString) -------------------- @Test - void skipChildrenPastMalformedString_fromString() throws Exception { - String doc = "{\"outer\":{\"k\":\"\\uDC00\"}}"; - try (JsonParser p = FACTORY.createParser(doc)) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - assertToken(JsonToken.START_OBJECT, p.nextToken()); - try { - p.skipChildren(); - fail("expected JsonParseException"); - } catch (JsonParseException e) { - String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); - assertTrue(msg.contains("surrogate"), - "expected surrogate error, got: " + e.getMessage()); - } - } + void skipChildrenPastLoneTrailingSurrogate() throws Exception { + assertSkipChildrenRejects("{\"outer\":{\"k\":\"\\uDC00\"}}"); } @Test - void skipChildrenPastMalformedString_fromReader() throws Exception { - String doc = "{\"outer\":{\"k\":\"\\uDC00\"}}"; - try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - assertToken(JsonToken.START_OBJECT, p.nextToken()); - try { - p.skipChildren(); - fail("expected JsonParseException"); - } catch (JsonParseException e) { - String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); - assertTrue(msg.contains("surrogate"), - "expected surrogate error, got: " + e.getMessage()); - } - } - } - - @Test - void skipChildrenPastMalformedString_highSurrogateFollowedByNonEscape() throws Exception { + void skipChildrenPastHighSurrogateFollowedByNonEscape() throws Exception { // High surrogate followed by a plain character rather than another escape - String doc = "{\"outer\":{\"k\":\"\\uD800x\"}}"; - try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - assertToken(JsonToken.START_OBJECT, p.nextToken()); - try { - p.skipChildren(); - fail("expected JsonParseException"); - } catch (JsonParseException e) { - String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); - assertTrue(msg.contains("surrogate"), - "expected surrogate error, got: " + e.getMessage()); - } - } + assertSkipChildrenRejects("{\"outer\":{\"k\":\"\\uD800x\"}}"); } - // ---- segment-boundary coverage (exercises _parseName2 bounds check) --- + // ---- text-buffer segment-boundary coverage (exercises _parseName2) ---- /** * Regression test for the buffer overflow that would occur if the high @@ -238,54 +154,111 @@ void skipChildrenPastMalformedString_highSurrogateFollowedByNonEscape() throws E * high-surrogate write on {@code outBuf.length - 1}. */ @Test - void longFieldNameWithSurrogatePairAtSegmentBoundary_readerInput() throws Exception { - // Sweep sizes so that at least one lands the hi write on the segment boundary + void longFieldNameWithSurrogatePairAtSegmentBoundary() throws Exception { int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; - for (int pad : padSizes) { - StringBuilder sb = new StringBuilder("{\""); - for (int i = 0; i < pad; i++) sb.append('a'); - sb.append("\\uD83D\\uDE00\":1}"); - String doc = sb.toString(); - try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - String name = p.currentName(); - int expectedCodePoints = pad + 1; - assertEquals(expectedCodePoints, name.codePointCount(0, name.length()), - "codepoint count for pad=" + pad); - assertEquals(0x1F600, name.codePointAt(pad), - "astral cp at index " + pad + " for pad=" + pad); - assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); - assertToken(JsonToken.END_OBJECT, p.nextToken()); + for (int mode : MODES) { + for (int pad : padSizes) { + String doc = "{\"" + pad(pad) + HI_ESCAPE + LO_ESCAPE + "\":1}"; + try (JsonParser p = open(FACTORY, mode, doc)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + String name = p.currentName(); + assertEquals(pad + 1, name.codePointCount(0, name.length()), + "codepoint count for pad=" + pad + ", mode=" + mode); + assertEquals(0x1F600, name.codePointAt(pad), + "astral cp at index " + pad + " for pad=" + pad + ", mode=" + mode); + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } } } } @Test - void longFieldNameWithLoneTrailingSurrogateAtSegmentBoundary_readerInput() throws Exception { + void longFieldNameWithLoneTrailingSurrogateAtSegmentBoundary() throws Exception { // Same padding sweep, but lone-trailing-surrogate escape — must still // reject cleanly at the boundary rather than overflowing. int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; for (int pad : padSizes) { - StringBuilder sb = new StringBuilder("{\""); - for (int i = 0; i < pad; i++) sb.append('a'); - sb.append("\\uDC00\":1}"); - String doc = sb.toString(); - try (JsonParser p = FACTORY.createParser(new StringReader(doc))) { - try { - while (p.nextToken() != null) { - if (p.currentToken() == JsonToken.FIELD_NAME) { - p.getText(); - } - } - fail("expected JsonParseException for lone trailing surrogate at pad=" + pad); - } catch (JsonParseException e) { - String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); - assertTrue(msg.contains("surrogate"), - "expected surrogate error at pad=" + pad + ", got: " + e.getMessage()); + assertRejects(FACTORY, "{\"" + pad(pad) + "\\uDC00\":1}"); + } + } + + // ---- input-buffer boundary coverage ----------------------------------- + // + // High-surrogate escape ending exactly at the end of the first input + // buffer, so the following low-surrogate escape (or other char, or EOF) + // is only seen after `_loadMore()`. Applies to Reader-backed input only: + // String input is never refilled. + + @Test + void highSurrogateAtInputBufferEnd_fieldName() throws Exception { + _testHighSurrogateAtInputBufferEnd(FACTORY, "{\"", "\":1}", false); + } + + @Test + void highSurrogateAtInputBufferEnd_stringValue() throws Exception { + _testHighSurrogateAtInputBufferEnd(FACTORY, "[\"", "\"]", false); + } + + @Test + void highSurrogateAtInputBufferEnd_aposStringValue() throws Exception { + _testHighSurrogateAtInputBufferEnd(APOS_FACTORY, "['", "']", false); + } + + @Test + void highSurrogateAtInputBufferEnd_skippedStringValue() throws Exception { + _testHighSurrogateAtInputBufferEnd(FACTORY, "[\"", "\"]", true); + } + + private void _testHighSurrogateAtInputBufferEnd(JsonFactory f, + String prefix, String suffix, boolean skip) throws Exception + { + final String lead = prefix + pad(INPUT_BUFFER_LEN - prefix.length() - HI_ESCAPE.length()) + + HI_ESCAPE; + assertEquals(INPUT_BUFFER_LEN, lead.length()); + + for (int mode : ALL_TEXT_MODES) { + // Valid pair split across buffers: accepted + try (JsonParser p = open(f, mode, lead + LO_ESCAPE + suffix)) { + String text = _readFirstString(p, skip); + if (!skip) { + assertEquals(0x1F600, text.codePointAt(text.length() - 2), "mode=" + mode); } } + // High surrogate followed by non-escape char in next buffer + try (JsonParser p = open(f, mode, lead + "x" + suffix)) { + _readFirstString(p, skip); + fail("Expected JsonParseException (mode=" + mode + ")"); + } catch (JsonParseException e) { + verifyException(e, "surrogate"); + } + // High surrogate as the very last content + try (JsonParser p = open(f, mode, lead)) { + _readFirstString(p, skip); + fail("Expected JsonParseException (mode=" + mode + ")"); + } catch (JsonParseException e) { + verifyException(e, "end-of-input"); + } + } + } + + // Returns first field name or String value; or, if `skip`, skips value + // (without accessing text) and returns null + private String _readFirstString(JsonParser p, boolean skip) throws Exception + { + JsonToken t = p.nextToken(); + if (t == JsonToken.START_OBJECT) { + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + return p.currentName(); } + assertToken(JsonToken.START_ARRAY, t); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + if (skip) { + assertToken(JsonToken.END_ARRAY, p.nextToken()); + return null; + } + return p.getText(); } // ---- mixed escape / literal-surrogate coverage (locks §5.6 behavior) -- @@ -300,36 +273,12 @@ void longFieldNameWithLoneTrailingSurrogateAtSegmentBoundary_readerInput() throw */ @Test void escapedHighFollowedByLiteralLowSurrogate_isRejected() throws Exception { - String doc = "{\"k\":\"\\uD800" + '\uDC00' + "\"}"; - for (boolean fromString : new boolean[]{true, false}) { - try (JsonParser p = open(doc, fromString)) { - try { - while (p.nextToken() != null) { p.getText(); } - fail("expected JsonParseException (fromString=" + fromString + ")"); - } catch (JsonParseException e) { - String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); - assertTrue(msg.contains("surrogate"), - "expected surrogate error, got: " + e.getMessage()); - } - } - } + assertRejects(FACTORY, "{\"k\":\"\\uD800" + '\uDC00' + "\"}"); } @Test void literalHighFollowedByEscapedLowSurrogate_isRejected() throws Exception { - String doc = "{\"k\":\"" + '\uD800' + "\\uDC00\"}"; - for (boolean fromString : new boolean[]{true, false}) { - try (JsonParser p = open(doc, fromString)) { - try { - while (p.nextToken() != null) { p.getText(); } - fail("expected JsonParseException (fromString=" + fromString + ")"); - } catch (JsonParseException e) { - String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); - assertTrue(msg.contains("surrogate"), - "expected surrogate error, got: " + e.getMessage()); - } - } - } + assertRejects(FACTORY, "{\"k\":\"" + '\uD800' + "\\uDC00\"}"); } @Test @@ -339,14 +288,13 @@ void bothLiteralSurrogatesFormingValidPair_isAccepted() throws Exception { // literal chars in the input stream and locks that we do not // regress it while validating the escape paths. String doc = "{\"k\":\"" + '\uD800' + '\uDC00' + "\"}"; - for (boolean fromString : new boolean[]{true, false}) { - try (JsonParser p = open(doc, fromString)) { + for (int mode : MODES) { + try (JsonParser p = open(FACTORY, mode, doc)) { assertToken(JsonToken.START_OBJECT, p.nextToken()); assertToken(JsonToken.FIELD_NAME, p.nextToken()); assertToken(JsonToken.VALUE_STRING, p.nextToken()); - String v = p.getText(); - assertEquals(0x10000, v.codePointAt(0), - "expected U+10000 from literal surrogate pair (fromString=" + fromString + ")"); + assertEquals(0x10000, p.getText().codePointAt(0), + "expected U+10000 from literal surrogate pair (mode=" + mode + ")"); assertToken(JsonToken.END_OBJECT, p.nextToken()); } } @@ -354,23 +302,24 @@ void bothLiteralSurrogatesFormingValidPair_isAccepted() throws Exception { // ---- helpers ---------------------------------------------------------- - private JsonParser open(String doc, boolean fromString) throws Exception { - return open(FACTORY, doc, fromString); - } - - private JsonParser open(JsonFactory factory, String doc, boolean fromString) throws Exception { - return fromString - ? factory.createParser(doc) - : factory.createParser(new StringReader(doc)); + private JsonParser open(JsonFactory f, int mode, String doc) throws Exception { + if (mode == MODE_STRING) { + return f.createParser(doc); + } + return createParser(f, mode, doc); } - private void assertRejects(String doc, boolean fromString) throws Exception { - assertRejects(FACTORY, doc, fromString); + private static String pad(int len) { + StringBuilder sb = new StringBuilder(len); + for (int i = 0; i < len; i++) { + sb.append('a'); + } + return sb.toString(); } - private void assertRejects(JsonFactory factory, String doc, boolean fromString) throws Exception { - try (JsonParser p = open(factory, doc, fromString)) { - try { + private void assertRejects(JsonFactory f, String doc) throws Exception { + for (int mode : MODES) { + try (JsonParser p = open(f, mode, doc)) { // Drive tokens until either the parser throws or the doc ends. while (p.nextToken() != null) { if (p.currentToken() == JsonToken.VALUE_STRING @@ -380,32 +329,41 @@ private void assertRejects(JsonFactory factory, String doc, boolean fromString) p.getText(); } } - fail("Expected JsonParseException for malformed surrogate escape in: " + doc); + fail("Expected JsonParseException for malformed surrogate escape (mode=" + + mode + ") in: " + doc); } catch (JsonParseException e) { - String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); - assertTrue(msg.contains("surrogate"), - "Expected surrogate error message, got: " + e.getMessage()); + verifyException(e, "surrogate"); } } } - private void assertAcceptsValidPair(String doc, boolean fromString) throws Exception { - assertAcceptsValidPair(FACTORY, doc, fromString); + private void assertSkipChildrenRejects(String doc) throws Exception { + for (int mode : MODES) { + try (JsonParser p = open(FACTORY, mode, doc)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + p.skipChildren(); + fail("Expected JsonParseException (mode=" + mode + ")"); + } catch (JsonParseException e) { + verifyException(e, "surrogate"); + } + } } - private void assertAcceptsValidPair(JsonFactory factory, String doc, boolean fromString) throws Exception { - try (JsonParser p = open(factory, doc, fromString)) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - assertEquals("k", p.currentName()); - assertToken(JsonToken.VALUE_STRING, p.nextToken()); - String text = p.getText(); - // U+10000 encodes as the surrogate pair D800 DC00 in Java strings. - assertEquals(2, text.length(), "expected two UTF-16 code units"); - int cp = text.codePointAt(0); - assertEquals(0x10000, cp, - "expected code point U+10000, got U+" + Integer.toHexString(cp)); - assertToken(JsonToken.END_OBJECT, p.nextToken()); + private void assertAcceptsValidPair(JsonFactory f, String doc) throws Exception { + for (int mode : MODES) { + try (JsonParser p = open(f, mode, doc)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("k", p.currentName()); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + String text = p.getText(); + // U+10000 encodes as the surrogate pair D800 DC00 in Java strings. + assertEquals(2, text.length(), "expected two UTF-16 code units (mode=" + mode + ")"); + assertEquals(0x10000, text.codePointAt(0), "mode=" + mode); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } } } } From 5ef043a7aeef97f3d57d8bc7fa017a4159ed7f55 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 1 Oct 2026 20:34:23 -0700 Subject: [PATCH 7/9] Remove validation of String values, for compatibility -- not done for any other parser; now matches byte-backed UTF8StreamJsonParser --- release-notes/CREDITS-2.x | 4 +- release-notes/VERSION-2.x | 5 +- .../core/json/ReaderBasedJsonParser.java | 90 +---- .../EscapedSurrogateInFieldName1683Test.java | 251 ++++++++++++ ...EscapedSurrogateInStringValue1683Test.java | 369 ------------------ 5 files changed, 256 insertions(+), 463 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInFieldName1683Test.java delete mode 100644 src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index da99fa2f06..757418ec29 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -559,9 +559,9 @@ Burak KALAYCI (@kalayciburak) * Reported #1713: `UTF32Reader` corrupts or drops the low surrogate when a supplementary character splits across a `read()` boundary (2.21.7) - + Elankumaran Srinivasan (@elang2) * Contributed #1683: `ReaderBasedJsonParser` should reject JSON-escaped lone surrogates in field names and string values (mirror of #1541 fix in `UTF8StreamJsonParser`) - (2.23.0) + (2.23.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 1ec0c115a0..c98344d00a 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -20,9 +20,8 @@ a pure JSON library. based on supplied length (requested by @kilink) (contributed by @seonwooj0810) -#1683: `ReaderBasedJsonParser` should reject JSON-escaped lone - surrogates in field names and string values (mirror of #1541 - fix in `UTF8StreamJsonParser`) +#1683: JSON`\uXXXX` escape accepts lone surrogates in field names for + `ReaderBasedJsonParser` (contributed by @elang2) 2.22.4 (not yet released) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java index 0764380067..b2f18cb027 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java @@ -2121,36 +2121,6 @@ protected JsonToken _handleApos() throws IOException // an UTF-16 surrogate pair, does that affect decoding? // For now let's assume it does not. c = _decodeEscaped(); - // [jackson-core#1683]: Validate JSON-escaped surrogates in - // apostrophe-quoted string value. - if (c >= 0xD800 && c <= 0xDFFF) { - if (c < 0xDC00) { // high surrogate - char hi = c; - if (_inputPtr >= _inputEnd) { - if (!_loadMore()) { - _reportInvalidEOF( - ": was expecting closing quote for a string value", - JsonToken.VALUE_STRING); - } - } - if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { - _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr], "string value"); - } - ++_inputPtr; - char lo = _decodeEscaped(); - if (lo < 0xDC00 || lo > 0xDFFF) { - _reportBrokenSurrogatePair(lo, "string value"); - } - if (outPtr >= outBuf.length) { - outBuf = _textBuffer.finishCurrentSegment(); - outPtr = 0; - } - outBuf[outPtr++] = hi; - c = lo; - } else { // lone low surrogate - _reportUnexpectedLowSurrogate(c, "string value"); - } - } } else if (i <= '\'') { if (i == '\'') { break; @@ -2279,39 +2249,6 @@ protected void _finishString2() throws IOException * For now let's assume it does not. */ c = _decodeEscaped(); - // [jackson-core#1683]: Validate JSON-escaped surrogates in - // string value. Mirror of [jackson-core#1541] fix in - // UTF8StreamJsonParser, applied to the Reader-based path. - if (c >= 0xD800 && c <= 0xDFFF) { - if (c < 0xDC00) { // high surrogate: must be followed by low surrogate escape - char hi = c; - if (_inputPtr >= _inputEnd) { - if (!_loadMore()) { - _reportInvalidEOF( - ": was expecting closing quote for a string value", - JsonToken.VALUE_STRING); - } - } - if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { - _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr], "string value"); - } - ++_inputPtr; - char lo = _decodeEscaped(); - if (lo < 0xDC00 || lo > 0xDFFF) { - _reportBrokenSurrogatePair(lo, "string value"); - } - // Emit high surrogate first, then fall through so the - // low surrogate is appended by the normal path below. - if (outPtr >= outBuf.length) { - outBuf = _textBuffer.finishCurrentSegment(); - outPtr = 0; - } - outBuf[outPtr++] = hi; - c = lo; - } else { // lone low surrogate - _reportUnexpectedLowSurrogate(c, "string value"); - } - } } else if (i < INT_SPACE) { _throwUnquotedSpace(i, "string value"); } // anything else? @@ -2360,32 +2297,7 @@ protected final void _skipString() throws IOException // Although chars outside of BMP are to be escaped as an UTF-16 surrogate pair, // does that affect decoding? For now let's assume it does not. _inputPtr = inPtr; - char decoded = _decodeEscaped(); - // [jackson-core#1683]: Validate JSON-escaped surrogates even when - // the string content is being skipped, so callers that stream - // over content with `skipChildren()` still see malformed input. - if (decoded >= 0xD800 && decoded <= 0xDFFF) { - if (decoded < 0xDC00) { // high surrogate: must be followed by low surrogate escape - if (_inputPtr >= _inputEnd) { - if (!_loadMore()) { - _reportInvalidEOF( - ": was expecting closing quote for a string value", - JsonToken.VALUE_STRING); - } - } - if (_inputBuffer[_inputPtr] != INT_BACKSLASH) { - _reportUnexpectedCharAfterHighSurrogate(_inputBuffer[_inputPtr], "string value"); - } - ++_inputPtr; - char lo = _decodeEscaped(); - if (lo < 0xDC00 || lo > 0xDFFF) { - _reportBrokenSurrogatePair(lo, "string value"); - } - } else { // lone low surrogate - _reportUnexpectedLowSurrogate(decoded, "string value"); - } - } - inBuf = _inputBuffer; + /*c = */ _decodeEscaped(); inPtr = _inputPtr; inLen = _inputEnd; } else if (i <= INT_QUOTE) { diff --git a/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInFieldName1683Test.java b/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInFieldName1683Test.java new file mode 100644 index 0000000000..06e7f2deaa --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInFieldName1683Test.java @@ -0,0 +1,251 @@ +package com.fasterxml.jackson.core.read; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.core.JUnit5TestBase; +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.core.json.JsonReadFeature; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Tests for [jackson-core#1683]: JSON-escaped lone surrogates in field names + * must be rejected by the Reader-based parser, matching the fix applied for + * the UTF-8 stream parser in [jackson-core#1541]. String values are + * (as with UTF-8 parser) not validated, and must pass through as-is. + * + *

Only exercises {@link com.fasterxml.jackson.core.json.ReaderBasedJsonParser} + * paths. Every test runs over {@link #MODES}: {@code String} input (whole + * content in one buffer), plain {@code Reader} and 1-char-at-a-time throttled + * {@code Reader}; the latter forces every escape to span input-buffer boundaries. + */ +class EscapedSurrogateInFieldName1683Test extends JUnit5TestBase +{ + // Local pseudo-mode for `JsonFactory.createParser(String)`, in addition + // to Reader-backed ALL_TEXT_MODES + private final static int MODE_STRING = -1; + + private final static int[] MODES = new int[] { + MODE_STRING, MODE_READER, MODE_READER_THROTTLED + }; + + // Size of char input buffer `ReaderBasedJsonParser` reads into + // (BufferRecycler.CHAR_TOKEN_BUFFER) + private final static int INPUT_BUFFER_LEN = 4000; + + // 6-char escape for high surrogate of U+1F600 + private final static String HI_ESCAPE = "\\uD83D"; + private final static String LO_ESCAPE = "\\uDE00"; + + private final JsonFactory FACTORY = new JsonFactory(); + + private final JsonFactory APOS_FACTORY = JsonFactory.builder() + .enable(JsonReadFeature.ALLOW_SINGLE_QUOTES) + .build(); + + // ---- field-name coverage ---------------------------------------------- + + // Each backslash-u escape is written using an explicit '\\' + 'u' + hex + // prefix so the Java pre-lexer doesn't fold it into a single UTF-16 code + // unit before it reaches the parser. + + @Test + void loneLeadingSurrogateInFieldName() throws Exception { + assertRejects(FACTORY, "{\"\\uD800\":1}"); + } + + @Test + void loneTrailingSurrogateInFieldName() throws Exception { + assertRejects(FACTORY, "{\"\\uDC00\":1}"); + } + + @Test + void reversedSurrogatePairInFieldName() throws Exception { + assertRejects(FACTORY, "{\"\\uDC00\\uD800\":1}"); + } + + @Test + void highSurrogateFollowedByNonEscapeInFieldName() throws Exception { + assertRejects(FACTORY, "{\"\\uD800x\":1}"); + } + + @Test + void validSurrogatePairInFieldName() throws Exception { + for (int mode : MODES) { + try (JsonParser p = open(FACTORY, mode, "{\"\\uD800\\uDC00\":1}")) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("\uD800\uDC00", p.currentName(), "mode=" + mode); + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } + } + } + + @Test + void loneSurrogateInSingleQuotedFieldName() throws Exception { + assertRejects(APOS_FACTORY, "{'\\uDC00':1}"); + } + + // ---- string values: not validated ------------------------------------- + + @Test + void escapedSurrogatesInStringValuePassThrough() throws Exception { + _testStringValuePassThrough(FACTORY, "[\"\\uD800\",\"\\uDC00\",\"\\uDC00\\uD800\",\"\\uD800\\uDC00\"]"); + } + + @Test + void escapedSurrogatesInSingleQuotedStringValuePassThrough() throws Exception { + _testStringValuePassThrough(APOS_FACTORY, "['\\uD800','\\uDC00','\\uDC00\\uD800','\\uD800\\uDC00']"); + } + + @Test + void escapedSurrogatesInSkippedStringValue() throws Exception { + for (int mode : MODES) { + try (JsonParser p = open(FACTORY, mode, "{\"outer\":{\"k\":\"\\uDC00\\uD800\"}}")) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + p.skipChildren(); + assertToken(JsonToken.END_OBJECT, p.currentToken()); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } + } + } + + private void _testStringValuePassThrough(JsonFactory f, String doc) throws Exception { + for (int mode : MODES) { + try (JsonParser p = open(f, mode, doc)) { + assertToken(JsonToken.START_ARRAY, p.nextToken()); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("\uD800", p.getText(), "mode=" + mode); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("\uDC00", p.getText(), "mode=" + mode); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("\uDC00\uD800", p.getText(), "mode=" + mode); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("\uD800\uDC00", p.getText(), "mode=" + mode); + assertToken(JsonToken.END_ARRAY, p.nextToken()); + } + } + } + + // ---- text-buffer segment-boundary coverage ---------------------------- + + /** + * Regression test for the buffer overflow that would occur if the high + * surrogate write in {@code _parseName2} landed on the last slot of the + * current text-buffer segment. Without the bounds check between the hi + * and lo writes, the fall-through path would then write the low + * surrogate at index {@code outBuf.length}, throwing + * {@link ArrayIndexOutOfBoundsException} instead of returning a valid + * astral field name. + * + *

The default first-segment size in {@code TextBuffer} is 200 UTF-16 + * code units; we sweep several padding sizes so at least one lands the + * high-surrogate write on {@code outBuf.length - 1}. + */ + @Test + void longFieldNameWithSurrogatePairAtSegmentBoundary() throws Exception { + int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; + for (int mode : MODES) { + for (int pad : padSizes) { + String doc = "{\"" + pad(pad) + HI_ESCAPE + LO_ESCAPE + "\":1}"; + try (JsonParser p = open(FACTORY, mode, doc)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + String name = p.currentName(); + assertEquals(pad + 1, name.codePointCount(0, name.length()), + "codepoint count for pad=" + pad + ", mode=" + mode); + assertEquals(0x1F600, name.codePointAt(pad), + "astral cp at index " + pad + " for pad=" + pad + ", mode=" + mode); + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + } + } + } + } + + @Test + void longFieldNameWithLoneTrailingSurrogateAtSegmentBoundary() throws Exception { + // Same padding sweep, but lone-trailing-surrogate escape — must still + // reject cleanly at the boundary rather than overflowing. + int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; + for (int pad : padSizes) { + assertRejects(FACTORY, "{\"" + pad(pad) + "\\uDC00\":1}"); + } + } + + // ---- input-buffer boundary coverage ----------------------------------- + + /** + * High-surrogate escape ending exactly at the end of the first input + * buffer, so the following low-surrogate escape (or other char, or EOF) + * is only seen after {@code _loadMore()}. Applies to Reader-backed input + * only: String input is never refilled. + */ + @Test + void highSurrogateAtInputBufferEndInFieldName() throws Exception { + final String lead = "{\"" + pad(INPUT_BUFFER_LEN - 2 - HI_ESCAPE.length()) + HI_ESCAPE; + assertEquals(INPUT_BUFFER_LEN, lead.length()); + + for (int mode : ALL_TEXT_MODES) { + // Valid pair split across buffers: accepted + try (JsonParser p = open(FACTORY, mode, lead + LO_ESCAPE + "\":1}")) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + String name = p.currentName(); + assertEquals(0x1F600, name.codePointAt(name.length() - 2), "mode=" + mode); + } + // High surrogate followed by non-escape char in next buffer + try (JsonParser p = open(FACTORY, mode, lead + "x\":1}")) { + p.nextToken(); + p.nextToken(); + fail("Expected JsonParseException (mode=" + mode + ")"); + } catch (JsonParseException e) { + verifyException(e, "surrogate"); + } + // High surrogate as the very last content + try (JsonParser p = open(FACTORY, mode, lead)) { + p.nextToken(); + p.nextToken(); + fail("Expected JsonParseException (mode=" + mode + ")"); + } catch (JsonParseException e) { + verifyException(e, "end-of-input"); + } + } + } + + // ---- helpers ---------------------------------------------------------- + + private JsonParser open(JsonFactory f, int mode, String doc) throws Exception { + if (mode == MODE_STRING) { + return f.createParser(doc); + } + return createParser(f, mode, doc); + } + + private static String pad(int len) { + StringBuilder sb = new StringBuilder(len); + for (int i = 0; i < len; i++) { + sb.append('a'); + } + return sb.toString(); + } + + private void assertRejects(JsonFactory f, String doc) throws Exception { + for (int mode : MODES) { + try (JsonParser p = open(f, mode, doc)) { + while (p.nextToken() != null) { } + fail("Expected JsonParseException for malformed surrogate escape (mode=" + + mode + ") in: " + doc); + } catch (JsonParseException e) { + verifyException(e, "surrogate"); + } + } + } +} diff --git a/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java b/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java deleted file mode 100644 index 4f055eb297..0000000000 --- a/src/test/java/com/fasterxml/jackson/core/read/EscapedSurrogateInStringValue1683Test.java +++ /dev/null @@ -1,369 +0,0 @@ -package com.fasterxml.jackson.core.read; - -import org.junit.jupiter.api.Test; - -import com.fasterxml.jackson.core.JUnit5TestBase; -import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonParseException; -import com.fasterxml.jackson.core.JsonToken; -import com.fasterxml.jackson.core.json.JsonReadFeature; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; - -/** - * Tests for [jackson-core#1683]: JSON-escaped lone surrogates in string values - * (and field names) must be rejected by the Reader-based parser, matching the - * fix applied for the UTF-8 stream parser in [jackson-core#1541]. - * - *

Only exercises {@link com.fasterxml.jackson.core.json.ReaderBasedJsonParser} - * paths; UTF-8 / async parsers are covered separately. Every test runs over - * {@link #MODES}: {@code String} input (whole content in one buffer), plain - * {@code Reader} and 1-char-at-a-time throttled {@code Reader}; the latter - * forces every escape to span input-buffer boundaries. - */ -class EscapedSurrogateInStringValue1683Test extends JUnit5TestBase -{ - // Local pseudo-mode for `JsonFactory.createParser(String)`, in addition - // to Reader-backed ALL_TEXT_MODES - private final static int MODE_STRING = -1; - - private final static int[] MODES = new int[] { - MODE_STRING, MODE_READER, MODE_READER_THROTTLED - }; - - // Size of char input buffer `ReaderBasedJsonParser` reads into - // (BufferRecycler.CHAR_TOKEN_BUFFER) - private final static int INPUT_BUFFER_LEN = 4000; - - // 6-char escape for high surrogate of U+1F600 - private final static String HI_ESCAPE = "\\uD83D"; - private final static String LO_ESCAPE = "\\uDE00"; - - private final JsonFactory FACTORY = new JsonFactory(); - - private final JsonFactory APOS_FACTORY = JsonFactory.builder() - .enable(JsonReadFeature.ALLOW_SINGLE_QUOTES) - .build(); - - // JSON documents. Each backslash-u escape is written using an explicit - // '\\' + 'u' + hex prefix so the Java pre-lexer doesn't fold it into a - // single UTF-16 code unit before it reaches the parser. - private static final String LONE_LEADING_VALUE = "{\"k\":\"\\uD800\"}"; - private static final String LONE_TRAILING_VALUE = "{\"k\":\"\\uDC00\"}"; - private static final String REVERSED_PAIR_VALUE = "{\"k\":\"\\uDC00\\uD800\"}"; - private static final String VALID_PAIR_VALUE = "{\"k\":\"\\uD800\\uDC00\"}"; - private static final String LONE_TRAILING_NAME = "{\"\\uDC00\":1}"; - private static final String LONE_LEADING_NAME = "{\"\\uD800\":1}"; - private static final String REVERSED_PAIR_NAME = "{\"\\uDC00\\uD800\":1}"; - - private static final String APOS_LONE_LEADING = "{'k':'\\uD800'}"; - private static final String APOS_LONE_TRAILING = "{'k':'\\uDC00'}"; - private static final String APOS_REVERSED_PAIR = "{'k':'\\uDC00\\uD800'}"; - private static final String APOS_VALID_PAIR = "{'k':'\\uD800\\uDC00'}"; - - // ---- string-value coverage -------------------------------------------- - - @Test - void loneLeadingSurrogateInStringValue() throws Exception { - assertRejects(FACTORY, LONE_LEADING_VALUE); - } - - @Test - void loneTrailingSurrogateInStringValue() throws Exception { - assertRejects(FACTORY, LONE_TRAILING_VALUE); - } - - @Test - void reversedSurrogatePairInStringValue() throws Exception { - assertRejects(FACTORY, REVERSED_PAIR_VALUE); - } - - @Test - void validSurrogatePairInStringValue() throws Exception { - assertAcceptsValidPair(FACTORY, VALID_PAIR_VALUE); - } - - // ---- field-name coverage ---------------------------------------------- - - @Test - void loneTrailingSurrogateInFieldName() throws Exception { - assertRejects(FACTORY, LONE_TRAILING_NAME); - } - - @Test - void loneLeadingSurrogateInFieldName() throws Exception { - assertRejects(FACTORY, LONE_LEADING_NAME); - } - - @Test - void reversedSurrogatePairInFieldName() throws Exception { - assertRejects(FACTORY, REVERSED_PAIR_NAME); - } - - // ---- single-quoted string coverage (exercises _handleApos) ------------ - - @Test - void singleQuotedStringWithLoneLeadingSurrogate() throws Exception { - assertRejects(APOS_FACTORY, APOS_LONE_LEADING); - } - - @Test - void singleQuotedStringWithLoneTrailingSurrogate() throws Exception { - assertRejects(APOS_FACTORY, APOS_LONE_TRAILING); - } - - @Test - void singleQuotedStringWithReversedSurrogatePair() throws Exception { - assertRejects(APOS_FACTORY, APOS_REVERSED_PAIR); - } - - @Test - void singleQuotedStringWithValidSurrogatePair() throws Exception { - assertAcceptsValidPair(APOS_FACTORY, APOS_VALID_PAIR); - } - - // ---- skipChildren coverage (exercises _skipString) -------------------- - - @Test - void skipChildrenPastLoneTrailingSurrogate() throws Exception { - assertSkipChildrenRejects("{\"outer\":{\"k\":\"\\uDC00\"}}"); - } - - @Test - void skipChildrenPastHighSurrogateFollowedByNonEscape() throws Exception { - // High surrogate followed by a plain character rather than another escape - assertSkipChildrenRejects("{\"outer\":{\"k\":\"\\uD800x\"}}"); - } - - // ---- text-buffer segment-boundary coverage (exercises _parseName2) ---- - - /** - * Regression test for the buffer overflow that would occur if the high - * surrogate write in {@code _parseName2} landed on the last slot of the - * current text-buffer segment. Without the bounds check between the hi - * and lo writes, the fall-through path would then write the low - * surrogate at index {@code outBuf.length}, throwing - * {@link ArrayIndexOutOfBoundsException} instead of returning a valid - * astral field name. - * - *

The default first-segment size in {@code TextBuffer} is 200 UTF-16 - * code units; we sweep several padding sizes so at least one lands the - * high-surrogate write on {@code outBuf.length - 1}. - */ - @Test - void longFieldNameWithSurrogatePairAtSegmentBoundary() throws Exception { - int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; - for (int mode : MODES) { - for (int pad : padSizes) { - String doc = "{\"" + pad(pad) + HI_ESCAPE + LO_ESCAPE + "\":1}"; - try (JsonParser p = open(FACTORY, mode, doc)) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - String name = p.currentName(); - assertEquals(pad + 1, name.codePointCount(0, name.length()), - "codepoint count for pad=" + pad + ", mode=" + mode); - assertEquals(0x1F600, name.codePointAt(pad), - "astral cp at index " + pad + " for pad=" + pad + ", mode=" + mode); - assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); - assertToken(JsonToken.END_OBJECT, p.nextToken()); - } - } - } - } - - @Test - void longFieldNameWithLoneTrailingSurrogateAtSegmentBoundary() throws Exception { - // Same padding sweep, but lone-trailing-surrogate escape — must still - // reject cleanly at the boundary rather than overflowing. - int[] padSizes = { 199, 200, 201, 399, 400, 401, 4000 }; - for (int pad : padSizes) { - assertRejects(FACTORY, "{\"" + pad(pad) + "\\uDC00\":1}"); - } - } - - // ---- input-buffer boundary coverage ----------------------------------- - // - // High-surrogate escape ending exactly at the end of the first input - // buffer, so the following low-surrogate escape (or other char, or EOF) - // is only seen after `_loadMore()`. Applies to Reader-backed input only: - // String input is never refilled. - - @Test - void highSurrogateAtInputBufferEnd_fieldName() throws Exception { - _testHighSurrogateAtInputBufferEnd(FACTORY, "{\"", "\":1}", false); - } - - @Test - void highSurrogateAtInputBufferEnd_stringValue() throws Exception { - _testHighSurrogateAtInputBufferEnd(FACTORY, "[\"", "\"]", false); - } - - @Test - void highSurrogateAtInputBufferEnd_aposStringValue() throws Exception { - _testHighSurrogateAtInputBufferEnd(APOS_FACTORY, "['", "']", false); - } - - @Test - void highSurrogateAtInputBufferEnd_skippedStringValue() throws Exception { - _testHighSurrogateAtInputBufferEnd(FACTORY, "[\"", "\"]", true); - } - - private void _testHighSurrogateAtInputBufferEnd(JsonFactory f, - String prefix, String suffix, boolean skip) throws Exception - { - final String lead = prefix + pad(INPUT_BUFFER_LEN - prefix.length() - HI_ESCAPE.length()) - + HI_ESCAPE; - assertEquals(INPUT_BUFFER_LEN, lead.length()); - - for (int mode : ALL_TEXT_MODES) { - // Valid pair split across buffers: accepted - try (JsonParser p = open(f, mode, lead + LO_ESCAPE + suffix)) { - String text = _readFirstString(p, skip); - if (!skip) { - assertEquals(0x1F600, text.codePointAt(text.length() - 2), "mode=" + mode); - } - } - // High surrogate followed by non-escape char in next buffer - try (JsonParser p = open(f, mode, lead + "x" + suffix)) { - _readFirstString(p, skip); - fail("Expected JsonParseException (mode=" + mode + ")"); - } catch (JsonParseException e) { - verifyException(e, "surrogate"); - } - // High surrogate as the very last content - try (JsonParser p = open(f, mode, lead)) { - _readFirstString(p, skip); - fail("Expected JsonParseException (mode=" + mode + ")"); - } catch (JsonParseException e) { - verifyException(e, "end-of-input"); - } - } - } - - // Returns first field name or String value; or, if `skip`, skips value - // (without accessing text) and returns null - private String _readFirstString(JsonParser p, boolean skip) throws Exception - { - JsonToken t = p.nextToken(); - if (t == JsonToken.START_OBJECT) { - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - return p.currentName(); - } - assertToken(JsonToken.START_ARRAY, t); - assertToken(JsonToken.VALUE_STRING, p.nextToken()); - if (skip) { - assertToken(JsonToken.END_ARRAY, p.nextToken()); - return null; - } - return p.getText(); - } - - // ---- mixed escape / literal-surrogate coverage (locks §5.6 behavior) -- - - /** - * The Reader path can accept a Java {@code String} input that contains a - * literal (unescaped) surrogate {@code char}. Verify the deliberate - * strict-rejection behavior when an escape surrogate is followed by, or - * preceded by, a literal surrogate char: both cases MUST reject with - * {@code JsonParseException}, even though a lenient reader might treat - * the escape+literal combination as a valid UTF-16 pair. - */ - @Test - void escapedHighFollowedByLiteralLowSurrogate_isRejected() throws Exception { - assertRejects(FACTORY, "{\"k\":\"\\uD800" + '\uDC00' + "\"}"); - } - - @Test - void literalHighFollowedByEscapedLowSurrogate_isRejected() throws Exception { - assertRejects(FACTORY, "{\"k\":\"" + '\uD800' + "\\uDC00\"}"); - } - - @Test - void bothLiteralSurrogatesFormingValidPair_isAccepted() throws Exception { - // Literal char pair — parser does not decode escapes, so no surrogate - // guard fires. This exercises the pre-existing accept path for - // literal chars in the input stream and locks that we do not - // regress it while validating the escape paths. - String doc = "{\"k\":\"" + '\uD800' + '\uDC00' + "\"}"; - for (int mode : MODES) { - try (JsonParser p = open(FACTORY, mode, doc)) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - assertToken(JsonToken.VALUE_STRING, p.nextToken()); - assertEquals(0x10000, p.getText().codePointAt(0), - "expected U+10000 from literal surrogate pair (mode=" + mode + ")"); - assertToken(JsonToken.END_OBJECT, p.nextToken()); - } - } - } - - // ---- helpers ---------------------------------------------------------- - - private JsonParser open(JsonFactory f, int mode, String doc) throws Exception { - if (mode == MODE_STRING) { - return f.createParser(doc); - } - return createParser(f, mode, doc); - } - - private static String pad(int len) { - StringBuilder sb = new StringBuilder(len); - for (int i = 0; i < len; i++) { - sb.append('a'); - } - return sb.toString(); - } - - private void assertRejects(JsonFactory f, String doc) throws Exception { - for (int mode : MODES) { - try (JsonParser p = open(f, mode, doc)) { - // Drive tokens until either the parser throws or the doc ends. - while (p.nextToken() != null) { - if (p.currentToken() == JsonToken.VALUE_STRING - || p.currentToken() == JsonToken.FIELD_NAME) { - // Force decode; some paths defer surrogate work until - // the caller pulls the text. - p.getText(); - } - } - fail("Expected JsonParseException for malformed surrogate escape (mode=" - + mode + ") in: " + doc); - } catch (JsonParseException e) { - verifyException(e, "surrogate"); - } - } - } - - private void assertSkipChildrenRejects(String doc) throws Exception { - for (int mode : MODES) { - try (JsonParser p = open(FACTORY, mode, doc)) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - assertToken(JsonToken.START_OBJECT, p.nextToken()); - p.skipChildren(); - fail("Expected JsonParseException (mode=" + mode + ")"); - } catch (JsonParseException e) { - verifyException(e, "surrogate"); - } - } - } - - private void assertAcceptsValidPair(JsonFactory f, String doc) throws Exception { - for (int mode : MODES) { - try (JsonParser p = open(f, mode, doc)) { - assertToken(JsonToken.START_OBJECT, p.nextToken()); - assertToken(JsonToken.FIELD_NAME, p.nextToken()); - assertEquals("k", p.currentName()); - assertToken(JsonToken.VALUE_STRING, p.nextToken()); - String text = p.getText(); - // U+10000 encodes as the surrogate pair D800 DC00 in Java strings. - assertEquals(2, text.length(), "expected two UTF-16 code units (mode=" + mode + ")"); - assertEquals(0x10000, text.codePointAt(0), "mode=" + mode); - assertToken(JsonToken.END_OBJECT, p.nextToken()); - } - } - } -} From 7bb756c7f5200aba8adddc51c50cc6b60dc475ab Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 2 Oct 2026 11:53:16 -0700 Subject: [PATCH 8/9] ... --- release-notes/CREDITS-2.x | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 757418ec29..03dae5508b 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -562,6 +562,5 @@ Burak KALAYCI (@kalayciburak) Elankumaran Srinivasan (@elang2) * Contributed #1683: `ReaderBasedJsonParser` should reject JSON-escaped lone - surrogates in field names and string values (mirror of #1541 fix in - `UTF8StreamJsonParser`) + surrogates in field names (mirror of #1541 fix in `UTF8StreamJsonParser`) (2.23.0) From 153e969a586ebaa954afebaa4fa6135c42def48d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 2 Oct 2026 12:14:21 -0700 Subject: [PATCH 9/9] tiny comment change --- .../jackson/core/json/ReaderBasedJsonParser.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java index b2f18cb027..e7f8a47400 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java @@ -1866,9 +1866,8 @@ private String _parseName2(int startPtr, int hash, int endChar) throws IOExcepti * For now let's assume it does not. */ c = _decodeEscaped(); - // [jackson-core#1683]: Validate JSON-escaped surrogates in - // field name. Mirror of [jackson-core#1541] fix in - // UTF8StreamJsonParser. + // 05-Sep-2026, elang2: [core#1683] Validate JSON-escaped surrogates + // in field name; mirror of [core#1541] fix in UTF8StreamJsonParser. if (c >= 0xD800 && c <= 0xDFFF) { if (c < 0xDC00) { // high surrogate: must be followed by low surrogate escape char hi = c; @@ -3077,9 +3076,8 @@ protected void _reportInvalidToken(String matchedPart, String msg) throws IOExce throw _constructReadException(fullMsg, loc); } - // [jackson-core#1683]: helpers for reporting malformed JSON-escaped surrogate - // sequences. Wording mirrors the messages introduced for UTF8StreamJsonParser - // by the [jackson-core#1541] fix, extended to name the offending context. + // 05-Sep-2026, elang2: [core#1683] Helpers for reporting malformed JSON-escaped + // surrogates; wording mirrors UTF8StreamJsonParser ([core#1541]), plus context. private void _reportUnexpectedLowSurrogate(int lo, String ctx) throws IOException { _reportError("Unexpected low surrogate in " + ctx + ": 0x" + Integer.toHexString(lo)); }