From e86a0aac726a8f83b11bb09fb93625c60818becd Mon Sep 17 00:00:00 2001 From: Erik Berlin Date: Thu, 5 Nov 2026 12:23:19 -0800 Subject: [PATCH] Java parser: include the position in string decoding errors StringDecoder built its own ParserError, so errors such as an invalid escape, an incomplete \u escape, a broken surrogate pair or a raw control character in a string had no line, column, message suffix or json_path. The parser's parseError, which adds all of these, was never used for them. StringDecoder now throws a DecodeError that carries the message and the offset to report, and the parser turns it into a regular parse error at that offset. The offsets follow the C parser: errors in an escape sequence are reported at its backslash, surrogate errors at the start of the text after the previous escape, and raw control characters where they are. The messages keep their current wording, apart from surrogate errors, whose snippet now starts at that same offset. This lets test_parse_error_position_at_newline run on JRuby, and adds a test for errors in escape sequences. Part of https://github.com/ruby/json/issues/1123 --- CHANGES.md | 1 + java/src/json/ext/ByteListTranscoder.java | 4 +- java/src/json/ext/Parser.java | 14 +-- java/src/json/ext/StringDecoder.java | 107 ++++++++++++++-------- test/json/json_parser_test.rb | 17 +++- 5 files changed, 94 insertions(+), 49 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 2bad52aed..10cc99d71 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,7 @@ * Fix `JSON::ParserError#json_path` raising `NoMethodError` when the document was parsed with `symbolize_names: true`. * Fix `JSON.generate` not detecting duplicate keys such as `{ "1" => 1, 1 => 2 }`. * JRuby: raise `TypeError` when a String subclass used as a Hash key has a `to_s` that doesn't return a String, like the C extension. +* JRuby: parser errors raised while decoding a string, such as an invalid escape or a raw control character, now include the position (`line`, `column` and the message suffix) and `json_path`, matching the C extension. ### 2026-10-13 (3.1.0) diff --git a/java/src/json/ext/ByteListTranscoder.java b/java/src/json/ext/ByteListTranscoder.java index 7ee9de344..964596f33 100644 --- a/java/src/json/ext/ByteListTranscoder.java +++ b/java/src/json/ext/ByteListTranscoder.java @@ -156,9 +156,9 @@ protected void quoteStop(int endPos) throws IOException { protected abstract void append(byte[] origin, int start, int length) throws IOException; - protected abstract RaiseException invalidUtf8(ThreadContext context); + protected abstract RuntimeException invalidUtf8(ThreadContext context); - protected RaiseException incompleteUtf8(ThreadContext context) { + protected RuntimeException incompleteUtf8(ThreadContext context) { return invalidUtf8(context); } } diff --git a/java/src/json/ext/Parser.java b/java/src/json/ext/Parser.java index eb5ce3a9a..26bf6ef87 100644 --- a/java/src/json/ext/Parser.java +++ b/java/src/json/ext/Parser.java @@ -732,13 +732,15 @@ private IRubyObject parseString(boolean isName) { } private ByteList decodeString(ThreadContext context, ByteList byteList, int start, int end, boolean isAscii) { - if (isAscii) { - return decoder.decodeNoValidate(context, byteList, start, end, chunks); - } - if (!validateUTF8Strings) { - return decoder.decodeNoValidate(context, byteList, start, end, chunks); + try { + if (isAscii || !validateUTF8Strings) { + return decoder.decodeNoValidate(context, byteList, start, end, chunks); + } + return decoder.decode(context, byteList, start, end); + } catch (StringDecoder.DecodeError e) { + cursor = begin + e.offset; + throw parseError(context.runtime.newString(e.message), null); } - return decoder.decode(context, byteList, start, end); } private static boolean isLetter(byte b) { diff --git a/java/src/json/ext/StringDecoder.java b/java/src/json/ext/StringDecoder.java index 362a0d685..1d8743317 100644 --- a/java/src/json/ext/StringDecoder.java +++ b/java/src/json/ext/StringDecoder.java @@ -5,7 +5,6 @@ */ package json.ext; -import org.jruby.exceptions.RaiseException; import org.jruby.runtime.ThreadContext; import org.jruby.util.ByteList; @@ -19,10 +18,31 @@ */ final class StringDecoder extends ByteListTranscoder { /** - * Stores the offset of the high surrogate when reading a surrogate pair, - * or -1 when not. + * A decoding error, with the offset in the source ByteList (relative to + * its begin) that the parser reports as the error's line and column. */ - private int surrogatePairStart = -1; + static final class DecodeError extends RuntimeException { + final ByteList message; + final int offset; + + DecodeError(ByteList message, int offset) { + super(null, null, false, false); + this.message = message; + this.offset = offset; + } + } + + /** + * Stores the offset of the backslash that starts the escape sequence + * being read, or -1 when not in one. + */ + private int escapeStart = -1; + + /** + * Stores the offset where the text after the last escape sequence (or the + * string) starts. The C parser reports surrogate errors there. + */ + private int segmentStart; private boolean allowControlCharacters = false; private boolean allowInvalidEscape = false; @@ -42,6 +62,8 @@ ByteList decode(ThreadContext context, ByteList src, int start, int end) { try { init(src, start, end); this.out = new ByteList(end - start); + escapeStart = -1; + segmentStart = start; while (hasNext()) { handleChar(context, readUtf8Char(context)); } @@ -58,6 +80,8 @@ ByteList decodeNoValidate(ThreadContext context, ByteList src, int start, int en try { init(src, start, end); this.out = new ByteList(end - start); + escapeStart = -1; + segmentStart = start; final byte[] data = src.unsafeBytes(); final int base = src.begin(); final int absEnd = base + srcEnd; @@ -114,8 +138,9 @@ private void handleChar(ThreadContext context, int c) throws IOException { } private void handleEscapeSequence(ThreadContext context) throws IOException { + escapeStart = charStart; ensureMin(context, 1); - int character = readUtf8Char(context); + int character = super.readUtf8Char(context); switch (character) { case 'b': append('\b'); @@ -148,18 +173,23 @@ private void handleEscapeSequence(ThreadContext context) throws IOException { handleLowSurrogate(context, (char)cp); } else if (Character.isLowSurrogate((char)cp)) { // low surrogate with no high surrogate - throw invalidUtf8(context); + throw invalidSurrogate(); } else { writeUtf8Char(cp); } break; default: + if (character < 0x20 && !allowControlCharacters) { + throw invalidControlChar(context, escapeStart); + } if (allowInvalidEscape) { writeUtf8Char(character); } else { throw invalidEscape(context); } } + escapeStart = -1; + segmentStart = pos; } protected void append(int b) throws IOException { @@ -171,22 +201,20 @@ protected void append(byte[] origin, int start, int length) throws IOException { } private void handleLowSurrogate(ThreadContext context, char highSurrogate) throws IOException { - surrogatePairStart = charStart; - ensureMin(context, 1); + if (pos + 1 > srcEnd) throw invalidSurrogate(); int lowSurrogate = readUtf8Char(context); if (lowSurrogate == '\\') { - ensureMin(context, 5); - if (readUtf8Char(context) != 'u') throw invalidUtf8(context); + if (pos + 5 > srcEnd) throw invalidSurrogate(); + if (readUtf8Char(context) != 'u') throw invalidSurrogate(); lowSurrogate = readHex(context); } if (Character.isLowSurrogate((char)lowSurrogate)) { writeUtf8Char(Character.toCodePoint(highSurrogate, (char)lowSurrogate)); - surrogatePairStart = -1; } else { - throw invalidUtf8(context); + throw invalidSurrogate(); } } @@ -239,40 +267,41 @@ private int readHex(ThreadContext context) { return result; } + // The offsets follow the C parser: errors in an escape sequence are + // reported at its backslash, surrogate errors at the text before it, and + // raw control characters where they are. The message prefixes are unchanged. + @Override - protected RaiseException invalidUtf8(ThreadContext context) { - ByteList message = new ByteList( - ByteList.plain("partial character in source, " + - "but hit end near ")); - int start = surrogatePairStart != -1 ? surrogatePairStart : charStart; - message.append(src, start, srcEnd - start); - return Utils.newException(context, Utils.M_PARSER_ERROR, - context.runtime.newString(message)); + protected DecodeError invalidUtf8(ThreadContext context) { + int start = escapeStart != -1 ? escapeStart : charStart; + return error("partial character in source, but hit end near ", start, start); + } + + protected DecodeError invalidControlChar(ThreadContext context) { + return invalidControlChar(context, charStart); } - protected RaiseException invalidControlChar(ThreadContext context) { - ByteList message = new ByteList( - ByteList.plain("invalid ASCII control character in string: ")); - message.append(src, charStart, srcEnd - charStart); - return Utils.newException(context, Utils.M_PARSER_ERROR, - context.runtime.newString(message)); + private DecodeError invalidControlChar(ThreadContext context, int offset) { + return error("invalid ASCII control character in string: ", charStart, offset); } - protected RaiseException invalidEscape(ThreadContext context) { - ByteList message = new ByteList( - ByteList.plain("invalid escape character in string: ")); - message.append(src, charStart, srcEnd - charStart); - return Utils.newException(context, Utils.M_PARSER_ERROR, - context.runtime.newString(message)); + private DecodeError invalidSurrogate() { + return error("partial character in source, but hit end near ", segmentStart, segmentStart); } - protected RaiseException invalidUnicodeEscape(ThreadContext context, int escapeStart) { - ByteList message = new ByteList( - ByteList.plain("incomplete unicode character escape sequence at ")); + protected DecodeError invalidEscape(ThreadContext context) { + return error("invalid escape character in string: ", charStart, escapeStart); + } + + protected DecodeError invalidUnicodeEscape(ThreadContext context, int numberStart) { // Point at the backslash-u that introduced the escape. - int start = Math.max(escapeStart - 2, 0); - message.append(src, start, srcEnd - start); - return Utils.newException(context, Utils.M_PARSER_ERROR, - context.runtime.newString(message)); + int start = Math.max(numberStart - 2, 0); + return error("incomplete unicode character escape sequence at ", start, start); + } + + private DecodeError error(String prefix, int snippetStart, int offset) { + ByteList message = new ByteList(ByteList.plain(prefix)); + message.append(src, snippetStart, srcEnd - snippetStart); + return new DecodeError(message, offset); } } diff --git a/test/json/json_parser_test.rb b/test/json/json_parser_test.rb index 0549f6f4c..3662085e2 100644 --- a/test/json/json_parser_test.rb +++ b/test/json/json_parser_test.rb @@ -948,8 +948,6 @@ def test_parse_error_position end def test_parse_error_position_at_newline - omit "JRuby string decoding errors don't contain positions" if RUBY_ENGINE == "jruby" - ["[1,\n\"ab\ncd\"]", "[1,\n\"日本\ncd\"]"].each do |json| error = assert_raise(JSON::ParserError) { JSON.parse(json) } assert_equal 2, error.line @@ -958,6 +956,21 @@ def test_parse_error_position_at_newline end end + def test_parse_error_position_in_string_escapes + { + %([1,\n"\\q"]) => [2, 2], + %([1,\n"ab\\u12"]) => [2, 4], + %([1,\n"\\u12\n4"]) => [2, 6], + %([1,\n"ab\\ud800x"]) => [2, 2], + %([1,\n"x\\n\\udc00"]) => [2, 5], + %(["日本\\q"]) => [1, 5], + }.each do |json, (line, column)| + error = assert_raise(JSON::ParserError) { JSON.parse(json) } + assert_equal [line, column], [error.line, error.column], json.inspect + assert_include error.message, "at line #{line} column #{column}" + end + end + def test_parse_error_position_with_multibyte_characters { '["日本", @]' => [1, 8],