From 49d5b5fa5d7ba67a6924ce3d042c129e40ae5825 Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 3 Aug 2026 17:15:02 +0900 Subject: [PATCH] Reject a byte position the lexer cannot start on `rbs_lexer_new` reaches `start_pos` by stepping one character at a time from the beginning of the buffer, so only the first byte of a character is a position it can stand on. Inside a character the step goes over `start_pos` and lexing quietly begins at the next one. Nothing downstream can tell: the walk keeps `line`, `column` and `char_pos` consistent, so the result points at a real position that simply is not the one asked for. The shift is caught by accident when what it lands on cannot open a token, but an ASCII letter right after the multibyte character parses cleanly in the wrong place -- and ruby/rbs#3082 gives the non-ASCII characters tokens of their own, taking even the accident away. Past the end of the buffer there is nothing left to step over, so the walk never finishes: `rbs_skip` stops moving at EOF while the loop waits for a position it will never reach. `parse_type("Integer", byte_range: 20...30)` hangs on master. `rbs_lexer_new` returns `NULL` for a `start_pos` it cannot reach, and the extension turns that into an `ArgumentError` alongside the reversed and negative ranges, since all are the caller's mistake rather than anything about the source text. Starting past the end is plain from the buffer's size, so the extension rejects it before parsing; a `NULL` that comes back after that check can only mean a start inside a character. `end_pos` keeps taking any value: clamping with a large number instead of measuring the buffer is ordinary, and the lexer stops at the end on its own. --- ext/rbs_extension/main.c | 34 ++++++++++++++++++++----- include/rbs/parser.h | 11 ++++++-- src/parser.c | 19 +++++++++++--- test/rbs/type_parsing_test.rb | 48 +++++++++++++++++++++++++++++++++-- 4 files changed, 98 insertions(+), 14 deletions(-) diff --git a/ext/rbs_extension/main.c b/ext/rbs_extension/main.c index ef845fc822..06434640ce 100644 --- a/ext/rbs_extension/main.c +++ b/ext/rbs_extension/main.c @@ -146,44 +146,66 @@ static VALUE parse_type_try(VALUE a) { return rbs_struct_to_ruby_value(ctx, type); } -static void validate_position_range(int start_pos, int end_pos) { +/** + * `end_pos` may point past the end of the buffer: clamping with a large + * number instead of measuring the buffer is ordinary, and the lexer stops at + * the end on its own. + * */ +static void validate_position_range(VALUE string, int start_pos, int end_pos) { if (start_pos < 0 || end_pos < 0) { rb_raise(rb_eArgError, "negative position range: %d...%d", start_pos, end_pos); } if (start_pos > end_pos) { rb_raise(rb_eArgError, "invalid position range: %d...%d", start_pos, end_pos); } + + long size = RSTRING_LEN(string); + if ((long) start_pos > size) { + rb_raise(rb_eArgError, "position range starts past the end of the buffer: %d...%d, buffer is %ld bytes", start_pos, end_pos, size); + } } static rbs_lexer_t *alloc_lexer_from_buffer(rbs_allocator_t *allocator, VALUE string, rb_encoding *encoding, int start_pos, int end_pos) { - validate_position_range(start_pos, end_pos); + validate_position_range(string, start_pos, end_pos); const char *encoding_name = rb_enc_name(encoding); - return rbs_lexer_new( + rbs_lexer_t *lexer = rbs_lexer_new( allocator, rbs_string_from_ruby_string(string), rbs_encoding_find((const uint8_t *) encoding_name, (const uint8_t *) (encoding_name + strlen(encoding_name))), start_pos, end_pos ); + + if (lexer == NULL) { + rb_raise(rb_eArgError, "position range starts inside a character: %d...%d", start_pos, end_pos); + } + + return lexer; } static rbs_parser_t *alloc_parser_from_buffer(VALUE buffer, int start_pos, int end_pos) { - validate_position_range(start_pos, end_pos); - VALUE string = rb_funcall(buffer, rb_intern("content"), 0); StringValue(string); + validate_position_range(string, start_pos, end_pos); + rb_encoding *encoding = rb_enc_get(string); const char *encoding_name = rb_enc_name(encoding); - return rbs_parser_new( + rbs_parser_t *parser = rbs_parser_new( rbs_string_from_ruby_string(string), rbs_encoding_find((const uint8_t *) encoding_name, (const uint8_t *) (encoding_name + strlen(encoding_name))), start_pos, end_pos ); + + if (parser == NULL) { + rb_raise(rb_eArgError, "position range starts inside a character: %d...%d", start_pos, end_pos); + } + + return parser; } static VALUE rbsparser_parse_type(VALUE self, VALUE buffer, VALUE start_pos, VALUE end_pos, VALUE variables, VALUE require_eof, VALUE void_allowed, VALUE self_allowed, VALUE classish_allowed) { diff --git a/include/rbs/parser.h b/include/rbs/parser.h index 598a76aff3..2ae53922be 100644 --- a/include/rbs/parser.h +++ b/include/rbs/parser.h @@ -89,8 +89,13 @@ RBS_NODISCARD bool rbs_parser_insert_typevar(rbs_parser_t *parser, rbs_constant_ * VALUE string = rb_funcall(buffer, rb_intern("content"), 0); * rbs_lexer_new(string, 0, 31) // New rbs_lexer_t with buffer content * ``` + * + * Returns `NULL` when `start_pos` is not the first byte of a character in + * `string` -- inside one, or past the end of the buffer. The lexer reaches + * `start_pos` a character at a time, so nowhere else is a position it can + * start from. * */ -rbs_lexer_t *rbs_lexer_new(rbs_allocator_t *, rbs_string_t string, const rbs_encoding_t *encoding, int start_pos, int end_pos); +RBS_NODISCARD rbs_lexer_t *rbs_lexer_new(rbs_allocator_t *, rbs_string_t string, const rbs_encoding_t *encoding, int start_pos, int end_pos); /** * Allocate new rbs_parser_t object. @@ -98,8 +103,10 @@ rbs_lexer_t *rbs_lexer_new(rbs_allocator_t *, rbs_string_t string, const rbs_enc * ``` * rbs_parser_new(buffer, string, encoding, 0, 1); * ``` + * + * Returns `NULL` for a `start_pos` that `rbs_lexer_new` rejects. * */ -rbs_parser_t *rbs_parser_new(rbs_string_t string, const rbs_encoding_t *encoding, int start_pos, int end_pos); +RBS_NODISCARD rbs_parser_t *rbs_parser_new(rbs_string_t string, const rbs_encoding_t *encoding, int start_pos, int end_pos); void rbs_parser_free(rbs_parser_t *parser); /** diff --git a/src/parser.c b/src/parser.c index 17212b3784..7be16b9f06 100644 --- a/src/parser.c +++ b/src/parser.c @@ -3539,12 +3539,18 @@ rbs_lexer_t *rbs_lexer_new(rbs_allocator_t *allocator, rbs_string_t string, cons lexer->current_character_bytes = 1; } - if (start_pos > 0) { - while (lexer->current.byte_pos < start_pos) { - rbs_skip(lexer); - } + // `rbs_skip` moves a whole character at a time, and moves nothing at all + // once the input is spent, so this walk can only ever stand on the first + // byte of a character. + while (lexer->current.byte_pos < start_pos && lexer->current_code_point != '\0') { + rbs_skip(lexer); } + // Stopping anywhere else means `start_pos` is a position the lexer cannot + // start from: over it, and the walk stepped across a character that + // straddles it; short of it, and the input ran out first. + if (lexer->current.byte_pos != start_pos) return NULL; + lexer->start = lexer->current; return lexer; @@ -3554,6 +3560,11 @@ rbs_parser_t *rbs_parser_new(rbs_string_t string, const rbs_encoding_t *encoding rbs_allocator_t *allocator = rbs_allocator_init(); rbs_lexer_t *lexer = rbs_lexer_new(allocator, string, encoding, start_pos, end_pos); + if (lexer == NULL) { + rbs_allocator_free(allocator); + return NULL; + } + rbs_parser_t *parser = rbs_allocator_alloc(allocator, rbs_parser_t); *parser = (rbs_parser_t) { diff --git a/test/rbs/type_parsing_test.rb b/test/rbs/type_parsing_test.rb index 97c5754b95..d4da7c338d 100644 --- a/test/rbs/type_parsing_test.rb +++ b/test/rbs/type_parsing_test.rb @@ -1019,14 +1019,58 @@ def test_parse__range_works end def test_parse__byte_range_incorrect - # We want a better error handling ergonomics, but currently simply raises a syntax error. + # `"🐕🐈"` is 10 bytes: `"` then two four-byte characters then `"`. Bytes 2 + # to 4 are inside the first of them, and the lexer can only ever stop on a + # character boundary, so there is no honest answer to give. input = '"🐕🐈"' + (2..4).each do |start| + exn = assert_raises ArgumentError do + Parser.parse_type(input, byte_range: start...) + end + + assert_equal "position range starts inside a character: #{start}...10", exn.message + end + + # Byte 5 opens `🐈`, so lexing starts exactly where it was asked to. This + # is the error the three positions above used to be silently rounded up + # to. exn = assert_raises RBS::ParsingError do - Parser.parse_type(input, byte_range: 2...) + Parser.parse_type(input, byte_range: 5...) end assert_equal "a.rbs:1:2...1:3: Syntax error: unexpected token for simple type, token=`🐈` (ErrorToken)", exn.message end + + def test_parse__byte_range_starting_past_the_end + # Reaching `start_pos` means stepping to it, and there is nothing left to + # step over here, so the lexer used to walk forever. + exn = assert_raises ArgumentError do + Parser.parse_type("Integer", byte_range: 20...30) + end + + assert_equal "position range starts past the end of the buffer: 20...30, buffer is 7 bytes", exn.message + + # The end of the buffer itself is a boundary like any other, and an + # `end_pos` past it is how a caller clamps without measuring. + assert_nil Parser.parse_type("Integer", byte_range: 7...7) + assert_equal "Integer", Parser.parse_type("Integer", byte_range: 0...9999).to_s + end + + def test_parse__byte_range_incorrect_in_euc_jp + # In EUC-JP the second byte of a character sits in the same range as a + # first byte, so decoding at the offset would happily report a character. + # Only the walk from the start of the buffer knows byte 1 is inside one. + euc = (+"\xC6\xFCFoo").force_encoding(Encoding::EUC_JP) # 日Foo + + exn = assert_raises ArgumentError do + Parser.parse_type(euc, byte_range: 1...) + end + + assert_equal "position range starts inside a character: 1...5", exn.message + + assert_equal RBS::TypeName.parse("Foo"), + Parser.parse_type(euc, byte_range: 2...).name + end end