From eac2c7ecf942e936bdaded59f7c9bf50deceffbb Mon Sep 17 00:00:00 2001 From: Alexei Stapinski Date: Sun, 21 Apr 2024 15:31:47 -0400 Subject: [PATCH 1/4] Set offset for terminator searching to use the terminator bytesize If a multi-byte line terminator is used (e.g. "\r\n") and a read attempt into the buffer gets only part of the multi-byte terminator, then this logic would get two lines together. More importantly, if this terminator was at the very end of a POP3 message (i.e. end of message last line is ".\r\n") then another read would be attempted for data which would never come. This would result in a Net::ReadTimeout. --- lib/net/protocol.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index 197ea09..7d8f1ec 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -195,7 +195,9 @@ def readuntil(terminator, ignore_eof = false) offset = @rbuf_offset begin until idx = @rbuf.index(terminator, offset) - offset = @rbuf.bytesize + if @rbuf.bytesize - terminator.bytesize > @rbuf_offset + offset = @rbuf.bytesize - terminator.bytesize + end rbuf_fill end return rbuf_consume(idx + terminator.bytesize - @rbuf_offset) From cc58835f0fbbf793ab7a990e4cec11130162d501 Mon Sep 17 00:00:00 2001 From: Alexei Stapinski Date: Wed, 24 Apr 2024 10:24:53 -0400 Subject: [PATCH 2/4] Tweak the readuntil offset logic Mainly add a + 1 to the offset as the offset was 1 character below the optimal offset if all but 1 byte of the terminator was present in @rbuf. --- lib/net/protocol.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index 7d8f1ec..3933bfe 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -195,9 +195,10 @@ def readuntil(terminator, ignore_eof = false) offset = @rbuf_offset begin until idx = @rbuf.index(terminator, offset) - if @rbuf.bytesize - terminator.bytesize > @rbuf_offset - offset = @rbuf.bytesize - terminator.bytesize - end + new_offset = @rbuf.bytesize - terminator.bytesize + 1 + # Only assign the offset if it will advance. + # Otherwise an empty @rbuf could result in a negative offset. + offset = new_offset if new_offset > offset rbuf_fill end return rbuf_consume(idx + terminator.bytesize - @rbuf_offset) From 5d63ff1d3cd0bdf97321080796a594778fa5abd6 Mon Sep 17 00:00:00 2001 From: Alexei Stapinski Date: Mon, 13 May 2024 16:05:27 -0400 Subject: [PATCH 3/4] Add a test for multi-byte terminators in each_message_chunk and readuntil --- test/net/protocol/test_protocol.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 2f42fa3..081ae36 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -27,6 +27,26 @@ def test_each_crlf_line end end + def test_each_message_chunk_boundary + assert_output("", "") do + # Create this first chunk where the chunk will end with the + # line terminator \r\n only being partially read up to and including \r. + chunk_1 = "#{"1" * (1024 * 16 - 1)}\r\n".b + chunk_2 = "Second line\r\n".b + chunk_3 = "Third line\r\n".b + test_string = "#{chunk_1}#{chunk_2}#{chunk_3}".b + sio = StringIO.new("#{test_string}.\r\n".b) + io = Net::InternetMessageIO.new(sio) + expected_chunks = [] + io.each_message_chunk do |chunk| + expected_chunks << chunk + end + assert_equal chunk_1, expected_chunks[0] + assert_equal chunk_2, expected_chunks[1] + assert_equal chunk_3, expected_chunks[2] + end + end + def create_mockio(capacity: 100, max: nil) mockio = Object.new mockio.instance_variable_set(:@str, +'') From 5f887c1e4144e0d702c474da2f51baebdae877e9 Mon Sep 17 00:00:00 2001 From: Alexei Stapinski Date: Mon, 13 May 2024 16:46:01 -0400 Subject: [PATCH 4/4] Replace each_message_chunk calculation with BUFSIZE Just a cleanup that I forgot to do. --- test/net/protocol/test_protocol.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 081ae36..bc88bf7 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -31,7 +31,7 @@ def test_each_message_chunk_boundary assert_output("", "") do # Create this first chunk where the chunk will end with the # line terminator \r\n only being partially read up to and including \r. - chunk_1 = "#{"1" * (1024 * 16 - 1)}\r\n".b + chunk_1 = "#{"1" * (Net::BufferedIO::BUFSIZE - 1)}\r\n".b chunk_2 = "Second line\r\n".b chunk_3 = "Third line\r\n".b test_string = "#{chunk_1}#{chunk_2}#{chunk_3}".b