From f3f3270f707930f60bc60e873f08b3eb88ba06f5 Mon Sep 17 00:00:00 2001 From: Masataka Pocke Kuwabara Date: Mon, 6 Jul 2026 13:16:42 +0900 Subject: [PATCH 1/2] [ruby/json] Fix ResumableParser losing tokens before a feed-boundary suspension When a feed boundary fell right after a consumed token -- a `,` or an opening `[` / `{` -- whose effect was not yet committed to the parser's persistent state, the token was dropped on resume, producing parse errors or silently wrong values. ## Reproduction ```ruby require "json" def parse_in_two(first, second, **opts) parser = JSON::ResumableParser.new(**opts) parser << first parser.parse parser << second parser.parse parser.value rescue JSON::ParserError => e e.message end # a ',' and its closing bracket split across the boundary p parse_in_two("[1,", "]", allow_trailing_comma: true) # a comment right after '[' split across the boundary p parse_in_two("[/*", "*/1]", allow_comments: true) ``` ## Expected Behavior ``` [1] [1] ``` Feeding `[1,]` or `[/**/1]` in a single chunk parses to `[1]`, so splitting it at the comma or inside the comment should make no difference. ## Actual Behavior ``` "unexpected character: ']'" 1 ``` For the comma, the closing bracket that arrived in the later chunk was rejected because the comma had been forgotten. For the bracket, the array's first element leaked out as a bare top-level document (`1` instead of `[1]`), and a following `parse` would raise `unexpected character: ']'` on the orphaned closing bracket. The same shapes affect objects (`{"a":1,` + `}`, `{/*` + `*/"a":1}`), line comments (`[//` + `"x\n1]"`), and comments right after a comma. It happens with the default configuration too, since comments are accepted (with a deprecation warning) unless disabled. ## Description Both bugs share one root cause: the parser consumed a token, then hit a suspension point (end of buffer, possibly inside a comment) before recording the token's effect in the persistent frame/phase state, so resuming had no memory of the token. They differ in where that record lives, so the fix has two parts. On `,`, the comma phases used to eat whitespace and comments before committing the frame's phase, in order to peek for a trailing-comma close. The fix advances the phase to `JSON_PHASE_VALUE` (array) or `JSON_PHASE_OBJECT_KEY` (object) immediately after consuming the comma. Trailing-comma detection moves into those phases: an element/key position that finds `]` / `}` (only reachable after a comma, since an empty container closes inline) hands off to the comma phase to close. On `[` / `{`, the bracket's frame is only pushed after the empty-container check, and pushing it earlier is not an option: an empty container is decoded inline without a frame, which is what keeps `[]` from consuming a nesting level (`[[]]` parses with `max_nesting: 1`). Committing a frame first would break that property or require reworking nesting accounting, affecting the non-resumable parser as well. Instead, widen the rewind: the suspension already rewinds the cursor, it just rewound to the start of the comment, which is after the bracket. `json_eat_comments` gains a `resume_pos` that, in resumable mode only, overrides the rewind target for all three suspension sites (unterminated block comment, unterminated line comment, and a comment marker split at the boundary), and the `[` / `{` cases pass the bracket's own position via the new `json_eat_whitespace_resume_at` variant. Re-reading the bracket and the comment on resume mirrors how strings and numbers already rewind to `value_start` when they hit the end of the buffer. Non-resumable parsing is unaffected in both parts: the same tokens are consumed in the same order, and error positions still point at the comment. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PpeQSEFkF1X1Uuzjx9BsYe https://github.com/ruby/json/commit/cc010c4688 --- ext/json/parser/parser.c | 68 ++++++++++++-------- test/json/resumable_parser_test.rb | 99 ++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 24 deletions(-) diff --git a/ext/json/parser/parser.c b/ext/json/parser/parser.c index 58dd281884f711..9b7b4be3927ed5 100644 --- a/ext/json/parser/parser.c +++ b/ext/json/parser/parser.c @@ -768,13 +768,18 @@ static const rb_data_type_t JSON_ParserConfig_type; const char *COMMENT_DEPRECATION_MESSAGE = "Encountered comment in JSON. This will raise an error in json 3.0 unless enabled via `allow_comments: true`"; NOINLINE(static) void -json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config) +json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config, const char *resume_pos) { if (config->on_comment == JSON_RAISE) { raise_syntax_error("unexpected token %s", state); } const char *start = state->cursor; + // An incomplete comment suspends a resumable parse by rewinding the cursor + // and throwing. Callers that already consumed a token not yet committed to + // the frame stack pass resume_pos so the rewind re-reads that token too. + // Non-resumable error positions keep pointing at the comment either way. + const char *rewind_pos = (state->parser && resume_pos) ? resume_pos : start; state->cursor++; switch (peek(state)) { @@ -786,7 +791,7 @@ json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config) // the comment unterminated instead of consuming to end as a one-shot // parse would. if (state->parser) { - raise_eos_error_at("unterminated comment, expected end of line", state, start); + raise_eos_error_at("unterminated comment, expected end of line", state, rewind_pos); } state->cursor = state->end; } else { @@ -800,7 +805,7 @@ json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config) while (true) { const char *next_match = memchr(state->cursor, '*', state->end - state->cursor); if (!next_match) { - raise_eos_error_at("unterminated comment, expected closing '*/'", state, start); + raise_eos_error_at("unterminated comment, expected closing '*/'", state, rewind_pos); } state->cursor = next_match + 1; @@ -812,7 +817,7 @@ json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config) break; } default: - raise_parse_error_at("unexpected token %s", state, start, eos(state)); + raise_parse_error_at("unexpected token %s", state, eos(state) ? rewind_pos : start, eos(state)); break; } @@ -823,7 +828,7 @@ json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config) } ALWAYS_INLINE(static) void -json_eat_whitespace(JSON_ParserState *state, JSON_ParserConfig *config, bool include_comments) +json_eat_whitespace_resume_at(JSON_ParserState *state, JSON_ParserConfig *config, bool include_comments, const char *resume_pos) { while (true) { switch (peek(state)) { @@ -858,7 +863,7 @@ json_eat_whitespace(JSON_ParserState *state, JSON_ParserConfig *config, bool inc return; } - json_eat_comments(state, config); + json_eat_comments(state, config, resume_pos); break; default: @@ -867,6 +872,12 @@ json_eat_whitespace(JSON_ParserState *state, JSON_ParserConfig *config, bool inc } } +ALWAYS_INLINE(static) void +json_eat_whitespace(JSON_ParserState *state, JSON_ParserConfig *config, bool include_comments) +{ + json_eat_whitespace_resume_at(state, config, include_comments, NULL); +} + static inline VALUE build_string(const char *start, const char *end, bool intern, bool symbolize) { if (symbolize) { @@ -1580,6 +1591,13 @@ ALWAYS_INLINE(static) bool json_parse_any(JSON_ParserState *state, JSON_ParserCo JSON_PHASE_VALUE: { json_eat_whitespace(state, config, true); + // A trailing comma lands us here expecting an element but finding the + // closing bracket; hand off to ARRAY_COMMA to close. An empty array + // closes inline at '[', so this position is only reached after a ','. + if (config->allow_trailing_comma && frame->type == JSON_FRAME_ARRAY && peek(state) == ']') { + goto JSON_PHASE_ARRAY_COMMA; + } + VALUE value; const char *value_start = state->cursor; @@ -1675,7 +1693,9 @@ ALWAYS_INLINE(static) bool json_parse_any(JSON_ParserState *state, JSON_ParserCo case '[': { state->cursor++; - json_eat_whitespace(state, config, true); + // The '[' is consumed but its frame is only pushed below, so a + // comment suspending here must resume from the bracket. + json_eat_whitespace_resume_at(state, config, true, value_start); const char next = peek(state); if (next == ']') { @@ -1704,7 +1724,8 @@ ALWAYS_INLINE(static) bool json_parse_any(JSON_ParserState *state, JSON_ParserCo case '{': { state->cursor++; - json_eat_whitespace(state, config, true); + // Same as '[': the frame is only pushed below. + json_eat_whitespace_resume_at(state, config, true, value_start); if (peek(state) == '}') { state->cursor++; @@ -1762,6 +1783,13 @@ ALWAYS_INLINE(static) bool json_parse_any(JSON_ParserState *state, JSON_ParserCo json_eat_whitespace(state, config, true); + // A trailing comma lands us here expecting a key but finding the closing + // brace; hand off to OBJECT_COMMA to close. An empty object closes inline + // at '{', so this position is only reached after a ','. + if (config->allow_trailing_comma && peek(state) == '}') { + goto JSON_PHASE_OBJECT_COMMA; + } + const char *start = state->cursor; if (RB_LIKELY(peek(state) == '"')) { @@ -1824,13 +1852,10 @@ ALWAYS_INLINE(static) bool json_parse_any(JSON_ParserState *state, JSON_ParserCo if (RB_LIKELY(next_char == ',')) { state->cursor++; - if (config->allow_trailing_comma) { - json_eat_whitespace(state, config, true); - if (peek(state) == ']') { - // Trailing comma: stay in COMMA to close on the next iteration. - goto JSON_PHASE_ARRAY_COMMA; - } - } + // Commit the phase before eating the whitespace that follows: an + // incomplete comment there would suspend the parse, and a phase not + // yet advanced past the ',' would drop it on resume. A trailing comma + // is recognized in JSON_PHASE_VALUE once the ']' is in the buffer. frame->phase = JSON_PHASE_VALUE; goto JSON_PHASE_VALUE; } else if (next_char == ']') { @@ -1869,15 +1894,10 @@ ALWAYS_INLINE(static) bool json_parse_any(JSON_ParserState *state, JSON_ParserCo if (RB_LIKELY(next_char == ',')) { state->cursor++; - json_eat_whitespace(state, config, true); - - if (config->allow_trailing_comma) { - if (peek(state) == '}') { - // Trailing comma: stay in COMMA to close on the next iteration. - goto JSON_PHASE_OBJECT_COMMA; - } - } - + // Commit the phase before eating the whitespace that follows: an + // incomplete comment there would suspend the parse, and a phase not + // yet advanced past the ',' would drop it on resume. A trailing comma + // is recognized in JSON_PHASE_OBJECT_KEY once the '}' is in the buffer. frame->phase = JSON_PHASE_OBJECT_KEY; goto JSON_PHASE_OBJECT_KEY; } else if (next_char == '}') { diff --git a/test/json/resumable_parser_test.rb b/test/json/resumable_parser_test.rb index 357052c9a4035a..5d7bf38bbf8ccc 100644 --- a/test/json/resumable_parser_test.rb +++ b/test/json/resumable_parser_test.rb @@ -241,6 +241,105 @@ def test_block_comment_spanning_feed_boundary_is_not_terminated_early assert_equal [[1], [3]], values end + def test_trailing_comma_split_across_feed_boundary + # With allow_trailing_comma the closing bracket may arrive in a later chunk + # than the comma; consuming the comma must not lose the ability to close. + parser = new_parser(allow_trailing_comma: true) + parser << '[1,' + refute parser.parse + parser << ']' + assert parser.parse + assert_equal [1], parser.value + + parser = new_parser(allow_trailing_comma: true) + parser << '{"a":1,' + refute parser.parse + parser << '}' + assert parser.parse + assert_equal({ "a" => 1 }, parser.value) + + # The boundary can also fall after an inner comma, then after the outer one. + parser = new_parser(allow_trailing_comma: true) + parser << '[[1,' + refute parser.parse + parser << '],]' + assert parser.parse + assert_equal [[1]], parser.value + end + + def test_trailing_comma_byte_by_byte + parser = new_parser(allow_trailing_comma: true) + '[1, 2, ]'.each_char { |c| parser << c; parser.parse } + assert_equal [1, 2], parser.value + + parser = new_parser(allow_trailing_comma: true) + '{ "a": 1, }'.each_char { |c| parser << c; parser.parse } + assert_equal({ "a" => 1 }, parser.value) + end + + def test_comment_after_comma_split_across_feed_boundary + # A comment right after a ',' straddling a feed boundary must not drop the + # comma: the value/key it separates must still be parsed on resume. + # The array case needs allow_trailing_comma: without it the array comma path + # commits its phase before eating the comment, so only the trailing-comma + # path exercises the eat-before-commit bug (the object path always did). + parser = new_parser(allow_comments: true, allow_trailing_comma: true) + parser << '[1,/*' + refute parser.parse + parser << '*/2]' + assert parser.parse + assert_equal [1, 2], parser.value + + parser = new_parser(allow_comments: true) + parser << '{"a":1,/*' + refute parser.parse + parser << '*/"b":2}' + assert parser.parse + assert_equal({ "a" => 1, "b" => 2 }, parser.value) + end + + def test_comment_after_container_open_split_across_feed_boundary + # A comment right after '[' or '{' straddling a feed boundary must not drop + # the opening token: it is consumed before its frame is pushed, so the + # suspension must resume from the bracket, not from inside the comment. + parser = new_parser(allow_comments: true) + parser << '[/*' + refute parser.parse + parser << '*/1]' + assert parser.parse + assert_equal [1], parser.value + + parser = new_parser(allow_comments: true) + parser << '{/*' + refute parser.parse + parser << '*/"a":1}' + assert parser.parse + assert_equal({ "a" => 1 }, parser.value) + + parser = new_parser(allow_comments: true) + parser << '[ /*' + refute parser.parse + parser << '*/ ]' + assert parser.parse + assert_equal [], parser.value + + # The boundary can even split the comment marker itself. + parser = new_parser(allow_comments: true) + parser << '[/' + refute parser.parse + parser << '**/1]' + assert parser.parse + assert_equal [1], parser.value + + # Line comments suspend the same way when their newline hasn't arrived. + parser = new_parser(allow_comments: true) + parser << '[//' + refute parser.parse + parser << "x\n1]" + assert parser.parse + assert_equal [1], parser.value + end + def test_rest @parser << '[1, 2, 3, "unterminated string' refute @parser.parse From 15af5bb0f3ecdc5e978169155ec6aa7ae85c268c Mon Sep 17 00:00:00 2001 From: Burdette Lamar Date: Thu, 9 Jul 2026 01:46:59 -0500 Subject: [PATCH 2/2] [DOC] Doc for Pathname#realpath and Pathname#realdirpath --- pathname_builtin.rb | 53 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/pathname_builtin.rb b/pathname_builtin.rb index f2dbee5a7cf7df..9b31b73b87af43 100644 --- a/pathname_builtin.rb +++ b/pathname_builtin.rb @@ -1716,18 +1716,59 @@ def split() array.map {|f| self.class.new(f) } end - # Returns the real (absolute) pathname for +self+ in the actual filesystem. + # :markup: markdown + # + # call-seq: + # realpath -> new_pathname # - # Does not contain symlinks or useless dots, +..+ and +.+. + # Returns a new pathname containing the real (absolute) pathname + # of the path in `self`; + # the new path is the path in the actual filesystem, + # and does not contain useless dot-entries (`'.'` or `'..'`) + # or symbolic links: + # + # ```ruby + # Pathname('/etc/./passwd/../../var').realpath + # # => # + # ``` # - # All components of the pathname must exist when this method is called. + # All components of the new path must exist: + # + # ```ruby + # Pathname('/etc/./passwd/../../var/nosuch').realpath + # # Raises Errno::ENOENT: No such file or directory. + # ``` + # + # \Method #realdirpath is similar, but does not require the last component to exist. def realpath(...) self.class.new(File.realpath(@path, ...)) end - # Returns the real (absolute) pathname of +self+ in the actual filesystem. + + # :markup: markdown + # + # call-seq: + # realdirpath -> new_pathname + # + # Returns a new pathname containing the real (absolute) pathname + # of the path in `self`; + # the new path is the path in the actual filesystem, + # and does not contain useless dot-entries (`'.'` or `'..'`) + # or symbolic links: + # + # ```ruby + # Pathname('/etc/./passwd/../../var').realdirpath + # # => # + # ``` + # + # Only the last component of the new path may be nonexistent: # - # Does not contain symlinks or useless dots, +..+ and +.+. + # ```ruby + # Pathname('/etc/./passwd/../../var/nosuch').realdirpath + # # => # + # Pathname('/etc/./passwd/../../var/nosuch/nosuch').realdirpath + # # Raises Errno::ENOENT: No such file or directory. + # ``` # - # The last component of the real pathname can be nonexistent. + # \Method #realpath is similar, but requires all components to exist. def realdirpath(...) self.class.new(File.realdirpath(@path, ...)) end end