From 753ace46feb620bce2716fddee92ebec73f1bfa1 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 25 Aug 2026 19:50:12 +1200 Subject: [PATCH 1/9] Lock `IO::Buffer` allocations during copy. (#18489) --- ext/-test-/io_buffer/io_buffer.c | 72 +++++++++++ include/ruby/io/buffer.h | 12 ++ io_buffer.c | 207 +++++++++++++++++++++++++------ test/ruby/test_io_buffer.rb | 46 +++++++ 4 files changed, 296 insertions(+), 41 deletions(-) diff --git a/ext/-test-/io_buffer/io_buffer.c b/ext/-test-/io_buffer/io_buffer.c index ef35548bcb1010..d8bf6e4a01efe7 100644 --- a/ext/-test-/io_buffer/io_buffer.c +++ b/ext/-test-/io_buffer/io_buffer.c @@ -89,6 +89,74 @@ io_buffer_for_writing_modify_string(VALUE self, VALUE string) return rb_io_buffer_for_writing(string, io_buffer_modify_string, string); } +static VALUE +io_buffer_locked_for_reading_callback(const void *base, size_t size, VALUE buffer) +{ + VALUE result = rb_ary_new_capa(3); + + rb_ary_push(result, rb_funcall(buffer, rb_intern("locked?"), 0)); + rb_ary_push(result, SIZET2NUM(size)); + rb_ary_push(result, size > 0 ? INT2FIX(*(const unsigned char *)base) : Qnil); + + return result; +} + +static VALUE +io_buffer_locked_for_reading(VALUE self, VALUE buffer) +{ + return rb_io_buffer_locked_for_reading(buffer, io_buffer_locked_for_reading_callback, buffer); +} + +static VALUE +io_buffer_locked_for_reading_raise_callback(const void *base, size_t size, VALUE argument) +{ + (void)base; + (void)size; + (void)argument; + + rb_raise(rb_eRuntimeError, "interrupted"); +} + +static VALUE +io_buffer_locked_for_reading_raise(VALUE self, VALUE buffer) +{ + return rb_io_buffer_locked_for_reading(buffer, io_buffer_locked_for_reading_raise_callback, Qnil); +} + +static VALUE +io_buffer_locked_for_writing_callback(void *base, size_t size, VALUE buffer) +{ + VALUE locked = rb_funcall(buffer, rb_intern("locked?"), 0); + + if (size > 0) { + *(unsigned char *)base = 'x'; + } + + return locked; +} + +static VALUE +io_buffer_locked_for_writing(VALUE self, VALUE buffer) +{ + return rb_io_buffer_locked_for_writing(buffer, io_buffer_locked_for_writing_callback, buffer); +} + +static VALUE +io_buffer_locked_for_writing_raise_callback(void *base, size_t size, VALUE argument) +{ + (void)base; + (void)size; + (void)argument; + + rb_raise(rb_eRuntimeError, "interrupted"); +} + +static VALUE +io_buffer_locked_for_writing_raise(VALUE self, VALUE buffer) +{ + return rb_io_buffer_locked_for_writing(buffer, io_buffer_locked_for_writing_raise_callback, Qnil); +} + static VALUE io_buffer_lock(VALUE self, VALUE buffer) { @@ -126,6 +194,10 @@ Init_io_buffer(void) rb_define_singleton_method(mIOBuffer, "for_writing_set_string", io_buffer_for_writing_set_string, 2); rb_define_singleton_method(mIOBuffer, "for_writing_readonly?", io_buffer_for_writing_readonly_p, 1); rb_define_singleton_method(mIOBuffer, "for_writing_modify_string", io_buffer_for_writing_modify_string, 1); + rb_define_singleton_method(mIOBuffer, "locked_for_reading", io_buffer_locked_for_reading, 1); + rb_define_singleton_method(mIOBuffer, "locked_for_reading_raise", io_buffer_locked_for_reading_raise, 1); + rb_define_singleton_method(mIOBuffer, "locked_for_writing", io_buffer_locked_for_writing, 1); + rb_define_singleton_method(mIOBuffer, "locked_for_writing_raise", io_buffer_locked_for_writing_raise, 1); rb_define_singleton_method(mIOBuffer, "lock", io_buffer_lock, 1); rb_define_singleton_method(mIOBuffer, "unlock", io_buffer_unlock, 1); rb_define_singleton_method(mIOBuffer, "new_locked", io_buffer_new_locked, 1); diff --git a/include/ruby/io/buffer.h b/include/ruby/io/buffer.h index d67ec6246dd3be..2d0244411b9cb6 100644 --- a/include/ruby/io/buffer.h +++ b/include/ruby/io/buffer.h @@ -104,6 +104,18 @@ enum rb_io_buffer_flags rb_io_buffer_get_bytes(VALUE self, void **base, size_t * void rb_io_buffer_get_bytes_for_reading(VALUE self, const void **base, size_t *size); void rb_io_buffer_get_bytes_for_writing(VALUE self, void **base, size_t *size); +// Lock the backing allocation, invoke the callback with its readable bytes, +// and automatically unlock it when the callback returns or raises. The bytes +// are only valid for the duration of the callback. This protects the lifetime +// of the allocation; it does not provide synchronization for its contents. +VALUE rb_io_buffer_locked_for_reading(VALUE self, VALUE (*callback)(const void *base, size_t size, VALUE argument), VALUE argument); + +// Lock the backing allocation, invoke the callback with its writable bytes, +// and automatically unlock it when the callback returns or raises. The bytes +// are only valid for the duration of the callback. This protects the lifetime +// of the allocation; it does not provide synchronization for its contents. +VALUE rb_io_buffer_locked_for_writing(VALUE self, VALUE (*callback)(void *base, size_t size, VALUE argument), VALUE argument); + VALUE rb_io_buffer_transfer(VALUE self); void rb_io_buffer_resize(VALUE self, size_t size); void rb_io_buffer_clear(VALUE self, uint8_t value, size_t offset, size_t length); diff --git a/io_buffer.c b/io_buffer.c index 6792577d9b6efd..6b22254a8bf370 100644 --- a/io_buffer.c +++ b/io_buffer.c @@ -1673,6 +1673,73 @@ rb_io_buffer_locked_ensure(VALUE self) return Qnil; } +struct io_buffer_readable_bytes_arguments { + VALUE self; + VALUE (*callback)(const void *base, size_t size, VALUE argument); + VALUE argument; +}; + +static VALUE +io_buffer_readable_bytes_call(VALUE _arguments) +{ + struct io_buffer_readable_bytes_arguments *arguments = (void *)_arguments; + + const void *base; + size_t size; + rb_io_buffer_get_bytes_for_reading(arguments->self, &base, &size); + + return arguments->callback(base, size, arguments->argument); +} + +VALUE +rb_io_buffer_locked_for_reading(VALUE self, VALUE (*callback)(const void *base, size_t size, VALUE argument), VALUE argument) +{ + struct rb_io_buffer *buffer = get_io_buffer(self); + io_buffer_validate_for_reading(buffer); + + struct io_buffer_readable_bytes_arguments arguments = { + .self = self, + .callback = callback, + .argument = argument, + }; + + rb_io_buffer_lock(self); + return rb_ensure(io_buffer_readable_bytes_call, (VALUE)&arguments, rb_io_buffer_locked_ensure, self); +} + +struct io_buffer_writable_bytes_arguments { + VALUE self; + VALUE (*callback)(void *base, size_t size, VALUE argument); + VALUE argument; +}; + +static VALUE +io_buffer_writable_bytes_call(VALUE _arguments) +{ + struct io_buffer_writable_bytes_arguments *arguments = (void *)_arguments; + + void *base; + size_t size; + rb_io_buffer_get_bytes_for_writing(arguments->self, &base, &size); + + return arguments->callback(base, size, arguments->argument); +} + +VALUE +rb_io_buffer_locked_for_writing(VALUE self, VALUE (*callback)(void *base, size_t size, VALUE argument), VALUE argument) +{ + get_io_buffer_for_writing(self); + + struct io_buffer_writable_bytes_arguments arguments = { + .self = self, + .callback = callback, + .argument = argument, + }; + + rb_io_buffer_lock(self); + return rb_ensure(io_buffer_writable_bytes_call, (VALUE)&arguments, rb_io_buffer_locked_ensure, self); +} + /* * call-seq: locked { ... } * @@ -2891,13 +2958,11 @@ io_buffer_memmove_unblock(void *data) } static void -io_buffer_memmove(struct rb_io_buffer *buffer, size_t offset, const void *source_base, size_t source_offset, size_t source_size, size_t length) +io_buffer_memmove(void *base, size_t size, size_t offset, const void *source_base, size_t source_offset, size_t source_size, size_t length) { - void *base; - size_t size; - io_buffer_get_bytes_for_writing(buffer, &base, &size); - - io_buffer_validate_range(buffer, offset, length); + if (size_sum_is_bigger_than(offset, length, size)) { + rb_raise(rb_eArgError, "Specified offset+length is bigger than the buffer size!"); + } if (size_sum_is_bigger_than(source_offset, length, source_size)) { rb_raise(rb_eArgError, "The computed source range exceeds the size of the source buffer!"); @@ -2920,45 +2985,114 @@ io_buffer_memmove(struct rb_io_buffer *buffer, size_t offset, const void *source } } -// (offset, length, source_offset) -> length -static VALUE -io_buffer_copy_from(struct rb_io_buffer *buffer, const void *source_base, size_t source_size, int argc, VALUE *argv) +static void +io_buffer_extract_copy_arguments(size_t source_size, int argc, VALUE *argv, size_t *offset, size_t *length, size_t *source_offset) { - size_t offset = 0; - size_t length; - size_t source_offset; - // The offset we copy into the buffer: if (argc >= 1) { - offset = io_buffer_extract_offset(argv[0]); + *offset = io_buffer_extract_offset(argv[0]); + } + else { + *offset = 0; } // The offset we start from within the string: if (argc >= 3) { - source_offset = io_buffer_extract_offset(argv[2]); + *source_offset = io_buffer_extract_offset(argv[2]); - if (source_offset > source_size) { + if (*source_offset > source_size) { rb_raise(rb_eArgError, "The given source offset is bigger than the source itself!"); } } else { - source_offset = 0; + *source_offset = 0; } // The length we are going to copy: if (argc >= 2 && !RB_NIL_P(argv[1])) { - length = io_buffer_extract_length(argv[1]); + *length = io_buffer_extract_length(argv[1]); } else { // Default to the source offset -> source size: - length = source_size - source_offset; + *length = source_size - *source_offset; } +} - io_buffer_memmove(buffer, offset, source_base, source_offset, source_size, length); +// (offset, length, source_offset) -> length +static VALUE +io_buffer_copy_from(struct rb_io_buffer *buffer, const void *source_base, size_t source_size, int argc, VALUE *argv) +{ + size_t offset, length, source_offset; + io_buffer_extract_copy_arguments(source_size, argc, argv, &offset, &length, &source_offset); + + void *base; + size_t size; + io_buffer_get_bytes_for_writing(buffer, &base, &size); + + io_buffer_memmove(base, size, offset, source_base, source_offset, source_size, length); return SIZET2NUM(length); } +struct io_buffer_copy_arguments { + VALUE destination; + const void *source_base; + size_t source_size; + int argc; + VALUE *argv; +}; + +// This is the innermost callback for IO::Buffer#copy. At this point the source +// is locked for reading and the destination is locked for writing, so both +// pointers and sizes remain valid while arguments are extracted, ranges are +// validated, and memmove potentially releases the GVL. +static VALUE +io_buffer_copy_to(void *base, size_t size, VALUE _arguments) +{ + struct io_buffer_copy_arguments *arguments = (void *)_arguments; + + size_t offset, length, source_offset; + io_buffer_extract_copy_arguments(arguments->source_size, arguments->argc, arguments->argv, &offset, &length, &source_offset); + + io_buffer_memmove(base, size, offset, arguments->source_base, source_offset, arguments->source_size, length); + + return SIZET2NUM(length); +} + +// This callback runs while the source is locked for reading. Retain its bytes +// in the callback arguments, then enter the destination's writable scope. The +// source scope remains active until that nested scope returns. +static VALUE +io_buffer_copy_from_readable(const void *base, size_t size, VALUE _arguments) +{ + struct io_buffer_copy_arguments *arguments = (void *)_arguments; + + arguments->source_base = base; + arguments->source_size = size; + + return rb_io_buffer_locked_for_writing(arguments->destination, io_buffer_copy_to, _arguments); +} + +static VALUE +io_buffer_initialize_copy_from(const void *base, size_t size, VALUE self) +{ + struct rb_io_buffer *buffer = get_io_buffer(self); + + io_buffer_initialize(self, buffer, NULL, size, io_flags_for_size(size), Qnil); + + struct io_buffer_copy_arguments arguments = { + .destination = self, + .source_base = base, + .source_size = size, + .argc = 0, + .argv = NULL, + }; + + // The source remains locked by the outer readable scope while the newly + // initialized destination is locked and populated by io_buffer_copy_to. + return rb_io_buffer_locked_for_writing(self, io_buffer_copy_to, (VALUE)&arguments); +} + /* * call-seq: * dup -> io_buffer @@ -2979,18 +3113,7 @@ io_buffer_copy_from(struct rb_io_buffer *buffer, const void *source_base, size_t static VALUE rb_io_buffer_initialize_copy(VALUE self, VALUE source) { - struct rb_io_buffer *buffer = get_io_buffer(self); - - const void *source_base; - size_t source_size; - - rb_io_buffer_get_bytes_for_reading(source, &source_base, &source_size); - - io_buffer_initialize(self, buffer, NULL, source_size, io_flags_for_size(source_size), Qnil); - - VALUE result = io_buffer_copy_from(buffer, source_base, source_size, 0, NULL); - RB_GC_GUARD(source); - return result; + return rb_io_buffer_locked_for_reading(source, io_buffer_initialize_copy_from, self); } /* @@ -3066,17 +3189,19 @@ io_buffer_copy(int argc, VALUE *argv, VALUE self) { rb_check_arity(argc, 1, 4); - struct rb_io_buffer *buffer = get_io_buffer(self); - VALUE source = argv[0]; - const void *source_base; - size_t source_size; - - rb_io_buffer_get_bytes_for_reading(source, &source_base, &source_size); + struct io_buffer_copy_arguments arguments = { + .destination = self, + .argc = argc-1, + .argv = argv+1, + }; - VALUE result = io_buffer_copy_from(buffer, source_base, source_size, argc-1, argv+1); - RB_GC_GUARD(source); - return result; + // Lock the source first, then io_buffer_copy_from_readable nests the + // destination lock. The scoped helpers use rb_ensure, so the destination + // is unlocked before the source on both normal and exceptional returns. + // If both buffers share an allocation, its reference-counted lock is + // acquired and released twice. + return rb_io_buffer_locked_for_reading(source, io_buffer_copy_from_readable, (VALUE)&arguments); } /* diff --git a/test/ruby/test_io_buffer.rb b/test/ruby/test_io_buffer.rb index 25c52892d44df5..08aa634b70288a 100644 --- a/test/ruby/test_io_buffer.rb +++ b/test/ruby/test_io_buffer.rb @@ -94,6 +94,52 @@ def test_internal_for_reading_unlocks_after_callback_exception assert_equal "hello!", string end + def test_internal_locked_for_reading + buffer = IO::Buffer.for("hello") + + assert_equal [true, 5, "h".ord], Bug::IOBuffer.locked_for_reading(buffer) + refute_predicate buffer, :locked? + end + + def test_internal_locked_for_reading_unlocks_after_callback_exception + buffer = IO::Buffer.for("hello") + + assert_raise(RuntimeError) do + Bug::IOBuffer.locked_for_reading_raise(buffer) + end + + refute_predicate buffer, :locked? + end + + def test_internal_locked_for_writing + buffer = IO::Buffer.new(5) + buffer.set_string("hello") + + assert_equal true, Bug::IOBuffer.locked_for_writing(buffer) + assert_equal "xello", buffer.get_string + refute_predicate buffer, :locked? + end + + def test_internal_locked_for_writing_rejects_readonly_buffer + buffer = IO::Buffer.for("hello") + + assert_raise(IO::Buffer::AccessError) do + Bug::IOBuffer.locked_for_writing(buffer) + end + + refute_predicate buffer, :locked? + end + + def test_internal_locked_for_writing_unlocks_after_callback_exception + buffer = IO::Buffer.new(5) + + assert_raise(RuntimeError) do + Bug::IOBuffer.locked_for_writing_raise(buffer) + end + + refute_predicate buffer, :locked? + end + def test_endian assert_equal 4, IO::Buffer::LITTLE_ENDIAN assert_equal 8, IO::Buffer::BIG_ENDIAN From e7619442d203ceb309f15a915ba8fdc0004ca5e4 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 18:37:27 +0900 Subject: [PATCH 2/9] [ruby/net-protocol] 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. https://github.com/ruby/net-protocol/commit/84ef3aba09 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 8c81298c0e420a..903ea35c0ef48d 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 be288812fd75e453b812e8aa29cb714e7970c495 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 18:37:28 +0900 Subject: [PATCH 3/9] [ruby/net-protocol] 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. https://github.com/ruby/net-protocol/commit/215cc35666 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 2f42fa3236705a..0693ad684fcb43 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 225ff017268303e9624ef37eec2c06c3a93ac241 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 17:27:30 +0900 Subject: [PATCH 4/9] Update rbs to 4.2.0 The bundled_gems workflow has failed every day since 2026-08-16 because rbs 4.0.3's Ripper_test.rb no longer finds Ripper on master. rbs 4.2.0 picks up the upstream fix that requires "ripper" in the stdlib tests. typeprof 0.32.0 never finishes load_core_rbs against rbs 4.2.0, so it is pinned to a master revision until the next release, and the newly added collection_install test is skipped because rbs's bundle_install helper runs whichever bundle comes first on PATH. --- gems/bundled_gems | 4 ++-- tool/rbs_skip_tests | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/gems/bundled_gems b/gems/bundled_gems index b35c2a6ea13438..48840ad446cdc2 100644 --- a/gems/bundled_gems +++ b/gems/bundled_gems @@ -16,8 +16,8 @@ net-imap 0.6.6 https://github.com/ruby/net-imap net-smtp 0.5.1 https://github.com/ruby/net-smtp matrix 0.4.3 https://github.com/ruby/matrix prime 0.1.4 https://github.com/ruby/prime -rbs 4.0.3 https://github.com/ruby/rbs -typeprof 0.32.0 https://github.com/ruby/typeprof +rbs 4.2.0 https://github.com/ruby/rbs +typeprof 0.32.0 https://github.com/ruby/typeprof b950f910c6b6e4736988637f46a1ae256384c9f7 debug 1.11.1 https://github.com/ruby/debug 6510cfbc7496c55ebbefa437a25c17ca58f7c5eb racc 1.8.1 https://github.com/ruby/racc mutex_m 0.3.0 https://github.com/ruby/mutex_m diff --git a/tool/rbs_skip_tests b/tool/rbs_skip_tests index 33cee2e6850149..3620e9f9d43fbb 100644 --- a/tool/rbs_skip_tests +++ b/tool/rbs_skip_tests @@ -25,6 +25,7 @@ test_collection_install__mutex_m__config__stdlib_source(RBS::CliTest) running te test_collection_install__mutex_m__dependency_no_bundled(RBS::CliTest) running tests without Bundler test_collection_install__mutex_m__no_bundled(RBS::CliTest) running tests without Bundler test_collection_install__mutex_m__rbs_dependency_and__gem_dependency(RBS::CliTest) running tests without Bundler +test_collection_install__nongem_stdlib_no_warning(RBS::CliTest) running tests without Bundler test_collection_install_frozen(RBS::CliTest) running tests without Bundler test_collection_install_gemspec(RBS::CliTest) running tests without Bundler test_collection_update(RBS::CliTest) running tests without Bundler From 48a3a07b8ce85842efbf276a4e920677c373d0ce Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 17:29:56 +0900 Subject: [PATCH 5/9] Unpin rbs from the bundled gems auto-update rbs was pinned in March 2026 while the 4.0 series was breaking CI. Now that 4.2.0 is in gems/bundled_gems and the tests pass again, the daily workflow can pick up later rbs releases on its own. --- tool/update-bundled_gems.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tool/update-bundled_gems.rb b/tool/update-bundled_gems.rb index 565a522aa04776..056c2a6524527c 100755 --- a/tool/update-bundled_gems.rb +++ b/tool/update-bundled_gems.rb @@ -5,7 +5,7 @@ # STDOUT is not usable in inplace edit mode output = $-i ? STDOUT : STDERR # Gems to skip auto-updating (e.g. when a new major version breaks CI) - pinned = %w[rbs] + pinned = %w[] } output = STDERR if ARGF.file == STDIN END { From f830d938a6195ca8c5cb9f64adecaabf978a6cf1 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 18:11:47 +0900 Subject: [PATCH 6/9] Skip the rbs 4.2.0 stdlib tests that still fail in CI rbs 4.2.0 adds Etc and IO#pathconf stdlib tests that need getpwent(), getgrent() and the pathconf constants, none of which exist on Windows, so they go in the Windows-only list. OpenURISingletonTest reaches www.ruby-lang.org and now fails on Ubuntu as well, so its skip moves out of the Windows list into the shared one. --- tool/rbs_skip_tests | 1 + tool/rbs_skip_tests_windows | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/tool/rbs_skip_tests b/tool/rbs_skip_tests index 3620e9f9d43fbb..6c19e73c792524 100644 --- a/tool/rbs_skip_tests +++ b/tool/rbs_skip_tests @@ -35,6 +35,7 @@ NetInstanceTest depending on external resources TestHTTPRequest depending on external resources TestSingletonNetHTTPResponse depending on external resources TestInstanceNetHTTPResponse depending on external resources +test_URI_open(OpenURISingletonTest) depending on external resources test_TOPDIR(RbConfigSingletonTest) `TOPDIR` is `nil` during CI while RBS type is declared as `String` diff --git a/tool/rbs_skip_tests_windows b/tool/rbs_skip_tests_windows index db12c69419e88e..6d6ce531cc37db 100644 --- a/tool/rbs_skip_tests_windows +++ b/tool/rbs_skip_tests_windows @@ -17,6 +17,15 @@ test_confstr(EtcSingletonTest) # NameError: uninitialized constant Etc::SC_ARG_MAX test_sysconf(EtcSingletonTest) +# NameError: uninitialized constant Etc::Group +test_each(EtcGroupSingletonTest) + +# NameError: uninitialized constant Etc::PC_PIPE_BUF +test_pathconf(IOInstanceTest) + +# `Etc::Passwd.each` returns the class itself instead of an Enumerator because getpwent() is unavailable +test_each(EtcPasswdSingletonTest) + # Errno::EACCES: Permission denied @ apply2files - C:/a/_temp/d20250813-10156-udw6rx/chmod test_chmod(FileInstanceTest) test_chmod(FileInstanceTest) @@ -71,9 +80,6 @@ test_each(OpenSSLConfigTest) test_lookup_and_set(OpenSSLConfigTest) test_sections(OpenSSLConfigTest) -# OpenSSL::SSL::SSLError: SSL_connect returned=1 errno=0 peeraddr=185.199.108.153:443 state=error: certificate verify failed (unable to get local issuer certificate) -test_URI_open(OpenURISingletonTest) - # ArgumentError: both textmode and binmode specified test_binwrite(PathnameInstanceTest) From a06bf2f8f764fa26d2f97ae6bf32a4a40f5f4d4b Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 25 Aug 2026 21:50:16 +1200 Subject: [PATCH 7/9] Lock `IO::Buffer` coercion callbacks. (#18490) --- ext/-test-/io_buffer/io_buffer.c | 33 +++++++++++++++++++-- internal/io_buffer.h | 16 ++++++---- io_buffer.c | 35 ++++++++++++++++++++-- test/ruby/test_io_buffer.rb | 51 ++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 11 deletions(-) diff --git a/ext/-test-/io_buffer/io_buffer.c b/ext/-test-/io_buffer/io_buffer.c index d8bf6e4a01efe7..60ff067db03d1b 100644 --- a/ext/-test-/io_buffer/io_buffer.c +++ b/ext/-test-/io_buffer/io_buffer.c @@ -44,6 +44,13 @@ io_buffer_raise(VALUE buffer, VALUE argument) rb_raise(rb_eRuntimeError, "interrupted"); } +static VALUE +io_buffer_locked_p(VALUE buffer, VALUE argument) +{ + (void)argument; + return rb_funcall(buffer, rb_intern("locked?"), 0); +} + static VALUE io_buffer_for_reading_get_string(VALUE self, VALUE object) { @@ -63,10 +70,15 @@ io_buffer_for_reading_object_id(VALUE self, VALUE object) } static VALUE -io_buffer_for_reading_raise(VALUE self, VALUE string) +io_buffer_for_reading_raise(VALUE self, VALUE object) { - StringValue(string); - return rb_io_buffer_for_reading(string, io_buffer_raise, Qnil); + return rb_io_buffer_for_reading(object, io_buffer_raise, Qnil); +} + +static VALUE +io_buffer_for_reading_locked_p(VALUE self, VALUE object) +{ + return rb_io_buffer_for_reading(object, io_buffer_locked_p, Qnil); } static VALUE @@ -89,6 +101,18 @@ io_buffer_for_writing_modify_string(VALUE self, VALUE string) return rb_io_buffer_for_writing(string, io_buffer_modify_string, string); } +static VALUE +io_buffer_for_writing_raise(VALUE self, VALUE object) +{ + return rb_io_buffer_for_writing(object, io_buffer_raise, Qnil); +} + +static VALUE +io_buffer_for_writing_locked_p(VALUE self, VALUE object) +{ + return rb_io_buffer_for_writing(object, io_buffer_locked_p, Qnil); +} + static VALUE io_buffer_locked_for_reading_callback(const void *base, size_t size, VALUE buffer) { @@ -191,9 +215,12 @@ Init_io_buffer(void) rb_define_singleton_method(mIOBuffer, "for_reading_readonly?", io_buffer_for_reading_readonly_p, 1); rb_define_singleton_method(mIOBuffer, "for_reading_object_id", io_buffer_for_reading_object_id, 1); rb_define_singleton_method(mIOBuffer, "for_reading_raise", io_buffer_for_reading_raise, 1); + rb_define_singleton_method(mIOBuffer, "for_reading_locked?", io_buffer_for_reading_locked_p, 1); rb_define_singleton_method(mIOBuffer, "for_writing_set_string", io_buffer_for_writing_set_string, 2); rb_define_singleton_method(mIOBuffer, "for_writing_readonly?", io_buffer_for_writing_readonly_p, 1); rb_define_singleton_method(mIOBuffer, "for_writing_modify_string", io_buffer_for_writing_modify_string, 1); + rb_define_singleton_method(mIOBuffer, "for_writing_raise", io_buffer_for_writing_raise, 1); + rb_define_singleton_method(mIOBuffer, "for_writing_locked?", io_buffer_for_writing_locked_p, 1); rb_define_singleton_method(mIOBuffer, "locked_for_reading", io_buffer_locked_for_reading, 1); rb_define_singleton_method(mIOBuffer, "locked_for_reading_raise", io_buffer_locked_for_reading_raise, 1); rb_define_singleton_method(mIOBuffer, "locked_for_writing", io_buffer_locked_for_writing, 1); diff --git a/internal/io_buffer.h b/internal/io_buffer.h index 51e78f13465aba..114282137796da 100644 --- a/internal/io_buffer.h +++ b/internal/io_buffer.h @@ -8,10 +8,13 @@ RUBY_SYMBOL_EXPORT_BEGIN /** * Wrap string_or_buffer as a read-only IO::Buffer view and invoke callback(buffer, argument). * - * - IO::Buffer: callback is called directly with no wrapping. + * The resulting buffer's backing allocation is locked for the duration of the + * callback and automatically unlocked when the callback returns or raises. + * + * - IO::Buffer: locked and passed directly to the callback. * - String: locked to prevent GC compaction from moving the backing memory, - * wrapped in a read-only IO::Buffer, callback called inside rb_ensure, buffer - * freed and string unlocked on exit. + * wrapped in a locked read-only IO::Buffer, callback called inside + * rb_ensure, buffer freed and string unlocked on exit. * - Other: TypeError raised. */ VALUE rb_io_buffer_for_reading(VALUE string_or_buffer, VALUE (*callback)(VALUE buffer, VALUE argument), VALUE argument); @@ -20,9 +23,12 @@ VALUE rb_io_buffer_for_reading(VALUE string_or_buffer, VALUE (*callback)(VALUE b * Wrap string_or_buffer as a writable IO::Buffer view and invoke callback(buffer, argument). * * - Read-only IO::Buffer: ArgumentError raised. - * - IO::Buffer: callback is called directly with no wrapping. + * The resulting buffer's backing allocation is locked for the duration of the + * callback and automatically unlocked when the callback returns or raises. + * + * - IO::Buffer: locked and passed directly to the callback. * - String: locked, wrapped in a writable IO::Buffer, callback called inside - * rb_ensure, buffer freed and string unlocked on exit. + * rb_ensure, buffer unlocked and freed, and string unlocked on exit. * - Other: TypeError raised. */ VALUE rb_io_buffer_for_writing(VALUE string_or_buffer, VALUE (*callback)(VALUE buffer, VALUE argument), VALUE argument); diff --git a/io_buffer.c b/io_buffer.c index 6b22254a8bf370..f7283e9593ff13 100644 --- a/io_buffer.c +++ b/io_buffer.c @@ -584,6 +584,35 @@ struct io_buffer_for_callback_arguments { VALUE argument; }; +static VALUE rb_io_buffer_locked_ensure(VALUE self); + +struct io_buffer_for_locked_callback_arguments { + VALUE buffer; + VALUE (*callback)(VALUE, VALUE); + VALUE argument; +}; + +static VALUE +io_buffer_for_locked_callback_call(VALUE _arguments) +{ + struct io_buffer_for_locked_callback_arguments *arguments = (void *)_arguments; + + return arguments->callback(arguments->buffer, arguments->argument); +} + +static VALUE +io_buffer_for_locked_callback(VALUE buffer, VALUE (*callback)(VALUE, VALUE), VALUE argument) +{ + struct io_buffer_for_locked_callback_arguments arguments = { + .buffer = buffer, + .callback = callback, + .argument = argument, + }; + + rb_io_buffer_lock(buffer); + return rb_ensure(io_buffer_for_locked_callback_call, (VALUE)&arguments, rb_io_buffer_locked_ensure, buffer); +} + static VALUE io_buffer_for_callback_call(VALUE _arguments) { @@ -596,7 +625,7 @@ io_buffer_for_callback_call(VALUE _arguments) arguments->locked = 1; } - return arguments->callback(arguments->instance, arguments->argument); + return io_buffer_for_locked_callback(arguments->instance, arguments->callback, arguments->argument); } static VALUE @@ -619,7 +648,7 @@ VALUE rb_io_buffer_for_reading(VALUE string_or_buffer, VALUE (*callback)(VALUE, VALUE), VALUE argument) { if (rb_obj_is_kind_of(string_or_buffer, rb_cIOBuffer)) { - return callback(string_or_buffer, argument); + return io_buffer_for_locked_callback(string_or_buffer, callback, argument); } else if (RB_TYPE_P(string_or_buffer, T_STRING)) { StringValue(string_or_buffer); @@ -652,7 +681,7 @@ rb_io_buffer_for_writing(VALUE string_or_buffer, VALUE (*callback)(VALUE, VALUE) if (io_buffer_readonly_p(buffer)) { rb_raise(rb_eArgError, "buffer is read-only"); } - return callback(string_or_buffer, argument); + return io_buffer_for_locked_callback(string_or_buffer, callback, argument); } else if (RB_TYPE_P(string_or_buffer, T_STRING)) { StringValue(string_or_buffer); diff --git a/test/ruby/test_io_buffer.rb b/test/ruby/test_io_buffer.rb index 08aa634b70288a..9eacc3927157a4 100644 --- a/test/ruby/test_io_buffer.rb +++ b/test/ruby/test_io_buffer.rb @@ -47,12 +47,15 @@ def test_internal_for_reading_with_string assert_equal "hello", Bug::IOBuffer.for_reading_get_string(string) assert_equal true, Bug::IOBuffer.for_reading_readonly?(string) + assert_equal true, Bug::IOBuffer.for_reading_locked?(string) end def test_internal_for_reading_with_io_buffer buffer = IO::Buffer.for("hello") assert_equal buffer.object_id, Bug::IOBuffer.for_reading_object_id(buffer) + assert_equal true, Bug::IOBuffer.for_reading_locked?(buffer) + refute_predicate buffer, :locked? end def test_internal_for_writing_with_string @@ -62,6 +65,14 @@ def test_internal_for_writing_with_string assert_equal "world", string assert_equal false, Bug::IOBuffer.for_writing_readonly?(string) + assert_equal true, Bug::IOBuffer.for_writing_locked?(string) + end + + def test_internal_for_writing_with_io_buffer + buffer = IO::Buffer.new(5) + + assert_equal true, Bug::IOBuffer.for_writing_locked?(buffer) + refute_predicate buffer, :locked? end def test_internal_for_writing_rejects_readonly_buffer @@ -94,6 +105,46 @@ def test_internal_for_reading_unlocks_after_callback_exception assert_equal "hello!", string end + def test_internal_for_reading_unlocks_io_buffer_after_callback_exception + buffer = IO::Buffer.new(5) + + assert_raise(RuntimeError) do + Bug::IOBuffer.for_reading_raise(buffer) + end + + refute_predicate buffer, :locked? + end + + def test_internal_for_writing_unlocks_io_buffer_after_callback_exception + buffer = IO::Buffer.new(5) + + assert_raise(RuntimeError) do + Bug::IOBuffer.for_writing_raise(buffer) + end + + refute_predicate buffer, :locked? + end + + def test_internal_for_reading_preserves_existing_lock + buffer = IO::Buffer.new(5) + Bug::IOBuffer.lock(buffer) + + assert_equal true, Bug::IOBuffer.for_reading_locked?(buffer) + assert_predicate buffer, :locked? + ensure + Bug::IOBuffer.unlock(buffer) if buffer&.locked? + end + + def test_internal_for_writing_preserves_existing_lock + buffer = IO::Buffer.new(5) + Bug::IOBuffer.lock(buffer) + + assert_equal true, Bug::IOBuffer.for_writing_locked?(buffer) + assert_predicate buffer, :locked? + ensure + Bug::IOBuffer.unlock(buffer) if buffer&.locked? + end + def test_internal_locked_for_reading buffer = IO::Buffer.for("hello") From ee3b80cdb4b3266bf5041da2360c533366967b9a Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 20:14:35 +0900 Subject: [PATCH 8/9] [ruby/net-protocol] Add limit option to Net::BufferedIO#readuntil readuntil buffers until the terminator arrives, so a peer that never sends one grows the read buffer without bound. The limit lets a protocol implementation cap what a single read may return, raising the new Net::ReadLimitExceeded instead of reading on. The exception derives from ProtocolError because an over-long line is the peer violating the protocol rather than an I/O failure, and because IOError would place it in the generic socket-error rescue that Net::HTTP retries on. https://github.com/ruby/net-http/issues/315 https://github.com/ruby/net-protocol/commit/e1b38d98b7 Co-Authored-By: Claude Fable 5 --- lib/net/protocol.rb | 39 +++++++++- test/net/protocol/test_protocol.rb | 117 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index 903ea35c0ef48d..410f83332bd999 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -79,6 +79,28 @@ class ProtoRetriableError < ProtocolError; end ProtocRetryError = ProtoRetriableError # :startdoc: + ## + # ReadLimitExceeded, a subclass of ProtocolError, is raised if the + # terminator is not found within the byte limit given to + # Net::BufferedIO#readuntil. + # + # The limit is the largest result readuntil may return, counting the + # terminator itself, so a limit of 4 accepts "abc\n" and rejects + # "abcd\n". Unlike the limit of IO#gets it never truncates a result to + # fit. Either the whole thing comes back or this is raised, except + # under ignore_eof, which still returns what was buffered when the + # stream ended. The count is in bytes while the IO hands back binary + # strings, which every real one does. + # + # It bounds one call, not a connection. The unconsumed buffer can + # still run one BUFSIZE past the limit, and a peer sending endless + # short lines is not bounded at all. + # + # Nothing is consumed when this is raised, so the usual response is to + # close the connection rather than read on under a wider limit. + + class ReadLimitExceeded < ProtocolError; end + ## # OpenTimeout, a subclass of Timeout::Error, is raised if a connection cannot # be created within the open_timeout. @@ -205,10 +227,19 @@ def read_all(dest = ''.b) dest end - def readuntil(terminator, ignore_eof = false) + def readuntil(terminator, ignore_eof = false, limit: nil) + unless limit.nil? || (Integer === limit && limit > 0) + # Integer === calls nothing on limit, and only an Integer is + # echoed back, so validation never runs the caller's code. + got = Integer === limit ? limit : "a non-Integer" + raise ArgumentError, "limit must be a positive Integer, got #{got}" + end offset = @rbuf_offset begin until idx = @rbuf.index(terminator, offset) + if limit && rbuf_size > limit + raise ReadLimitExceeded, "exceeded the #{limit} byte read limit" + end # 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 @@ -219,7 +250,11 @@ def readuntil(terminator, ignore_eof = false) offset = [@rbuf.bytesize - terminator.bytesize + 1, @rbuf_offset].max rbuf_fill end - return rbuf_consume(idx + terminator.bytesize - @rbuf_offset) + len = idx + terminator.bytesize - @rbuf_offset + if limit && len > limit + raise ReadLimitExceeded, "exceeded the #{limit} byte read limit" + end + return rbuf_consume(len) rescue EOFError raise unless ignore_eof return rbuf_consume diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 0693ad684fcb43..145d46b8c2a587 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -65,6 +65,115 @@ def test_readuntil end end + def test_readuntil_limit + io = Net::BufferedIO.new(StringIO.new("123\n45678\n".dup)) + assert_equal "123\n", io.readuntil("\n", limit: 4) + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\n", limit: 4) } + end + + # The limit measures the result, not the position of the terminator in + # the buffer, so bytes consumed by an earlier read must not count + # against it. + def test_readuntil_limit_ignores_already_consumed_bytes + io = Net::BufferedIO.new(StringIO.new("123\n45678\n".dup)) + assert_equal "123\n", io.readuntil("\n", limit: 4) + assert_equal "45678\n", io.readuntil("\n", limit: 6) + end + + def test_readuntil_limit_is_a_protocol_error + assert_operator Net::ReadLimitExceeded, :<, Net::ProtocolError + end + + # Which of the two checks fires is decided by how the peer split its + # writes, so both have to report the same thing. + def test_readuntil_limit_message_does_not_depend_on_chunking + whole = Net::BufferedIO.new(StringIO.new("45678\n".dup)) + split = Net::BufferedIO.new(FakeReadPartialIO.new(["45678", "\n"])) + messages = [whole, split].map do |io| + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\n", limit: 4) }.message + end + assert_equal messages.first, messages.last + assert_match(/\b4\b/, messages.first) + assert_match(/limit/, messages.first) + end + + def test_readuntil_limit_after_a_long_earlier_read + io = Net::BufferedIO.new(FakeReadPartialIO.new(["aaaaaaaaaa\nbc", "\n"])) + assert_equal "aaaaaaaaaa\n", io.readuntil("\n", limit: 11) + assert_equal "bc\n", io.readuntil("\n", limit: 3) + end + + def test_readuntil_limit_rejects_values_that_are_not_a_positive_integer + { 0 => "0", -1 => "-1", false => "a non-Integer", + 4.5 => "a non-Integer", "4" => "a non-Integer" }.each do |limit, expected| + io = Net::BufferedIO.new(StringIO.new("123\n".dup)) + e = assert_raise(ArgumentError, "limit: #{limit.inspect}") do + io.readuntil("\n", limit: limit) + end + assert_equal "limit must be a positive Integer, got #{expected}", e.message + end + + io = Net::BufferedIO.new(StringIO.new("123\n".dup)) + assert_equal "123\n", io.readuntil("\n", limit: nil) + end + + def test_readuntil_limit_counts_the_terminator + io = Net::BufferedIO.new(StringIO.new("1234\n".dup)) + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\n", limit: 4) } + + io = Net::BufferedIO.new(StringIO.new("1234\n".dup)) + assert_equal "1234\n", io.readuntil("\n", limit: 5) + end + + def test_readuntil_limit_consumes_nothing_when_it_raises + io = Net::BufferedIO.new(StringIO.new("45678\nrest\n".dup)) + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\n", limit: 4) } + assert_equal "45678\n", io.readuntil("\n", limit: 6) + assert_equal "rest\n", io.readuntil("\n") + end + + def test_readuntil_limit_ignore_eof + io = Net::BufferedIO.new(StringIO.new("abc".dup)) + assert_equal "abc", io.readuntil("\n", true, limit: 10) + end + + # The EOF path returns the buffer without consulting the limit, so + # only the loop's earlier check keeps it inside. + def test_readuntil_limit_bounds_what_ignore_eof_returns_at_eof + io = Net::BufferedIO.new(FakeReadPartialIO.new(["abcde"])) + assert_equal "abcde", io.readuntil("\n", true, limit: 5) + + io = Net::BufferedIO.new(FakeReadPartialIO.new(["abcdef"])) + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\n", true, limit: 5) } + end + + def test_readuntil_limit_applies_with_ignore_eof + io = Net::BufferedIO.new(StringIO.new("abcdefghij".dup)) + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\n", true, limit: 5) } + end + + # Never yields the terminator. Capping the reads makes a regression in + # the limit check fail instead of running the CI host out of memory. + class EndlessIO + MAX_READS = 2 + + def initialize + @reads = 0 + end + + def read_nonblock(size, buf = nil, exception: false) + @reads += 1 + raise "readuntil ignored its limit: #{@reads} reads" if @reads > MAX_READS + s = ("a" * size).b + buf ? buf.replace(s) : s + end + end + + def test_readuntil_limit_endless_stream + io = Net::BufferedIO.new(EndlessIO.new) + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\n", limit: 1024) } + end + def test_write0_multibyte mockio = create_mockio(max: 1) io = Net::BufferedIO.new(mockio) @@ -162,6 +271,14 @@ def test_shareable_buffer_leak # https://github.com/ruby/net-protocol/pull/19 assert_equal expected_chunks, actual_chunks end + def test_readuntil_limit_with_a_terminator_spanning_chunks + io = Net::BufferedIO.new(FakeReadPartialIO.new(["abc\r", "\ndef\r\n"])) + assert_equal "abc\r\n", io.readuntil("\r\n", limit: 5) + + io = Net::BufferedIO.new(FakeReadPartialIO.new(["abc\r", "\ndef\r\n"])) + assert_raise(Net::ReadLimitExceeded) { io.readuntil("\r\n", limit: 4) } + 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) From c344021a1185e3b64eb47aec4ff9fcd3a081858e Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 25 Aug 2026 20:14:49 +0900 Subject: [PATCH 9/9] [ruby/net-protocol] Trim the comments added with the readuntil rewind The floor's two reasons fit in one block, and the two tests that restated them are named for what they cover. https://github.com/ruby/net-protocol/commit/6ffd258b7c Co-Authored-By: Claude Fable 5 --- lib/net/protocol.rb | 11 ++++------- test/net/protocol/test_protocol.rb | 9 ++------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index 410f83332bd999..a4ac3c7c10a388 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -240,13 +240,10 @@ def readuntil(terminator, ignore_eof = false, limit: nil) if limit && rbuf_size > limit raise ReadLimitExceeded, "exceeded the #{limit} byte read limit" end - # 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. + # Rewind so a terminator split across reads is still found. The + # floor guards two things. String#index reads a negative offset + # as counting from the end, and an offset below @rbuf_offset + # matches inside bytes already returned. offset = [@rbuf.bytesize - terminator.bytesize + 1, @rbuf_offset].max rbuf_fill end diff --git a/test/net/protocol/test_protocol.rb b/test/net/protocol/test_protocol.rb index 145d46b8c2a587..44df36f3589d3b 100644 --- a/test/net/protocol/test_protocol.rb +++ b/test/net/protocol/test_protocol.rb @@ -239,9 +239,8 @@ def test_write0_timeout_multi2 class FakeReadPartialIO def initialize(chunks) - # 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. + # Binary, like a real IO. String#b also copies, which matters + # because rbuf_fill clears a string it was not handed as the buffer. @chunks = chunks.map(&:b) end @@ -295,8 +294,6 @@ def test_readuntil_terminator_spanning_more_than_two_chunks # https://github.com 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 @@ -304,8 +301,6 @@ def test_readuntil_does_not_rewind_into_consumed_bytes # https://github.com/ruby 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