From 84ef3aba09c37a6343b5a09b80e83b82132af35d Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 18:37:27 +0900 Subject: [PATCH 1/3] Fix readuntil missing a terminator that spans two reads BufferedIO#readuntil resumed searching at the previous buffer end, so a multi-byte terminator split across two fills, such as "\r" ending one chunk and "\n" starting the next, was never matched and readuntil returned data past the terminator. Rewind the search by terminator.bytesize - 1 before refilling, floored at @rbuf_offset. Co-Authored-By: Claude Fable 5 --- lib/net/protocol.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index 8c81298..903ea35 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -209,7 +209,14 @@ def readuntil(terminator, ignore_eof = false) offset = @rbuf_offset begin until idx = @rbuf.index(terminator, offset) - offset = @rbuf.bytesize + # Rewind by terminator.bytesize - 1 so that a terminator split + # across reads is not missed, however many reads it spans. + # @rbuf_offset is the floor for two reasons. A negative offset + # makes String#index search relative to the end of the buffer, + # skipping a match near its start. An offset below @rbuf_offset + # matches a terminator beginning inside bytes already returned + # to the caller, yielding a slice that does not end with one. + offset = [@rbuf.bytesize - terminator.bytesize + 1, @rbuf_offset].max rbuf_fill end return rbuf_consume(idx + terminator.bytesize - @rbuf_offset) From 215cc3566663fab49520cbea03f094cf07fa5cc7 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 18:37:28 +0900 Subject: [PATCH 2/3] Add regression tests for the readuntil rewind Cover a terminator split across two reads and across more than two, the negative rewind the floor clamps, the floor keeping the search out of consumed bytes, and the ignore_eof branch that returns data without a terminator. FakeReadPartialIO now signals EOF instead of raising TypeError, and hands out binary chunks so @rbuf stays binary the way a real IO leaves it. Co-Authored-By: Claude Fable 5 --- test/net/protocol/test_protocol.rb | 48 ++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 2f42fa3..0693ad6 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -130,15 +130,20 @@ def test_write0_timeout_multi2 class FakeReadPartialIO def initialize(chunks) - @chunks = chunks.map(&:dup) + # Binary, like the bytes a real IO hands back. String#b also copies, + # which matters because rbuf_fill clears a string read_nonblock + # returns without having been handed it as the buffer. + @chunks = chunks.map(&:b) end def read_nonblock(size, buf = nil, exception: false) + chunk = @chunks.shift + return nil if chunk.nil? if buf - buf.replace(@chunks.shift) + buf.replace(chunk) buf else - @chunks.shift + chunk end end end @@ -156,4 +161,41 @@ def test_shareable_buffer_leak # https://github.com/ruby/net-protocol/pull/19 io.read(5, reader) assert_equal expected_chunks, actual_chunks end + + def test_readuntil_terminator_spanning_chunks # https://github.com/ruby/net-protocol/pull/66 + fake_io = FakeReadPartialIO.new(["abc\r", "\ndef\r\n"]) + io = Net::BufferedIO.new(fake_io) + assert_equal "abc\r\n", io.readuntil("\r\n") + assert_equal "def\r\n", io.readuntil("\r\n") + end + + def test_readuntil_terminator_spanning_more_than_two_chunks # https://github.com/ruby/net-protocol/pull/66 + fake_io = FakeReadPartialIO.new(["a", "\r", "\n", "\r", "\n"]) + io = Net::BufferedIO.new(fake_io) + assert_equal "a\r\n\r\n", io.readuntil("\r\n\r\n") + end + + def test_readuntil_clamps_a_negative_rewind # https://github.com/ruby/net-protocol/pull/66 + fake_io = FakeReadPartialIO.new(["ab\n"]) + io = Net::BufferedIO.new(fake_io) + # Any buffer shorter than the terminator drives the rewind below zero, + # and String#index reads a negative offset as counting from the end. + assert_equal "ab", io.readuntil("ab") + end + + def test_readuntil_does_not_rewind_into_consumed_bytes # https://github.com/ruby/net-protocol/pull/66 + fake_io = FakeReadPartialIO.new(["ab\r\n\r", "\nc"]) + io = Net::BufferedIO.new(fake_io) + assert_equal "ab\r", io.readuntil("\r") + # The terminator is longer than what is left unconsumed, so the rewind + # would reach back into the bytes readuntil already returned. + assert_raise(EOFError) { io.readuntil("\r\n\r\n") } + end + + def test_readuntil_ignore_eof_returns_what_is_left # https://github.com/ruby/net-protocol/pull/66 + fake_io = FakeReadPartialIO.new(["ab\r\n\r", "\nc"]) + io = Net::BufferedIO.new(fake_io) + assert_equal "ab\r", io.readuntil("\r") + assert_equal "\n\r\nc", io.readuntil("\r\n\r\n", true) + end end From 0dbbaea0eb498e66deb768763ce92324dd0d8143 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 18:37:28 +0900 Subject: [PATCH 3/3] Ignore Gemfile.lock Running the tests leaves one behind, and a prior branch committed it by accident. Anchored to the root like every other entry. Co-Authored-By: Claude Fable 5 --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 9106b2a..dc96cb9 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ /pkg/ /spec/reports/ /tmp/ +/Gemfile.lock