[pull] master from ruby:master#1177
Merged
Merged
Conversation
Co-authored-by: Jeremy Evans <code@jeremyevans.net>
…s complete A `//` line comment split across a feed boundary was treated as fully consumed, so comment body delivered in a later chunk leaked out as parsed JSON values. ## Reproduction ```ruby require "json" parser = JSON::ResumableParser.new(allow_comments: true) documents = [] "[1]//[999]\n[3]".each_char do |char| parser << char documents << parser.value while parser.parse end p documents ``` ## Expected Behavior ``` [[1], [3]] ``` The `[999]` is inside the `//` comment (which runs until the newline), so it is ignored, leaving the two documents `[1]` and `[3]`. This is what feeding the whole string in a single chunk produces. ## Actual Behavior ``` [[1], [999], [3]] ``` The commented-out `[999]` leaked out as a real value. When the comment body does not form valid JSON the split feed raises a ParserError instead. This happens with the default configuration too, since comments are accepted (with a deprecation warning) unless disabled. ## Description A `//` line comment is only terminated by a newline. `json_eat_comments` searched for that newline and, when none was present in the buffer, moved the cursor to the end of the buffer and returned as if the comment had been fully consumed. For the one-shot `JSON.parse` this is correct: the buffer holds the whole document, so a `//` comment with no trailing newline genuinely runs to the end of input. For `ResumableParser`, however, the end of the buffer is only a chunk boundary, not the end of input. Reaching it does not mean the comment is over -- the newline (and possibly more comment body) may still arrive in a later `<<`. Swallowing the buffered bytes as a finished comment loses the "still inside a comment" state, so any comment body delivered in the next chunk was parsed as JSON instead of being ignored. Because the result then depended on where the input happened to be split, the same byte stream could produce different values. The block-comment branch already handles this correctly: when it cannot find the closing `*/` it rewinds to the comment start and raises an EOS-tagged error, which `ResumableParser#parse` swallows into a `false` return so the comment is retried once more input is available. This change makes the line-comment branch behave the same way, but only in resumable mode (detected via `state->parser`, the same discriminator `raise_parse_error` already uses). In non-resumable mode the previous behaviour is preserved: a `//` comment with no trailing newline is still consumed to the end of input. As a consequence, a stream that ends with an unterminated line comment now stays incomplete (`parse` keeps returning `false`) until a newline arrives, which mirrors how a trailing bare number is only considered complete once a following separator is seen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpeQSEFkF1X1Uuzjx9BsYe ruby/json@9dbfeb83df
There was no single API answering "does the stream end in the middle of
a document?" once all parseable fed bytes have been consumed. Callers
had to combine two complementary APIs:
!parser.rest.empty? || !parser.partial_value.nil?
`rest` only reflects unconsumed tokenizer bytes, so it is empty when the
stream is truncated exactly on a token boundary (right after a ':' or
','), while `partial_value` is nil when truncation happens mid-token
before any container is registered. Neither alone covers all shapes,
and `partial_value` materializes the partially built Ruby objects just
to test for nil.
`partial_value?` answers the same question as `!partial_value.nil?` by
looking at the parser's internal value stack directly, without building
the partial Ruby object graph.
`empty?` is strict: true only when the buffer is fully consumed, no
document is under construction and no parsed value awaits retrieval
with `value`. It is defined in Ruby as the composition of the three
underlying predicates so its definition doubles as documentation:
def empty?
eos? && !partial_value? && !value?
end
ruby/json@0864e83701
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
4 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 : )