Skip to content

Commit 9dbfeb8

Browse files
pockeclaude
authored andcommitted
Fix ResumableParser treating unterminated line comments as 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
1 parent ea008e8 commit 9dbfeb8

3 files changed

Lines changed: 53 additions & 3 deletions

File tree

‎ext/json/ext/parser/parser.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,11 +779,18 @@ json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config)
779779

780780
switch (peek(state)) {
781781
case '/': {
782-
state->cursor = memchr(state->cursor, '\n', state->end - state->cursor);
783-
if (!state->cursor) {
782+
const char *newline = memchr(state->cursor, '\n', state->end - state->cursor);
783+
if (!newline) {
784+
// state->parser marks resumable mode, where the buffer end is only a
785+
// chunk boundary: the terminating newline may still arrive, so leave
786+
// the comment unterminated instead of consuming to end as a one-shot
787+
// parse would.
788+
if (state->parser) {
789+
raise_eos_error_at("unterminated comment, expected end of line", state, start);
790+
}
784791
state->cursor = state->end;
785792
} else {
786-
state->cursor++;
793+
state->cursor = newline + 1;
787794
}
788795
break;
789796
}

‎test/json/json_parser_test.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,7 @@ def test_parse_comments
526526
JSON
527527
assert_equal({ "key1" => "value1" }, parse(json, allow_comments: true))
528528
assert_equal({}, parse('{} /**/', allow_comments: true))
529+
assert_equal({}, parse('{} // eol comment ending at eof', allow_comments: true))
529530
assert_raise(ParserError) { parse('{} /* comment not closed', allow_comments: true) }
530531
assert_raise(ParserError) { parse('{} /*/', allow_comments: true) }
531532
assert_raise(ParserError) { parse('{} /x wrong comment', allow_comments: true) }

‎test/json/resumable_parser_test.rb‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,48 @@ def test_incomplete_input_at_structural_positions_resumes
199199
assert_incomplete "{\"a\":1,"
200200
end
201201

202+
def test_line_comment_spanning_feed_boundary_is_not_terminated_early
203+
# A `//` line comment is only terminated by a newline. When the newline
204+
# has not arrived yet, the comment must stay incomplete rather than being
205+
# treated as consumed -- otherwise its body, delivered in a later chunk,
206+
# leaks out as parsed values.
207+
values = []
208+
parser = new_parser(allow_comments: true)
209+
parser << '[1] //'
210+
values << parser.value while parser.parse
211+
212+
parser << "[2]\n[3]" # [2] belongs to the comment, [3] is a real document
213+
values << parser.value while parser.parse
214+
215+
assert_equal [[1], [3]], values
216+
end
217+
218+
def test_line_comment_terminated_by_newline_across_feeds
219+
values = []
220+
parser = new_parser(allow_comments: true)
221+
parser << '[1] //co'
222+
values << parser.value while parser.parse
223+
224+
parser << "mment\n[2]"
225+
values << parser.value while parser.parse
226+
227+
assert_equal [[1], [2]], values
228+
end
229+
230+
def test_block_comment_spanning_feed_boundary_is_not_terminated_early
231+
# A `/* */` block comment whose closing `*/` has not arrived yet must stay
232+
# incomplete, mirroring the line-comment behaviour above.
233+
values = []
234+
parser = new_parser(allow_comments: true)
235+
parser << '[1] /*'
236+
values << parser.value while parser.parse
237+
238+
parser << '[2]*/[3]' # [2] belongs to the comment, [3] is a real document
239+
values << parser.value while parser.parse
240+
241+
assert_equal [[1], [3]], values
242+
end
243+
202244
def test_rest
203245
@parser << '[1, 2, 3, "unterminated string'
204246
refute @parser.parse

0 commit comments

Comments
 (0)