Fix readuntil missing a terminator that spans two reads

.gitignore

@@ -6,3 +6,4 @@
 /pkg/
 /spec/reports/
 /tmp/
+/Gemfile.lock

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)

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