From 63890312d79947f7e083c77e14d33e2d6b771f55 Mon Sep 17 00:00:00 2001 From: Masataka Pocke Kuwabara Date: Thu, 2 Jul 2026 18:52:53 +0900 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_01PpeQSEFkF1X1Uuzjx9BsYe --- ext/json/ext/parser/parser.c | 13 ++++++--- test/json/json_parser_test.rb | 1 + test/json/resumable_parser_test.rb | 42 ++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/ext/json/ext/parser/parser.c b/ext/json/ext/parser/parser.c index a60f6a3c..a4c7dc16 100644 --- a/ext/json/ext/parser/parser.c +++ b/ext/json/ext/parser/parser.c @@ -779,11 +779,18 @@ json_eat_comments(JSON_ParserState *state, JSON_ParserConfig *config) switch (peek(state)) { case '/': { - state->cursor = memchr(state->cursor, '\n', state->end - state->cursor); - if (!state->cursor) { + const char *newline = memchr(state->cursor, '\n', state->end - state->cursor); + if (!newline) { + // state->parser marks resumable mode, where the buffer end is only a + // chunk boundary: the terminating newline may still arrive, so leave + // 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); + } state->cursor = state->end; } else { - state->cursor++; + state->cursor = newline + 1; } break; } diff --git a/test/json/json_parser_test.rb b/test/json/json_parser_test.rb index 9000d115..2f79b87c 100644 --- a/test/json/json_parser_test.rb +++ b/test/json/json_parser_test.rb @@ -526,6 +526,7 @@ def test_parse_comments JSON assert_equal({ "key1" => "value1" }, parse(json, allow_comments: true)) assert_equal({}, parse('{} /**/', allow_comments: true)) + assert_equal({}, parse('{} // eol comment ending at eof', allow_comments: true)) assert_raise(ParserError) { parse('{} /* comment not closed', allow_comments: true) } assert_raise(ParserError) { parse('{} /*/', allow_comments: true) } assert_raise(ParserError) { parse('{} /x wrong comment', allow_comments: true) } diff --git a/test/json/resumable_parser_test.rb b/test/json/resumable_parser_test.rb index 734d6e22..ee8e2f8d 100644 --- a/test/json/resumable_parser_test.rb +++ b/test/json/resumable_parser_test.rb @@ -199,6 +199,48 @@ def test_incomplete_input_at_structural_positions_resumes assert_incomplete "{\"a\":1," end + def test_line_comment_spanning_feed_boundary_is_not_terminated_early + # A `//` line comment is only terminated by a newline. When the newline + # has not arrived yet, the comment must stay incomplete rather than being + # treated as consumed -- otherwise its body, delivered in a later chunk, + # leaks out as parsed values. + values = [] + parser = new_parser(allow_comments: true) + parser << '[1] //' + values << parser.value while parser.parse + + parser << "[2]\n[3]" # [2] belongs to the comment, [3] is a real document + values << parser.value while parser.parse + + assert_equal [[1], [3]], values + end + + def test_line_comment_terminated_by_newline_across_feeds + values = [] + parser = new_parser(allow_comments: true) + parser << '[1] //co' + values << parser.value while parser.parse + + parser << "mment\n[2]" + values << parser.value while parser.parse + + assert_equal [[1], [2]], values + end + + def test_block_comment_spanning_feed_boundary_is_not_terminated_early + # A `/* */` block comment whose closing `*/` has not arrived yet must stay + # incomplete, mirroring the line-comment behaviour above. + values = [] + parser = new_parser(allow_comments: true) + parser << '[1] /*' + values << parser.value while parser.parse + + parser << '[2]*/[3]' # [2] belongs to the comment, [3] is a real document + values << parser.value while parser.parse + + assert_equal [[1], [3]], values + end + def test_rest @parser << '[1, 2, 3, "unterminated string' refute @parser.parse