Fix a bug that Prism.parse_stream may write too much data to internal buffer

[kou]

pm_source_stream_read() has 4096 bytes internal buffer to read a line:

prism/src/source.c

Lines 368 to 369 in 39fb41f

	#define LINE_SIZE 4096
	char line[LINE_SIZE];

It uses gets(4095) to read the next chunk to the buffer:

prism/ext/prism/extension.c

Line 1076 in 39fb41f

VALUE line = rb_funcall((VALUE) stream, rb_intern("gets"), 1, INT2FIX(size - 1));

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.

This also changes pm_source_stream_fgets_t's return type to int (read bytes) from char * (read data) to reduce needless length check by caller.

[Copilot]

Pull request overview

This PR fixes a potential buffer overflow when Prism.parse_stream reads from a Ruby IO via gets(limit), where gets may exceed the byte limit to avoid splitting a multi-byte character.

Changes:

• Reduce the gets limit used by parse_stream_fgets to leave headroom for multi-byte characters.
• Add a regression test that exercises a long line with a multi-byte character at the boundary.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
test/prism/api/parse_stream_test.rb	Adds a test covering long-line streaming with a multi-byte boundary case.
ext/prism/extension.c	Adjusts the gets limit to avoid overfilling the fixed-size internal line buffer.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Earlopain]

There is this condition

prism/src/source.c

Lines 375 to 384 in 39fb41f

	if (length == LINE_SIZE) {
	/*
	* If we read a line that is the maximum size and it doesn't end
	* with a newline, then we'll just append it to the buffer and
	* continue reading.
	*/
	length--;
	pm_buffer_append_string(buffer, line, length);
	continue;
	}

which will now basically never be reached.

[Earlopain]

../../../../ext/prism/extension.c:1079:46: warning: ‘/*’ within comment [-Wcomment]

[kou]

Good point.

I've changed the "reduce gets size in fgets callbacks" approach to the "increase the internal buffer size in pm_source_stream_read()" approach. I also changed the optimization condition to "whether the last character is \n or not" that also works when all of the internal buffer isn't used.

[kou]

We can keep using \n for the sentinel character but I've changed to another character for easy to understand. If you don't like it, I'll revert it.

[kou]

We can keep using == SENTINEL_CHAR not != '\0' here. I think that != '\0' is easier to understand but I'll revert this if you don't like it.