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