From 44b2dcb2f494a91204622d53b630d72d4807f07f Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Thu, 27 Aug 2026 22:45:27 +0300 Subject: [PATCH 1/6] Reject negative read lengths before changing the buffer --- lib/net/protocol.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index a62a269..b514f4d 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -189,6 +189,7 @@ def close public def read(len, dest = ''.b, ignore_eof = false) + raise ArgumentError, "negative length #{len} given" if len < 0 LOG "reading #{len} bytes..." read_bytes = 0 begin From d89c7b148ba69007c441d47062f4f1fd8601075a Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 17 Sep 2026 12:05:22 +0900 Subject: [PATCH 2/6] Test that a negative read length leaves the buffer alone Before the check, read(-1) walked @rbuf_offset backwards and a later read_all handed back bytes an earlier read had already consumed. Co-Authored-By: Claude Opus 5 --- test/net/protocol/test_protocol.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 44df36f..70a4c34 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -310,4 +310,14 @@ def test_readuntil_ignore_eof_returns_what_is_left # https://github.com/ruby/net assert_equal "ab\r", io.readuntil("\r") assert_equal "\n\r\nc", io.readuntil("\r\n\r\n", true) end + + # The length reaches rbuf_consume, which walks @rbuf_offset backwards by + # it, so a negative one has to be rejected before the buffer moves. + def test_read_rejects_a_negative_length # https://github.com/ruby/net-protocol/pull/69 + io = Net::BufferedIO.new(StringIO.new("abcdef".dup)) + assert_equal "a", io.read(1) + e = assert_raise(ArgumentError) { io.read(-1) } + assert_equal "negative length -1 given", e.message + assert_equal "bcdef", io.read_all + end end From 85361b97cd94eb01d099c0d64ca298387aba0e08 Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Thu, 27 Aug 2026 22:45:28 +0300 Subject: [PATCH 3/6] Wait for readability when an SSL write requests it --- lib/net/protocol.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index b514f4d..c4c530e 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -385,6 +385,9 @@ def write0(*strs) # next string end # continue looping + when :wait_readable + (io = @io.to_io).wait_readable(@write_timeout) or raise Net::WriteTimeout.new(io) + # continue looping when :wait_writable (io = @io.to_io).wait_writable(@write_timeout) or raise Net::WriteTimeout.new(io) # continue looping From 6f093887222b9364bbc953d6aebebae8b2f5797f Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 17 Sep 2026 12:06:37 +0900 Subject: [PATCH 4/6] Test that write0 honours a :wait_readable from the socket The case had no branch for it, so the retry loop spun without waiting and without ever reaching the write_timeout. The fake refuses the write until it has been waited on, which turns a regression into a failure rather than a hang. Co-Authored-By: Claude Opus 5 --- test/net/protocol/test_protocol.rb | 49 ++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 70a4c34..0d00d7b 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -320,4 +320,53 @@ def test_read_rejects_a_negative_length # https://github.com/ruby/net-protocol/p assert_equal "negative length -1 given", e.message assert_equal "bcdef", io.read_all end + + # OpenSSL::Buffering#write_nonblock documents :wait_readable, which a + # renegotiation produces. Takes the write only once the caller has + # waited, and caps the attempts so a regression fails instead of + # spinning until CI gives up. + class WaitReadableWriteIO + MAX_WRITES = 3 + + attr_reader :string, :waits + + def initialize(becomes_readable: true) + @becomes_readable = becomes_readable + @string = "".b + @writes = 0 + @waits = 0 + end + + def to_io; self; end + + def wait_readable(_timeout) + @waits += 1 + @becomes_readable + end + + def write_nonblock(str, exception: true) + @writes += 1 + raise "write0 ignored :wait_readable: #{@writes} attempts" if @writes > MAX_WRITES + return :wait_readable if @waits.zero? + @string << str + str.bytesize + end + end + + def test_write0_waits_for_readability # https://github.com/ruby/net-protocol/pull/70 + mockio = WaitReadableWriteIO.new + io = Net::BufferedIO.new(mockio) + io.write_timeout = 0.1 + assert_equal 5, io.write("hello") + assert_equal "hello", mockio.string + assert_equal 1, mockio.waits + end + + def test_write0_times_out_waiting_for_readability # https://github.com/ruby/net-protocol/pull/70 + mockio = WaitReadableWriteIO.new(becomes_readable: false) + io = Net::BufferedIO.new(mockio) + io.write_timeout = 0.1 + assert_raise(Net::WriteTimeout) { io.write("hello") } + assert_equal 1, mockio.waits + end end From a1bd220aad1bee75565a40a58fab030fbec720fa Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Thu, 27 Aug 2026 22:45:29 +0300 Subject: [PATCH 5/6] Restore message logging and clear write state on exceptional exits --- lib/net/protocol.rb | 53 +++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index c4c530e..763ec6a 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -357,9 +357,9 @@ def writing @debug_output << '<- ' if @debug_output yield @debug_output << "\n" if @debug_output - bytes = @written_bytes + @written_bytes + ensure @written_bytes = nil - bytes end def write0(*strs) @@ -430,12 +430,15 @@ def initialize(*, **) def each_message_chunk LOG 'reading message...' LOG_off() - read_bytes = 0 - while (line = readuntil("\r\n")) != ".\r\n" - read_bytes += line.size - yield line.delete_prefix('.') + begin + read_bytes = 0 + while (line = readuntil("\r\n")) != ".\r\n" + read_bytes += line.size + yield line.delete_prefix('.') + end + ensure + LOG_on() end - LOG_on() LOG "read message (#{read_bytes} bytes)" end @@ -461,12 +464,15 @@ def write_message_0(src) def write_message(src) LOG "writing message from #{src.class}" LOG_off() - len = writing { - using_each_crlf_line { - write_message_0 src + begin + len = writing { + using_each_crlf_line { + write_message_0 src + } } - } - LOG_on() + ensure + LOG_on() + end LOG "wrote #{len} bytes" len end @@ -474,16 +480,19 @@ def write_message(src) def write_message_by_block(&block) LOG 'writing message from block' LOG_off() - len = writing { - using_each_crlf_line { - begin - block.call(WriteAdapter.new(self.method(:write_message_0))) - rescue LocalJumpError - # allow `break' from writer block - end + begin + len = writing { + using_each_crlf_line { + begin + block.call(WriteAdapter.new(self.method(:write_message_0))) + rescue LocalJumpError + # allow `break' from writer block + end + } } - } - LOG_on() + ensure + LOG_on() + end LOG "wrote #{len} bytes" len end @@ -503,6 +512,8 @@ def using_each_crlf_line write0 "\r\n" end write0 ".\r\n" + nil + ensure @wbuf = nil end From 8ce0d368362338f6f505409d99ecbe997283f1f8 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 17 Sep 2026 12:08:20 +0900 Subject: [PATCH 6/6] Test that an exception out of a message leaves the IO usable LOG_off had no counterpart on the exception path, so a raise from the block or from the source silenced the connection's debug_output for good. The two success cases go with them because the restructure rewrote the method that had no coverage at all. Co-Authored-By: Claude Opus 5 --- test/net/protocol/test_protocol.rb | 57 ++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 0d00d7b..a6820bf 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -369,4 +369,61 @@ def test_write0_times_out_waiting_for_readability # https://github.com/ruby/net- assert_raise(Net::WriteTimeout) { io.write("hello") } assert_equal 1, mockio.waits end + + def test_write_message_by_block # https://github.com/ruby/net-protocol/pull/71 + sio = StringIO.new("".dup) + imio = Net::InternetMessageIO.new(sio) + assert_equal 10, imio.write_message_by_block { |dest| dest.write("hello\r\n") } + assert_equal "hello\r\n.\r\n", sio.string + end + + # `break' leaves through write_message_by_block itself, so the message + # stays unterminated and the method answers nil. + def test_write_message_by_block_allows_break # https://github.com/ruby/net-protocol/pull/71 + sio = StringIO.new("".dup) + imio = Net::InternetMessageIO.new(sio) + assert_nil imio.write_message_by_block { |dest| dest.write("a\r\n"); break } + assert_equal "a\r\n", sio.string + end + + def test_write_message_by_block_restores_logging_when_the_block_raises # https://github.com/ruby/net-protocol/pull/71 + sio = StringIO.new("".dup) + imio = Net::InternetMessageIO.new(sio) + debug = "".dup + imio.debug_output = debug + + assert_raise(RuntimeError) do + imio.write_message_by_block { |dest| dest.write("partial"); raise "boom" } + end + + assert_same debug, imio.debug_output + # Nothing outside reaches the half-written line or the byte count, so + # they have to be read back from the inside. + assert_nil imio.instance_variable_get(:@wbuf) + assert_nil imio.instance_variable_get(:@written_bytes) + end + + def test_write_message_restores_logging_when_the_source_raises # https://github.com/ruby/net-protocol/pull/71 + sio = StringIO.new("".dup) + imio = Net::InternetMessageIO.new(sio) + debug = "".dup + imio.debug_output = debug + + src = Object.new + def src.each; yield "partial"; raise "boom"; end + + assert_raise(RuntimeError) { imio.write_message(src) } + + assert_same debug, imio.debug_output + end + + def test_each_message_chunk_restores_logging_when_the_block_raises # https://github.com/ruby/net-protocol/pull/71 + imio = Net::InternetMessageIO.new(StringIO.new("line\r\n.\r\n".dup)) + debug = "".dup + imio.debug_output = debug + + assert_raise(RuntimeError) { imio.each_message_chunk { raise "boom" } } + + assert_same debug, imio.debug_output + end end