From 58e3ebd75fde5dccd8d67bc43a3eea929c1c44f6 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Wed, 28 Oct 2026 01:30:17 +0900 Subject: [PATCH 1/9] Fetch the underlying data after argument conversions Converting an argument (`to_int`, `to_str`, `to_s`, ...) may call back into Ruby code, which may call `StringIO#reopen` on the same stream. `reopen(other)` replaces the underlying `struct StringIO` and frees the old one, so a pointer fetched before the conversion became stale. Fetch the pointer again after the conversions, and after yielding in `each_line`. In `strio_init`, let `rb_io_extract_modeenc` write into a local instead of `ptr->flags`. Reported-by: Zabith (https://hackerone.com/nmzabith) Co-Authored-By: Claude Mythos 5.1 --- ext/stringio/stringio.c | 62 ++++++++++++++++++++++---------- test/stringio/test_stringio.rb | 64 ++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 18 deletions(-) diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c index 4f953f6..3157950 100644 --- a/ext/stringio/stringio.c +++ b/ext/stringio/stringio.c @@ -92,7 +92,7 @@ struct StringIO { }; static struct StringIO *get_strio_for_read(VALUE self); -static VALUE strio_init(int, VALUE *, struct StringIO *, VALUE); +static VALUE strio_init(int, VALUE *, VALUE); static VALUE strio_unget_bytes(struct StringIO *, const char *, rb_len_t); static rb_len_t strio_write(VALUE self, VALUE str); @@ -191,12 +191,12 @@ strio_substr(struct StringIO *ptr, rb_len_t pos, rb_len_t len, rb_encoding *enc) } static VALUE -strio_readbuf(struct StringIO *ptr, VALUE str) +strio_readbuf(VALUE self, VALUE str) { if (!NIL_P(str)) { StringValue(str); rb_str_modify(str); - if (str == ptr->string) { + if (str == get_strio_for_read(self)->string) { rb_raise(rb_eArgError, "cannot read into the underlying string"); } } @@ -321,7 +321,7 @@ strio_initialize(int argc, VALUE *argv, VALUE self) DATA_PTR(self) = ptr = strio_alloc(); } rb_call_super(0, 0); - return strio_init(argc, argv, ptr, self); + return strio_init(argc, argv, self); } static int @@ -392,20 +392,24 @@ set_encoding_by_bom(struct StringIO *ptr) } static VALUE -strio_init(int argc, VALUE *argv, struct StringIO *ptr, VALUE self) +strio_init(int argc, VALUE *argv, VALUE self) { + struct StringIO *ptr; VALUE string, vmode, opt; int oflags; + rb_io_mode_t fmode; rb_io_enc_t convconfig; argc = rb_scan_args(argc, argv, "02:", &string, &vmode, &opt); - rb_io_extract_modeenc(&vmode, 0, opt, &oflags, &ptr->flags, &convconfig); + rb_io_extract_modeenc(&vmode, 0, opt, &oflags, &fmode, &convconfig); if (!NIL_P(string)) { StringValue(string); } else if (!argc) { string = rb_enc_str_new("", 0, rb_default_external_encoding()); } + ptr = StringIOForRead(self); + ptr->flags = fmode; if (!NIL_P(string) && readonly_string_p(string)) { if (ptr->flags & FMODE_WRITABLE) { @@ -599,6 +603,7 @@ strio_set_string(VALUE self, VALUE string) rb_io_taint_check(self); ptr->flags &= ~FMODE_READWRITE; StringValue(string); + ptr = StringIO(self); ptr->flags = readonly_string_p(string) ? FMODE_READABLE : FMODE_READWRITE; ptr->pos = 0; ptr->lineno = 0; @@ -826,7 +831,8 @@ strio_get_lineno(VALUE self) static VALUE strio_set_lineno(VALUE self, VALUE lineno) { - StringIO(self)->lineno = NUM2LEN(lineno); + rb_len_t n = NUM2LEN(lineno); + StringIO(self)->lineno = n; return lineno; } @@ -888,7 +894,7 @@ strio_reopen(int argc, VALUE *argv, VALUE self) if (argc == 1 && !RB_TYPE_P(*argv, T_STRING)) { return strio_copy(self, *argv); } - return strio_init(argc, argv, StringIO(self), self); + return strio_init(argc, argv, self); } /* @@ -916,6 +922,7 @@ strio_set_pos(VALUE self, VALUE pos) { struct StringIO *ptr = StringIO(self); rb_len_t p = NUM2LEN(pos); + ptr = StringIO(self); if (p < 0) { error_inval(0); } @@ -954,13 +961,16 @@ strio_seek(int argc, VALUE *argv, VALUE self) VALUE whence; struct StringIO *ptr = StringIO(self); rb_len_t amount, offset; + long w; rb_scan_args(argc, argv, "11", NULL, &whence); amount = NUM2LEN(argv[0]); if (CLOSED(self)) { rb_raise(rb_eIOError, "closed stream"); } - switch (NIL_P(whence) ? 0 : NUM2LONG(whence)) { + w = NIL_P(whence) ? 0 : NUM2LONG(whence); + ptr = StringIO(self); + switch (w) { case 0: offset = 0; break; @@ -1211,6 +1221,9 @@ strio_ungetc(VALUE self, VALUE c) } else { StringValue(c); + ptr = readable(self); + check_modifiable(ptr); + if (NIL_P(ptr->string)) return Qnil; if (RSTRING_LEN(c) == 0) return Qnil; enc = rb_enc_get(ptr->string); enc2 = rb_enc_get(c); @@ -1241,12 +1254,15 @@ strio_ungetbyte(VALUE self, VALUE c) /* rb_int_and() not visible from exts */ VALUE v = rb_funcall(c, '&', 1, INT2FIX(0xff)); const char cc = NUM2INT(v) & 0xFF; - strio_unget_bytes(ptr, &cc, 1); + c = rb_str_new(&cc, 1); } else { StringValue(c); - strio_unget_string(ptr, c); } + ptr = readable(self); + check_modifiable(ptr); + if (NIL_P(ptr->string)) return Qnil; + strio_unget_string(ptr, c); return Qnil; } @@ -1463,8 +1479,9 @@ struct getline_arg { }; static struct getline_arg * -prepare_getline_args(struct StringIO *ptr, struct getline_arg *arg, int argc, VALUE *argv) +prepare_getline_args(VALUE self, struct StringIO **ptrp, struct getline_arg *arg, int argc, VALUE *argv) { + struct StringIO *ptr; VALUE rs, lim, opts; rb_len_t limit = -1; int respect_chomp; @@ -1494,6 +1511,7 @@ prepare_getline_args(struct StringIO *ptr, struct getline_arg *arg, int argc, VA if (!NIL_P(lim)) limit = NUM2LEN(lim); break; } + *ptrp = ptr = readable(self); if (!NIL_P(ptr->string) && !NIL_P(rs)) { rb_encoding *enc_rs, *enc_io; enc_rs = rb_enc_get(rs); @@ -1742,7 +1760,7 @@ strio_gets(int argc, VALUE *argv, VALUE self) struct getline_arg arg; VALUE str; - if (prepare_getline_args(ptr, &arg, argc, argv)->limit == 0) { + if (prepare_getline_args(self, &ptr, &arg, argc, argv)->limit == 0) { if (NIL_P(ptr->string)) return Qnil; return rb_enc_str_new(0, 0, get_enc(ptr)); } @@ -1977,11 +1995,11 @@ strio_each(int argc, VALUE *argv, VALUE self) RETURN_ENUMERATOR(self, argc, argv); - if (prepare_getline_args(ptr, &arg, argc, argv)->limit == 0) { + if (prepare_getline_args(self, &ptr, &arg, argc, argv)->limit == 0) { rb_raise(rb_eArgError, "invalid limit: 0 for each_line"); } - while (!NIL_P(line = strio_getline(&arg, ptr))) { + while (!NIL_P(line = strio_getline(&arg, StringIOForRead(self)))) { rb_yield(line); } return self; @@ -2002,7 +2020,7 @@ strio_readlines(int argc, VALUE *argv, VALUE self) struct StringIO *ptr = readable(self); struct getline_arg arg; - if (prepare_getline_args(ptr, &arg, argc, argv)->limit == 0) { + if (prepare_getline_args(self, &ptr, &arg, argc, argv)->limit == 0) { rb_raise(rb_eArgError, "invalid limit: 0 for readlines"); } @@ -2045,6 +2063,7 @@ strio_write(VALUE self, VALUE str) if (!RB_TYPE_P(str, T_STRING)) str = rb_obj_as_string(str); + ptr = writable(self); enc = get_enc(ptr); if (!enc) return 0; enc2 = rb_enc_get(str); @@ -2206,6 +2225,7 @@ strio_putc(VALUE self, VALUE ch) } else { char c = NUM2CHR(ch); + ptr = writable(self); if (NIL_P(ptr->string)) return ch; str = rb_str_new(&c, 1); } @@ -2320,7 +2340,7 @@ strio_read(int argc, VALUE *argv, VALUE self) switch (argc) { case 2: - str = strio_readbuf(ptr, argv[1]); + str = strio_readbuf(self, argv[1]); /* fall through */ case 1: if (!NIL_P(argv[0])) { @@ -2328,6 +2348,8 @@ strio_read(int argc, VALUE *argv, VALUE self) if (len < 0) { rb_raise(rb_eArgError, "negative length %"PRIdLEN" given", len); } + ptr = readable(self); + str = strio_readbuf(self, str); if (eos_p(ptr)) { if (!NIL_P(str)) rb_str_resize(str, 0); return len > 0 ? Qnil : rb_str_new(0, 0); @@ -2337,6 +2359,7 @@ strio_read(int argc, VALUE *argv, VALUE self) } /* fall through */ case 0: + ptr = readable(self); if (NIL_P(ptr->string)) return Qnil; len = RSTRING_LEN(ptr->string); if (len <= ptr->pos) { @@ -2462,7 +2485,8 @@ strio_pread(int argc, VALUE *argv, VALUE self) rb_syserr_fail_str(EINVAL, rb_sprintf("pread: Invalid offset argument: %" PRIsVALUE, rb_offset)); } - rb_buf = strio_readbuf(ptr, rb_buf); + rb_buf = strio_readbuf(self, rb_buf); + ptr = readable(self); if (len == 0) { if (NIL_P(rb_buf)) { @@ -2592,6 +2616,7 @@ strio_truncate(VALUE self, VALUE len) VALUE string = writable(self)->string; rb_len_t l = NUM2LEN(len); rb_len_t plen; + string = writable(self)->string; if (l < 0) { error_inval("negative length"); } @@ -2674,6 +2699,7 @@ strio_set_encoding(int argc, VALUE *argv, VALUE self) enc = convconfig.enc2; } } + ptr = StringIO(self); ptr->enc = enc; if (!NIL_P(ptr->string) && WRITABLE(self) && !str_chilled_p(ptr->string)) { rb_enc_associate(ptr->string, enc); diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index fd88de5..9b2b28d 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1206,6 +1206,70 @@ def test_initialize_copy assert_not_predicate(f2, :frozen?) end + def test_reopen_during_argument_conversion + ops = [ + ->(f, c) {f.read(c)}, + ->(f, c) {f.read(5, c)}, + ->(f, c) {f.pread(5, c)}, + ->(f, c) {f.pread(5, 0, c)}, + ->(f, c) {f.pos = c}, + ->(f, c) {f.seek(c)}, + ->(f, c) {f.lineno = c}, + ->(f, c) {f.truncate(c)}, + ->(f, c) {f.write(c)}, + ->(f, c) {f.putc(c)}, + ->(f, c) {f.ungetc(c)}, + ->(f, c) {f.ungetbyte(c)}, + ->(f, c) {f.gets(c)}, + ->(f, c) {f.gets(nil, c)}, + ->(f, c) {f.readlines(c)}, + ->(f, c) {f.each_line(c) {}}, + ->(f, c) {f.string = c}, + ->(f, c) {f.reopen("", c)}, + ] + ops.each do |op| + f = StringIO.new("hello\n" * 10) + g = StringIO.new("world\n" * 10) + c = Object.new + c.define_singleton_method(:to_int) {f.reopen(g); 5} + c.define_singleton_method(:to_str) {f.reopen(g); "r".dup} + c.define_singleton_method(:to_s) {f.reopen(g); "r".dup} + op.call(f, c) + assert_same(g.string, f.string, op.source_location.join(":")) + end + + f = StringIO.new("hello") + g = StringIO.new("world") + c = Object.new + c.define_singleton_method(:to_str) {f.reopen(g); "US-ASCII"} + f.set_encoding(c) + assert_equal(Encoding::US_ASCII, g.external_encoding) + end + + def test_reopen_in_each_line_block + f = StringIO.new("a\nb\nc\n") + g = StringIO.new("x\ny\n") + lines = [] + f.each_line {|l| lines << l; f.reopen(g)} + assert_equal(["a\n", "x\n", "y\n"], lines) + end + + def test_reopen_during_argument_conversion_to_underlying_string + ops = [ + ->(f, c, g) {f.read(c, g.string)}, + ->(f, c, g) {f.read(5, c)}, + ->(f, c, g) {f.pread(5, 0, c)}, + ] + ops.each do |op| + f = StringIO.new("hello") + g = StringIO.new("world") + c = Object.new + c.define_singleton_method(:to_int) {f.reopen(g); 5} + c.define_singleton_method(:to_str) {f.reopen(g); g.string} + assert_raise(ArgumentError, op.source_location.join(":")) {op.call(f, c, g)} + end + end + private def assert_string(content, encoding, str, mesg = nil) From f9ba9577f485690c6a2780e13f1b6d4d7b0deb9b Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 17:01:38 +0900 Subject: [PATCH 2/9] Don't clear flags of the old stream in `StringIO#string=` `string=` cleared the read/write flags before converting the argument. If the conversion reopens the receiver, the flags of the old stream are cleared but never restored, because the new flags are set on the replacement. Another stream that shares the old data (e.g. one made by `dup`) is left closed. The clearing is a leftover of the old code that closed the stream for a `nil` argument. The flags are overwritten after the conversion, so just remove it, and fetch the data only once after the conversion. This also stops leaving the stream closed when the conversion raises. Co-Authored-By: Claude Opus 5.5 --- ext/java/org/jruby/ext/stringio/StringIO.java | 1 - ext/stringio/stringio.c | 3 +-- test/stringio/test_stringio.rb | 10 ++++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/ext/java/org/jruby/ext/stringio/StringIO.java b/ext/java/org/jruby/ext/stringio/StringIO.java index 85bb955..d9070a6 100644 --- a/ext/java/org/jruby/ext/stringio/StringIO.java +++ b/ext/java/org/jruby/ext/stringio/StringIO.java @@ -1413,7 +1413,6 @@ public IRubyObject set_string(ThreadContext context, IRubyObject arg) { boolean locked = lock(context, ptr); try { - ptr.flags &= ~OpenFile.READWRITE; RubyString str = arg.convertToString(); ptr.flags = str.isFrozen() ? OpenFile.READABLE : OpenFile.READWRITE; ptr.pos = 0; diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c index 3157950..5ab04f4 100644 --- a/ext/stringio/stringio.c +++ b/ext/stringio/stringio.c @@ -598,10 +598,9 @@ strio_get_string(VALUE self) static VALUE strio_set_string(VALUE self, VALUE string) { - struct StringIO *ptr = StringIO(self); + struct StringIO *ptr; rb_io_taint_check(self); - ptr->flags &= ~FMODE_READWRITE; StringValue(string); ptr = StringIO(self); ptr->flags = readonly_string_p(string) ? FMODE_READABLE : FMODE_READWRITE; diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index 9b2b28d..5623737 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1246,6 +1246,16 @@ def test_reopen_during_argument_conversion assert_equal(Encoding::US_ASCII, g.external_encoding) end + def test_set_string_reopen_during_conversion_keeps_shared_stream + f = StringIO.new("hello") + shared = f.dup + g = StringIO.new("world") + c = Object.new + c.define_singleton_method(:to_str) {f.reopen(g); "new".dup} + f.string = c + assert_equal(["new", "hello"], [g.string, shared.read]) + end + def test_reopen_in_each_line_block f = StringIO.new("a\nb\nc\n") g = StringIO.new("x\ny\n") From d3076cfebc5f6e08e1e70977cd7e39d124209e73 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 17:01:38 +0900 Subject: [PATCH 3/9] Fetch the underlying data after argument conversions on JRuby Apply the same fix as the C extension to the JRuby extension. `StringIO#reopen(other)` replaces `ptr`, so data fetched before an argument conversion may belong to the old stream. Convert arguments before taking the lock and fetch `ptr` again after the conversions in `read`, `pread`, `seek`, `string=`, `truncate`, `ungetc`, `ungetbyte` and `strioInit`. `readBuf` now compares the buffer with the current underlying string instead of the one passed in. Use `num2int`/`num2long` instead of `fix2int`/`fix2long` for `lineno=`, `pos=`, `read`, `pread` and `truncate`. The latter assume the argument is a `Fixnum` and raise `ClassCastException` for an object that responds to `to_int`. Co-Authored-By: Claude Opus 5.5 --- ext/java/org/jruby/ext/stringio/StringIO.java | 156 +++++++++++------- 1 file changed, 96 insertions(+), 60 deletions(-) diff --git a/ext/java/org/jruby/ext/stringio/StringIO.java b/ext/java/org/jruby/ext/stringio/StringIO.java index d9070a6..478b960 100644 --- a/ext/java/org/jruby/ext/stringio/StringIO.java +++ b/ext/java/org/jruby/ext/stringio/StringIO.java @@ -336,53 +336,58 @@ private void strioInit(ThreadContext context, int argc, IRubyObject arg0, IRubyO StringIOData ptr = this.getPtrForWrite(); - boolean locked = lock(context, ptr); - try { - IRubyObject maybeOptions = context.nil; - switch (argc) { - case 1: - maybeOptions = ArgsUtil.getOptionsArg(runtime, arg0); - if (maybeOptions.isNil()) { - string = arg0; - } - break; - case 2: - string = arg0; - maybeOptions = ArgsUtil.getOptionsArg(runtime, arg1); - if (maybeOptions.isNil()) { - vmode = arg1; - } - break; - case 3: + IRubyObject maybeOptions = context.nil; + switch (argc) { + case 1: + maybeOptions = ArgsUtil.getOptionsArg(runtime, arg0); + if (maybeOptions.isNil()) { string = arg0; + } + break; + case 2: + string = arg0; + maybeOptions = ArgsUtil.getOptionsArg(runtime, arg1); + if (maybeOptions.isNil()) { vmode = arg1; - maybeOptions = ArgsUtil.getOptionsArg(runtime, arg2); - if (maybeOptions.isNil()) { - throw context.runtime.newArgumentError(argc, 0, 2); - } - break; - } - if (!maybeOptions.isNil()) { - argc--; - } - Object vmodeAndVpermP = VMODE_VPERM_TL.get(); - EncodingUtils.vmode(vmodeAndVpermP, vmode); - IOEncodable.ConvConfig ioEncodable = new IOEncodable.ConvConfig(); - int[] fmode = FMODE_TL.get(); - - // switch to per-use oflags if it is ever used in the future - EncodingUtils.extractModeEncoding(context, ioEncodable, vmodeAndVpermP, maybeOptions, OFLAGS_UNUSED, fmode); - ptr.flags = fmode[0]; - vmode = EncodingUtils.vmode(vmodeAndVpermP); - - // clear shared vmodeVperm - clearVmodeVperm(vmodeAndVpermP); - - if (!string.isNil()) { - string = string.convertToString(); - } else if (argc == 0) { - string = RubyString.newEmptyString(runtime, runtime.getDefaultExternalEncoding()); - } + } + break; + case 3: + string = arg0; + vmode = arg1; + maybeOptions = ArgsUtil.getOptionsArg(runtime, arg2); + if (maybeOptions.isNil()) { + throw context.runtime.newArgumentError(argc, 0, 2); + } + break; + } + if (!maybeOptions.isNil()) { + argc--; + } + Object vmodeAndVpermP = VMODE_VPERM_TL.get(); + EncodingUtils.vmode(vmodeAndVpermP, vmode); + IOEncodable.ConvConfig ioEncodable = new IOEncodable.ConvConfig(); + int[] fmodeP = FMODE_TL.get(); + + // switch to per-use oflags if it is ever used in the future + EncodingUtils.extractModeEncoding(context, ioEncodable, vmodeAndVpermP, maybeOptions, OFLAGS_UNUSED, fmodeP); + int fmode = fmodeP[0]; + vmode = EncodingUtils.vmode(vmodeAndVpermP); + + // clear shared vmodeVperm + clearVmodeVperm(vmodeAndVpermP); + + if (!string.isNil()) { + string = string.convertToString(); + } else if (argc == 0) { + string = RubyString.newEmptyString(runtime, runtime.getDefaultExternalEncoding()); + } + + // The conversions above may reopen this stream. + ptr = this.getPtrForWrite(); + + boolean locked = lock(context, ptr); + try { + ptr.flags = fmode; if (!string.isNil() && string.isFrozen()) { if ((ptr.flags & OpenFile.WRITABLE) != 0) { @@ -998,7 +1003,8 @@ public IRubyObject lineno(ThreadContext context) { @JRubyMethod(name = "lineno=", required = 1) public IRubyObject set_lineno(ThreadContext context, IRubyObject arg) { - getPtrForWrite().lineno = RubyNumeric.fix2int(arg); + int lineno = RubyNumeric.num2int(arg); + getPtrForWrite().lineno = lineno; return context.nil; } @@ -1014,7 +1020,7 @@ public IRubyObject pos(ThreadContext context) { public IRubyObject set_pos(IRubyObject arg) { checkInitialized(); - long p = RubyNumeric.fix2long(arg); + long p = RubyNumeric.num2long(arg); if (p < 0) throw getRuntime().newErrnoEINVALError(arg.toString()); @@ -1103,19 +1109,26 @@ private IRubyObject readCommon(ThreadContext context, int argc, IRubyObject arg0 IRubyObject str = context.nil; boolean binary = false; StringIOData ptr = this.getPtrForWrite(); + int len = 0; + + // Convert the arguments before fetching the data, as the + // conversions may reopen this stream. + if (argc == 2) str = readBuf(context, arg1); + if (argc >= 1 && !arg0.isNil()) { + len = RubyNumeric.num2int(arg0); + str = readBuf(context, str); + } + checkReadable(); + ptr = this.getPtrForWrite(); int pos = ptr.pos; boolean locked = lock(context, ptr); try { - int len; final RubyString string; switch (argc) { case 2: - str = readBuf(context, ptr, arg1); case 1: if (!arg0.isNil()) { - len = RubyNumeric.fix2int(arg0); - if (len < 0) { throw runtime.newArgumentError("negative length " + len + " given"); } @@ -1203,16 +1216,16 @@ private RubyString preadCommon(ThreadContext context, int argc, IRubyObject arg0 switch (argc) { case 3: - str = readBuf(context, ptr, arg2); + str = readBuf(context, arg2); case 2: - len = RubyNumeric.fix2int(arg0); + len = RubyNumeric.num2int(arg0); if (!arg0.isNil()) { if (len < 0) { throw runtime.newArgumentError("negative length " + len + " given"); } } - offset = RubyNumeric.fix2int(arg1); + offset = RubyNumeric.num2int(arg1); if (offset < 0) { throw runtime.newErrnoEINVALError("pread: Invalid offset argument"); } @@ -1222,6 +1235,11 @@ private RubyString preadCommon(ThreadContext context, int argc, IRubyObject arg0 throw runtime.newArgumentError(argc, 0, 2); } + // The conversions above may reopen this stream. + str = readBuf(context, str); + checkReadable(); + ptr = this.getPtrForRead(); + boolean locked = lock(context, ptr); try { if (len == 0) { @@ -1378,9 +1396,14 @@ private RubyFixnum seekCommon(ThreadContext context, int argc, IRubyObject arg0, checkOpen(); + int w = whence.isNil() ? 0 : RubyNumeric.num2int(whence); + // The conversion above may reopen this stream. + ptr = this.getPtrForWrite(); + checkOpen(); + boolean locked = lock(context, ptr); try { - switch (whence.isNil() ? 0 : RubyNumeric.num2int(whence)) { + switch (w) { case 0: break; case 1: @@ -1409,11 +1432,14 @@ private RubyFixnum seekCommon(ThreadContext context, int argc, IRubyObject arg0, @JRubyMethod(name = "string=", required = 1) public IRubyObject set_string(ThreadContext context, IRubyObject arg) { + checkFrozen(); + + RubyString str = arg.convertToString(); + // The conversion above may reopen this stream. StringIOData ptr = this.getPtrForWrite(); boolean locked = lock(context, ptr); try { - RubyString str = arg.convertToString(); ptr.flags = str.isFrozen() ? OpenFile.READABLE : OpenFile.READWRITE; ptr.pos = 0; ptr.lineno = 0; @@ -1441,7 +1467,8 @@ public IRubyObject sync(ThreadContext context) { public IRubyObject truncate(ThreadContext context, IRubyObject len) { checkWritable(); - int l = RubyFixnum.fix2int(len); + int l = RubyNumeric.num2int(len); + checkWritable(); StringIOData ptr = this.getPtrForWrite(); RubyString string = ptr.string; @@ -1488,6 +1515,10 @@ public IRubyObject ungetc(ThreadContext context, IRubyObject arg) { ungetbyteCommon(context, buf, 0, len); } else { arg = arg.convertToString(); + // The conversion above may reopen this stream. + checkModifiable(); + checkReadable(); + if (getPtrForWrite().string == null) return context.nil; enc = getEncoding(); RubyString argStr = (RubyString) arg; enc2 = argStr.getEncoding(); @@ -1592,7 +1623,12 @@ public IRubyObject ungetbyte(ThreadContext context, IRubyObject arg) { if (arg instanceof RubyInteger) { ungetbyteCommon(context, ((RubyInteger) ((RubyInteger) arg).op_mod(context, 256)).getIntValue()); } else { - ungetbyteCommon(context, arg.convertToString()); + RubyString str = arg.convertToString(); + // The conversion above may reopen this stream. + checkReadable(); + checkModifiable(); + if (getPtrForWrite().string == null) return context.nil; + ungetbyteCommon(context, str); } return context.nil; @@ -1694,12 +1730,12 @@ private static IRubyObject substrString(RubyString ch, IRubyObject str, Ruby run return str; } - private static IRubyObject readBuf(ThreadContext context, StringIOData ptr, IRubyObject arg) { + private IRubyObject readBuf(ThreadContext context, IRubyObject arg) { if (!arg.isNil()) { arg = arg.convertToString(); RubyString str = (RubyString) arg; modifyString(str); - if (str == ptr.string) { + if (str == getPtrForRead().string) { throw context.runtime.newArgumentError("cannot read into the underlying string"); } } From d432c9200042e8dd7261d9362875616f1fd950ed Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 17:17:49 +0900 Subject: [PATCH 4/9] Check closed state after converting whence in `StringIO#seek` `seek` checked whether the stream is closed before converting `whence`. `whence.to_int` may reopen the receiver onto a closed stream, and then `seek` changed the position of that closed stream instead of raising `IOError`. Check the closed state after the conversion instead. This also matches `IO#seek`, which converts `whence` before checking the closed state. Co-Authored-By: Claude Opus 5.5 --- ext/java/org/jruby/ext/stringio/StringIO.java | 2 -- ext/stringio/stringio.c | 4 ++-- test/stringio/test_stringio.rb | 10 ++++++++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ext/java/org/jruby/ext/stringio/StringIO.java b/ext/java/org/jruby/ext/stringio/StringIO.java index 478b960..79f8fdc 100644 --- a/ext/java/org/jruby/ext/stringio/StringIO.java +++ b/ext/java/org/jruby/ext/stringio/StringIO.java @@ -1394,8 +1394,6 @@ private RubyFixnum seekCommon(ThreadContext context, int argc, IRubyObject arg0, StringIOData ptr = this.getPtrForWrite(); - checkOpen(); - int w = whence.isNil() ? 0 : RubyNumeric.num2int(whence); // The conversion above may reopen this stream. ptr = this.getPtrForWrite(); diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c index 5ab04f4..abd9068 100644 --- a/ext/stringio/stringio.c +++ b/ext/stringio/stringio.c @@ -964,11 +964,11 @@ strio_seek(int argc, VALUE *argv, VALUE self) rb_scan_args(argc, argv, "11", NULL, &whence); amount = NUM2LEN(argv[0]); + w = NIL_P(whence) ? 0 : NUM2LONG(whence); + ptr = StringIO(self); if (CLOSED(self)) { rb_raise(rb_eIOError, "closed stream"); } - w = NIL_P(whence) ? 0 : NUM2LONG(whence); - ptr = StringIO(self); switch (w) { case 0: offset = 0; diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index 5623737..17aa713 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1256,6 +1256,16 @@ def test_set_string_reopen_during_conversion_keeps_shared_stream assert_equal(["new", "hello"], [g.string, shared.read]) end + def test_seek_reopen_to_closed_during_whence_conversion + f = StringIO.new("hello") + g = StringIO.new("world") + g.close + c = Object.new + c.define_singleton_method(:to_int) {f.reopen(g); 0} + assert_raise(IOError) {f.seek(3, c)} + assert_equal(0, g.pos) + end + def test_reopen_in_each_line_block f = StringIO.new("a\nb\nc\n") g = StringIO.new("x\ny\n") From aac7ef0cd22b6af9877aec75265354aff2be9cab Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 17:36:07 +0900 Subject: [PATCH 5/9] Check readability for each line in `StringIO#each_line` `each_line` checked readability only once before the loop. If the block closed the stream, or reopened it onto a stream that is not readable, `each_line` kept reading lines from it. Use `readable()` to fetch the stream for each line, as the JRuby extension and `IO` do. Also remove the `readable()` call at the beginning of `gets`, `each_line` and `readlines`. Its result is overwritten by `prepare_getline_args()`, which checks readability after converting the arguments. As a result, these methods convert the arguments before checking readability, and `each_line` without a block returns an enumerator even for a closed stream, as `IO` and the JRuby extension do. Co-Authored-By: Claude Opus 5.5 --- ext/stringio/stringio.c | 8 ++++---- test/stringio/test_stringio.rb | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c index abd9068..7267b56 100644 --- a/ext/stringio/stringio.c +++ b/ext/stringio/stringio.c @@ -1755,7 +1755,7 @@ strio_getline(struct getline_arg *arg, struct StringIO *ptr) static VALUE strio_gets(int argc, VALUE *argv, VALUE self) { - struct StringIO *ptr = readable(self); + struct StringIO *ptr; struct getline_arg arg; VALUE str; @@ -1989,7 +1989,7 @@ static VALUE strio_each(int argc, VALUE *argv, VALUE self) { VALUE line; - struct StringIO *ptr = readable(self); + struct StringIO *ptr; struct getline_arg arg; RETURN_ENUMERATOR(self, argc, argv); @@ -1998,7 +1998,7 @@ strio_each(int argc, VALUE *argv, VALUE self) rb_raise(rb_eArgError, "invalid limit: 0 for each_line"); } - while (!NIL_P(line = strio_getline(&arg, StringIOForRead(self)))) { + while (!NIL_P(line = strio_getline(&arg, readable(self)))) { rb_yield(line); } return self; @@ -2016,7 +2016,7 @@ static VALUE strio_readlines(int argc, VALUE *argv, VALUE self) { VALUE ary, line; - struct StringIO *ptr = readable(self); + struct StringIO *ptr; struct getline_arg arg; if (prepare_getline_args(self, &ptr, &arg, argc, argv)->limit == 0) { diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index 17aa713..de92f6a 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1274,6 +1274,38 @@ def test_reopen_in_each_line_block assert_equal(["a\n", "x\n", "y\n"], lines) end + def test_reopen_to_unreadable_in_each_line_block + f = StringIO.new("a\nb\n") + g = StringIO.new("x\ny\n") + g.close_read + lines = [] + assert_raise(IOError) {f.each_line {|l| lines << l; f.reopen(g)}} + assert_equal(["a\n"], lines) + end + + def test_close_in_each_line_block + f = StringIO.new("a\nb\n") + lines = [] + assert_raise(IOError) {f.each_line {|l| lines << l; f.close}} + assert_equal(["a\n"], lines) + end + + def test_each_line_without_block_on_closed_stream + f = StringIO.new("a\nb\n") + f.close + assert_kind_of(Enumerator, f.each_line) + end + + def test_getline_methods_convert_arguments_before_readable_check + f = StringIO.new("a\nb\n") + f.close + c = Object.new + c.define_singleton_method(:to_str) {raise TypeError, "converted"} + assert_raise_with_message(TypeError, "converted") {f.gets(c)} + assert_raise_with_message(TypeError, "converted") {f.readlines(c)} + assert_raise_with_message(TypeError, "converted") {f.each_line(c) {}} + end + def test_reopen_during_argument_conversion_to_underlying_string ops = [ ->(f, c, g) {f.read(c, g.string)}, From 086d092d859ca14816b0dbb8ddae6a7af9c0dd2e Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 17:55:47 +0900 Subject: [PATCH 6/9] Fetch the underlying data after conversions and yields in more methods on JRuby `write` checked writability only before `to_s`. If `to_s` reopened the receiver onto a read-only stream, `write` wrote to that stream. Check writability again after the conversion, as the C extension does. `each_byte` and `each_codepoint` fetched the data only once before the loop. If the block reopened the receiver, they kept reading the old data. Fetch the data for each byte or codepoint, as `strio_to_read()` in the C extension does, and don't hold the lock while yielding. As a side effect, `each_byte` and `each_codepoint` no longer raise `NullPointerException` for a null device stream, and they read data appended in the block, as the C extension does. Co-Authored-By: Claude Fable 5.2 --- ext/java/org/jruby/ext/stringio/StringIO.java | 70 +++++++++++-------- test/stringio/test_stringio.rb | 25 +++++++ 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/ext/java/org/jruby/ext/stringio/StringIO.java b/ext/java/org/jruby/ext/stringio/StringIO.java index 79f8fdc..95b65da 100644 --- a/ext/java/org/jruby/ext/stringio/StringIO.java +++ b/ext/java/org/jruby/ext/stringio/StringIO.java @@ -630,22 +630,24 @@ public IRubyObject each_byte(ThreadContext context, Block block) { if (!block.isGiven()) return enumeratorize(runtime, this, "each_byte"); - checkReadable(); - StringIOData ptr = this.getPtrForWrite(); + while (true) { + // Fetch the data for each byte, since the block may close or + // reopen this stream. + checkReadable(); + StringIOData ptr = this.getPtrForWrite(); + int c; - boolean locked = lock(context, ptr); - try { - ByteList bytes = ptr.string.getByteList(); - - // Check the length every iteration, since - // the block can modify this string. - while (ptr.pos < bytes.length()) { - // check readability for each loop, since it could get closed - checkReadable(); - block.yield(context, runtime.newFixnum(bytes.get(ptr.pos++) & 0xFF)); + boolean locked = lock(context, ptr); + try { + if (ptr.string == null) break; + ByteList bytes = ptr.string.getByteList(); + if (ptr.pos >= bytes.length()) break; + c = bytes.get(ptr.pos++) & 0xFF; + } finally { + if (locked) unlock(ptr); } - } finally { - if (locked) unlock(ptr); + + block.yield(context, runtime.newFixnum(c)); } return this; @@ -1747,6 +1749,8 @@ private long stringIOWrite(ThreadContext context, Ruby runtime, IRubyObject arg) RubyString str = arg.asString(); int len, olen; + // The conversion above may reopen this stream. + checkWritable(); StringIOData ptr = this.getPtrForWrite(); boolean locked = lock(context, ptr); @@ -1976,31 +1980,35 @@ public IRubyObject each_codepoint(ThreadContext context, Block block) { if (!block.isGiven()) return enumeratorize(runtime, this, "each_codepoint"); checkReadable(); + final Encoding enc = getEncoding(); - StringIOData ptr = this.getPtrForWrite(); - - boolean locked = lock(context, ptr); - try { - final Encoding enc = getEncoding(); - RubyString myString = ptr.string; - final ByteList string = myString.getByteList(); - final byte[] stringBytes = string.getUnsafeBytes(); - int begin = string.getBegin(); - for (; ; ) { - // check readability for each loop, since it could get closed - checkReadable(); + while (true) { + // Fetch the data for each codepoint, since the block may close or + // reopen this stream. + checkReadable(); + StringIOData ptr = this.getPtrForWrite(); + int c; + boolean locked = lock(context, ptr); + try { + if (ptr.string == null) break; + final ByteList string = ptr.string.getByteList(); + final byte[] stringBytes = string.getUnsafeBytes(); + int begin = string.getBegin(); int pos = ptr.pos; - if (pos >= string.realSize()) return this; + if (pos >= string.realSize()) break; - int c = StringSupport.codePoint(runtime, enc, stringBytes, begin + pos, stringBytes.length); + c = StringSupport.codePoint(runtime, enc, stringBytes, begin + pos, stringBytes.length); int n = StringSupport.codeLength(enc, c); ptr.pos = pos + n; - block.yield(context, runtime.newFixnum(c)); + } finally { + if (locked) unlock(ptr); } - } finally { - if (locked) unlock(ptr); + + block.yield(context, runtime.newFixnum(c)); } + + return this; } public static class GenericReadable { diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index de92f6a..b914c58 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1274,6 +1274,31 @@ def test_reopen_in_each_line_block assert_equal(["a\n", "x\n", "y\n"], lines) end + def test_reopen_to_unwritable_during_write_conversion + f = StringIO.new("hello".dup) + g = StringIO.new("world".dup, "r") + c = Object.new + c.define_singleton_method(:to_s) {f.reopen(g); "XY"} + assert_raise(IOError) {f.write(c)} + assert_equal("world", g.string) + end + + def test_reopen_in_each_byte_block + f = StringIO.new("ab") + g = StringIO.new("xy") + bytes = [] + f.each_byte {|b| bytes << b; f.reopen(g)} + assert_equal("axy".bytes, bytes) + end + + def test_reopen_in_each_codepoint_block + f = StringIO.new("ab") + g = StringIO.new("xy") + codepoints = [] + f.each_codepoint {|c| codepoints << c; f.reopen(g)} + assert_equal("axy".codepoints, codepoints) + end + def test_reopen_to_unreadable_in_each_line_block f = StringIO.new("a\nb\n") g = StringIO.new("x\ny\n") From 7863553f31c7ae0a1fdfcb5df2425b3e8f9fc44f Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 17:58:54 +0900 Subject: [PATCH 7/9] Fetch the encoding for each codepoint in `StringIO#each_codepoint` `each_codepoint` fetched the encoding only once before the loop. If the block reopened the receiver onto a stream in another encoding, or changed the encoding by `set_encoding`, the rest of the data was read in the old encoding. Fetch the encoding for each codepoint, as `each_char` does. On JRuby, also pass the end of the string instead of the end of the backing array to `StringSupport.codePoint()`. It read bytes after the end of the string when the string shared its backing array and ended with a truncated character. Co-Authored-By: Claude Fable 5.2 --- ext/java/org/jruby/ext/stringio/StringIO.java | 6 ++---- ext/stringio/stringio.c | 3 +-- test/stringio/test_stringio.rb | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/ext/java/org/jruby/ext/stringio/StringIO.java b/ext/java/org/jruby/ext/stringio/StringIO.java index 95b65da..6778a5d 100644 --- a/ext/java/org/jruby/ext/stringio/StringIO.java +++ b/ext/java/org/jruby/ext/stringio/StringIO.java @@ -1979,9 +1979,6 @@ public IRubyObject each_codepoint(ThreadContext context, Block block) { if (!block.isGiven()) return enumeratorize(runtime, this, "each_codepoint"); - checkReadable(); - final Encoding enc = getEncoding(); - while (true) { // Fetch the data for each codepoint, since the block may close or // reopen this stream. @@ -1998,7 +1995,8 @@ public IRubyObject each_codepoint(ThreadContext context, Block block) { int pos = ptr.pos; if (pos >= string.realSize()) break; - c = StringSupport.codePoint(runtime, enc, stringBytes, begin + pos, stringBytes.length); + final Encoding enc = getEncoding(); + c = StringSupport.codePoint(runtime, enc, stringBytes, begin + pos, begin + string.realSize()); int n = StringSupport.codeLength(enc, c); ptr.pos = pos + n; } finally { diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c index 7267b56..21e0529 100644 --- a/ext/stringio/stringio.c +++ b/ext/stringio/stringio.c @@ -1427,9 +1427,8 @@ strio_each_codepoint(VALUE self) RETURN_ENUMERATOR(self, 0, 0); - ptr = readable(self); - enc = get_enc(ptr); while ((ptr = strio_to_read(self)) != NULL) { + enc = get_enc(ptr); c = rb_enc_codepoint_len(RSTRING_PTR(ptr->string)+ptr->pos, RSTRING_END(ptr->string), &n, enc); ptr->pos += n; diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index b914c58..64b8d8e 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1299,6 +1299,22 @@ def test_reopen_in_each_codepoint_block assert_equal("axy".codepoints, codepoints) end + def test_reopen_to_other_encoding_in_each_codepoint_block + f = StringIO.new("ab") + g = StringIO.new("xy".encode("UTF-16LE")) + codepoints = [] + f.each_codepoint {|c| codepoints << c; f.reopen(g)} + assert_equal("axy".codepoints, codepoints) + end + + def test_each_codepoint_truncated_char_in_shared_string + s = "aあzz" + f = StringIO.new(s.byteslice(0, 3)) + codepoints = [] + assert_raise(ArgumentError) {f.each_codepoint {|c| codepoints << c}} + assert_equal([0x61], codepoints) + end + def test_reopen_to_unreadable_in_each_line_block f = StringIO.new("a\nb\n") g = StringIO.new("x\ny\n") From 985beb95ba9848fa554631bc548b27a4116fcb04 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 18:09:10 +0900 Subject: [PATCH 8/9] Check frozen state after argument conversions in `StringIO#reopen` `strio_init()` fetched the data by `StringIOForRead()` after the argument conversions. It doesn't check whether the receiver is frozen, so `reopen` modified the receiver even if a conversion froze it. Use `StringIO()` instead, as other methods and the JRuby extension do. `strio_init()` is also used by `initialize`. So calling `initialize` again on a frozen `StringIO` now raises `FrozenError` instead of modifying it, as the JRuby extension does. Co-Authored-By: Claude Fable 5.2 --- ext/stringio/stringio.c | 2 +- test/stringio/test_stringio.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c index 21e0529..b777a3c 100644 --- a/ext/stringio/stringio.c +++ b/ext/stringio/stringio.c @@ -408,7 +408,7 @@ strio_init(int argc, VALUE *argv, VALUE self) else if (!argc) { string = rb_enc_str_new("", 0, rb_default_external_encoding()); } - ptr = StringIOForRead(self); + ptr = StringIO(self); ptr->flags = fmode; if (!NIL_P(string) && readonly_string_p(string)) { diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index 64b8d8e..3eab59d 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1274,6 +1274,21 @@ def test_reopen_in_each_line_block assert_equal(["a\n", "x\n", "y\n"], lines) end + def test_reopen_freeze_during_argument_conversion + f = StringIO.new("hello") + c = Object.new + c.define_singleton_method(:to_str) {f.freeze; "new"} + assert_raise(FrozenError) {f.reopen(c, "r")} + assert_equal("hello", f.string) + end + + def test_initialize_frozen + f = StringIO.new("hello") + f.freeze + assert_raise(FrozenError) {f.send(:initialize, "new")} + assert_equal("hello", f.string) + end + def test_reopen_to_unwritable_during_write_conversion f = StringIO.new("hello".dup) g = StringIO.new("world".dup, "r") From 82fe418da570035a165f5a0839d9788c836a8288 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 Nov 2026 18:27:20 +0900 Subject: [PATCH 9/9] Fetch the underlying data after loading a transcoder or an encoding `rb_str_conv_enc()` and `rb_enc_find_index()` may load a transcoder or an encoding. Loading calls `to_path` of objects in `$LOAD_PATH`, which may reopen the stream. `write`, `ungetc` and `set_encoding_by_bom` kept using the pointer fetched before that. Fetch the pointer again after them. `set_encoding_by_bom()` detects the BOM again if the underlying string is changed. The fetched stream may be a null device stream, so check it: * `write` returns 0 for a null device stream. This also fixes a crash of `write` to a null device stream with an encoding, such as `StringIO.new(nil, external_encoding: "UTF-8")`, on both of the C extension and the JRuby extension. * `detect_bom()` accepts `nil`. This also fixes a crash of `set_encoding_by_bom` of a null device stream on both extensions. Co-Authored-By: Claude Fable 5.2 --- ext/java/org/jruby/ext/stringio/StringIO.java | 2 + ext/stringio/stringio.c | 32 +++++++-- test/stringio/test_stringio.rb | 67 ++++++++++++++++++- 3 files changed, 93 insertions(+), 8 deletions(-) diff --git a/ext/java/org/jruby/ext/stringio/StringIO.java b/ext/java/org/jruby/ext/stringio/StringIO.java index 6778a5d..713224d 100644 --- a/ext/java/org/jruby/ext/stringio/StringIO.java +++ b/ext/java/org/jruby/ext/stringio/StringIO.java @@ -1766,6 +1766,7 @@ private long stringIOWrite(ThreadContext context, Ruby runtime, IRubyObject arg) str = converted; encStr = str.getEncoding(); } + if (ptr.string == null) return 0; final ByteList strByteList = str.getByteList(); len = str.size(); if (len == 0) return 0; @@ -1921,6 +1922,7 @@ private static Encoding detectBOM(ThreadContext context, RubyString str, ObjectO int p; int len; + if (str == null) return null; ByteList byteList = str.getByteList(); byte[] bytes = byteList.unsafeBytes(); p = byteList.begin(); diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c index b777a3c..768d925 100644 --- a/ext/stringio/stringio.c +++ b/ext/stringio/stringio.c @@ -330,6 +330,7 @@ detect_bom(VALUE str, int *bomlen) const char *p; rb_len_t len; + if (NIL_P(str)) return 0; RSTRING_GETMEM(str, p, len); if (len < 1) return 0; switch ((unsigned char)p[0]) { @@ -374,12 +375,22 @@ detect_bom(VALUE str, int *bomlen) return 0; } -static rb_encoding * -set_encoding_by_bom(struct StringIO *ptr) +static struct StringIO * +set_encoding_by_bom(VALUE self) { - int bomlen, idx = detect_bom(ptr->string, &bomlen); + struct StringIO *ptr; + VALUE string; + int bomlen, idx; rb_encoding *extenc = NULL; + /* Finding an encoding may load it, and that may call back into + * Ruby code. Detect again if the underlying string is changed. */ + do { + string = StringIO(self)->string; + idx = detect_bom(string, &bomlen); + ptr = StringIO(self); + } while (ptr->string != string); + if (idx) { extenc = rb_enc_from_index(idx); ptr->pos = bomlen; @@ -388,7 +399,7 @@ set_encoding_by_bom(struct StringIO *ptr) } } ptr->enc = extenc; - return extenc; + return ptr; } static VALUE @@ -433,7 +444,7 @@ strio_init(int argc, VALUE *argv, VALUE self) } ptr->pos = 0; ptr->lineno = 0; - if (ptr->flags & FMODE_SETENC_BY_BOM) set_encoding_by_bom(ptr); + if (ptr->flags & FMODE_SETENC_BY_BOM) ptr = set_encoding_by_bom(self); RB_FL_SET_RAW(self, (ptr->flags & FMODE_READWRITE) * (STRIO_READABLE / FMODE_READABLE)); return self; } @@ -1228,6 +1239,10 @@ strio_ungetc(VALUE self, VALUE c) enc2 = rb_enc_get(c); if (enc != enc2 && enc != rb_ascii8bit_encoding()) { c = rb_str_conv_enc(c, enc2, enc); + /* Loading a transcoder may call back into Ruby code. */ + ptr = readable(self); + check_modifiable(ptr); + if (NIL_P(ptr->string)) return Qnil; } strio_unget_string(ptr, c); return Qnil; @@ -2071,7 +2086,10 @@ strio_write(VALUE self, VALUE str) rb_enc_check(rb_enc_from_encoding(enc), str); } str = converted; + /* Loading a transcoder may call back into Ruby code. */ + ptr = writable(self); } + if (NIL_P(ptr->string)) return 0; len = RSTRING_LEN(str); if (len == 0) return 0; check_modifiable(ptr); @@ -2718,9 +2736,9 @@ strio_set_encoding(int argc, VALUE *argv, VALUE self) static VALUE strio_set_encoding_by_bom(VALUE self) { - struct StringIO *ptr = StringIO(self); + struct StringIO *ptr = set_encoding_by_bom(self); - if (!set_encoding_by_bom(ptr)) return Qnil; + if (!ptr->enc) return Qnil; return rb_enc_from_encoding(ptr->enc); } diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb index 3eab59d..7a5c6c2 100644 --- a/test/stringio/test_stringio.rb +++ b/test/stringio/test_stringio.rb @@ -1274,6 +1274,71 @@ def test_reopen_in_each_line_block assert_equal(["a\n", "x\n", "y\n"], lines) end + # Loading a transcoder or an encoding calls #to_path of objects in + # $LOAD_PATH. This must run in a separate process because it's + # called only when they aren't loaded yet. + def assert_reopen_during_load(prepare, operate) + assert_separately(%w[-rstringio], "#{<<-"begin;"}\n#{<<-"end;"}") + begin; + f, g = #{prepare} + path = Object.new + path.define_singleton_method(:to_path) do + if f + f.reopen(g) + f = nil + GC.start + end + "/nonexistent" + end + $LOAD_PATH.unshift(path) + begin + stream = f + actual = #{operate} + ensure + $LOAD_PATH.delete(path) + end + f, = #{prepare} + stream = f + expected = #{operate} + assert_equal(expected, actual) + end; + end + + def test_reopen_during_transcoder_load_in_write + assert_reopen_during_load(<<-'PREPARE', <<-'OPERATE') + 2.times.map {StringIO.new("".encode("EUC-JP"))} + PREPARE + [stream.write("\u3042"), stream.string] + OPERATE + end + + def test_reopen_during_transcoder_load_in_ungetc + assert_reopen_during_load(<<-'PREPARE', <<-'OPERATE') + 2.times.map {StringIO.new("abc".encode("EUC-JP"))} + PREPARE + [stream.ungetc("\u3042"), stream.string] + OPERATE + end + + def test_reopen_during_encoding_load_in_set_encoding_by_bom + assert_reopen_during_load(<<-'PREPARE', <<-'OPERATE') + 2.times.map {StringIO.new("\xFE\xFFab".b)} + PREPARE + [stream.set_encoding_by_bom, stream.pos, stream.string.encoding] + OPERATE + end + + def test_write_null_device_with_encoding + f = StringIO.new(nil, external_encoding: "UTF-8") + assert_equal(0, f.write("abc")) + assert_nil(f.string) + end + + def test_set_encoding_by_bom_null_device + assert_nil(StringIO.new(nil).set_encoding_by_bom) + assert_nil(StringIO.new(nil, "r:BOM|UTF-8").string) + end + def test_reopen_freeze_during_argument_conversion f = StringIO.new("hello") c = Object.new @@ -1323,7 +1388,7 @@ def test_reopen_to_other_encoding_in_each_codepoint_block end def test_each_codepoint_truncated_char_in_shared_string - s = "aあzz" + s = "a\u3042zz" f = StringIO.new(s.byteslice(0, 3)) codepoints = [] assert_raise(ArgumentError) {f.each_codepoint {|c| codepoints << c}}