diff --git a/ext/-test-/io_buffer/io_buffer.c b/ext/-test-/io_buffer/io_buffer.c index ef35548bcb1010..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,86 @@ 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) +{ + 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) { @@ -123,9 +215,16 @@ 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); + 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/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/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/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 6792577d9b6efd..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); @@ -1673,6 +1702,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 +2987,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 +3014,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; } +} + +// (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(buffer, offset, source_base, source_offset, source_size, length); + 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 +3142,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 +3218,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/lib/net/protocol.rb b/lib/net/protocol.rb index 8c81298c0e420a..a4ac3c7c10a388 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,14 +227,31 @@ 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) - offset = @rbuf.bytesize + if limit && rbuf_size > limit + raise ReadLimitExceeded, "exceeded the #{limit} byte read limit" + end + # 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 - 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 2f42fa3236705a..44df36f3589d3b 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) @@ -130,15 +239,19 @@ def test_write0_timeout_multi2 class FakeReadPartialIO def initialize(chunks) - @chunks = chunks.map(&:dup) + # 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 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 +269,45 @@ 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_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) + 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) + 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") + 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 diff --git a/test/ruby/test_io_buffer.rb b/test/ruby/test_io_buffer.rb index 25c52892d44df5..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,92 @@ 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") + + 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 diff --git a/tool/rbs_skip_tests b/tool/rbs_skip_tests index 33cee2e6850149..6c19e73c792524 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 @@ -34,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) 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 {