[pull] master from ruby:master#1195
Merged
Merged
Conversation
…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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpeQSEFkF1X1Uuzjx9BsYe
ruby/json@cc010c4688
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )