Skip to content

Commit 9b82f79

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 4095 is a multi-byte character. In the case, `parse_stream_fgets()` writes 4097 or more data to the buffer. We can avoid it by increasing internal buffer size to `4096 + 6` where `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 ab9412c commit 9b82f79

2 files changed

Lines changed: 52 additions & 14 deletions

File tree

src/source.c

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -365,28 +365,50 @@ bool
365365
pm_source_stream_read(pm_source_t *source) {
366366
pm_buffer_t *buffer = source->stream.buffer;
367367

368+
/*
369+
* We should use rb_enc_mbmaxlen() of stream.external_encoding for
370+
* the max character length but we don't want to do it here for
371+
* performance. We can detect the max character length in all
372+
* available encodings by the following command line in the Ruby
373+
* source directory:
374+
*
375+
* grep -Erh "max (enc|byte) length" enc |
376+
* grep -Eo '[0-9]+' |
377+
* sort |
378+
* tail -n1 # => 6
379+
*/
380+
#define MAX_ENC_LEN 6
381+
368382
#define LINE_SIZE 4096
369-
char line[LINE_SIZE];
370383

371-
while (memset(line, '\n', LINE_SIZE), source->stream.fgets(line, LINE_SIZE, source->stream.stream) != NULL) {
372-
size_t length = LINE_SIZE;
373-
while (length > 0 && line[length - 1] == '\n') length--;
384+
/*
385+
* io.gets(size) may read size or more bytes when the last
386+
* character is a multi-byte character. "+ MAX_ENC_LEN" is for the
387+
* case.
388+
*/
389+
char line[LINE_SIZE + MAX_ENC_LEN];
390+
391+
/* This must not be '\0' to detect the `\0` written by fgets(). */
392+
#define SENTINEL_CHAR 'x'
393+
394+
while (memset(line, SENTINEL_CHAR, LINE_SIZE + MAX_ENC_LEN), source->stream.fgets(line, LINE_SIZE, source->stream.stream) != NULL) {
395+
size_t length = LINE_SIZE + MAX_ENC_LEN;
396+
while (length > 0 && line[length - 1] != '\0') length--;
374397

375-
if (length == LINE_SIZE) {
398+
/* Remove '\0' written by fgets() */
399+
length -= 1;
400+
401+
/* Append the line to the buffer. */
402+
pm_buffer_append_string(buffer, line, length);
403+
404+
if (line[length - 1] != '\n') {
376405
/*
377-
* If we read a line that is the maximum size and it doesn't end
378-
* with a newline, then we'll just append it to the buffer and
379-
* continue reading.
406+
* If it doesn't end with a newline, then we'll continue
407+
* reading.
380408
*/
381-
length--;
382-
pm_buffer_append_string(buffer, line, length);
383409
continue;
384410
}
385411

386-
/* Append the line to the buffer. */
387-
length--;
388-
pm_buffer_append_string(buffer, line, length);
389-
390412
/*
391413
* Check if the line matches the __END__ marker. If it does, then stop
392414
* reading and return false. In most circumstances, this means we should
@@ -426,7 +448,9 @@ pm_source_stream_read(pm_source_t *source) {
426448
}
427449
}
428450

451+
#undef SENTINEL_CHAR
429452
#undef LINE_SIZE
453+
#undef MAX_ENC_LEN
430454

431455
source->stream.eof = true;
432456
source->source = (const uint8_t *) pm_buffer_value(buffer);

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" * 4093) + "\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)