Skip to content

Commit ff6095a

Browse files
committed
Fix a bug that Prism.parse_stream may write too much data to internal buffer
`pm_source_stream_read()` has 4096 bytes internal buffer to read a line: https://github.com/ruby/prism/blob/39fb41f06e5f4ccbe1783122fa8a4470e09dfcfe/src/source.c#L368-L369 It uses `gets(4095)` to read the next chunk to the buffer: https://github.com/ruby/prism/blob/39fb41f06e5f4ccbe1783122fa8a4470e09dfcfe/ext/prism/extension.c#L1076 But `gets(4095)` may read 4095 or more data when the character at 4094 is a multi-byte character. In the case, `parse_stream_fgets()` writes 4097 or more data to the buffer. We can avoid it by using `gets(4095 - 6)` not `gets(4095)`. `6` is the max character length in all available encodings. We can reduce it by using `stream.external_encoding` and `rb_enc_mbmaxlen()` but it may slow down. So I used constant `6` in this change.
1 parent 39fb41f commit ff6095a

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

ext/prism/extension.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,9 +1071,21 @@ parse_stream_eof(void *stream) {
10711071
*/
10721072
static char *
10731073
parse_stream_fgets(char *string, int size, void *stream) {
1074-
RUBY_ASSERT(size > 0);
1074+
/* We should use rb_enc_mbmaxlen() of stream.external_encoding but
1075+
* we don't want to do it here for performance. We can detect the
1076+
* max character length in all available encodings by the
1077+
* following command line in the Ruby source directory:
1078+
*
1079+
* grep -Eh "max (enc|byte) length" enc/*.c |
1080+
* grep -Eo '[0-9]+' |
1081+
* sort |
1082+
* tail -n1 # => 6
1083+
*/
1084+
#define MAX_ENC_LEN 6
1085+
RUBY_ASSERT(size > MAX_ENC_LEN);
10751086

1076-
VALUE line = rb_funcall((VALUE) stream, rb_intern("gets"), 1, INT2FIX(size - 1));
1087+
VALUE line = rb_funcall((VALUE) stream, rb_intern("gets"), 1, INT2FIX(size - MAX_ENC_LEN - 1));
1088+
#undef MAX_ENC_LEN
10771089
if (NIL_P(line)) {
10781090
return NULL;
10791091
}

test/prism/api/parse_stream_test.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,19 @@ def test_nul_bytes
114114
assert result.success?
115115
assert_equal 3, result.value.statements.body.length
116116
end
117+
118+
def test_long_line
119+
# The important point of this test is that the character at 4095
120+
# is a multi-byte character (U+3042 HIRAGANA LETTER A) in this
121+
# case. gets(4095) may return 4095 or more bytes if the last
122+
# character is a multi-byte character. For example,
123+
# StringIO.new("\u3042").gets(1).bytesize is 3 not 1.
124+
long_string = "\"" + ("a" * 4094) + "\u3042" + "\""
125+
io = StringIO.new(long_string)
126+
result = Prism.parse_stream(io)
127+
128+
assert result.success?
129+
assert_equal long_string[1..-2], result.value.statements.body[0].content
130+
end
117131
end
118132
end

0 commit comments

Comments
 (0)