Fix IO#read_nonblock, IO#readpartial, IO#sysread and preserve buffer's encoding

Author: andrykonchin

CHANGELOG.md

@@ -9,6 +9,7 @@ Bug fixes:
 * Fix `IO#{wait,wait_readable,wait_writable}` methods and switch the current thread into a sleep state (@andrykonchin).
 * Fix `rb_global_variable()` for `Float` and bignum values during the `Init_` function (#3478, @eregon).
 * Fix parsing literal floats when the locale does not use `.` for the decimal separator (e.g. `LANG=fr_FR.UTF-8`) (#3512, @eregon).
+* Fix `IO#{read_nonblock,readpartial,sysread}` and preserve a provided buffer's encoding (#3506, @andrykonchyn).
 
 Compatibility:
 * Move `IO#wait_readable`, `IO#wait_writable`, `IO#wait_priority` and `IO#wait` into core library (@andrykonchin).

spec/ruby/core/io/pread_spec.rb

@@ -22,7 +22,7 @@
 
     it "accepts a length, an offset, and an output buffer" do
       buffer = +"foo"
-      @file.pread(3, 4, buffer)
+      @file.pread(3, 4, buffer).should.equal?(buffer)
       buffer.should == "567"
     end
 
@@ -38,6 +38,13 @@
       buffer.should == "12345"
     end
 
+    it "preserves the encoding of the given buffer" do
+      buffer = ''.encode(Encoding::ISO_8859_1)
+      @file.pread(10, 0, buffer)
+
+      buffer.encoding.should == Encoding::ISO_8859_1
+    end
+
     it "does not advance the file pointer" do
       @file.pread(4, 0).should == "1234"
       @file.read.should == "1234567890"

spec/ruby/core/io/read_spec.rb

@@ -376,6 +376,21 @@
     buf.should == @contents[0..4]
   end
 
+  it "preserves the encoding of the given buffer" do
+    buffer = ''.encode(Encoding::ISO_8859_1)
+    @io.read(10, buffer)
+
+    buffer.encoding.should == Encoding::ISO_8859_1
+  end
+
+  # https://bugs.ruby-lang.org/issues/20416
+  it "does not preserve the encoding of the given buffer when max length is not provided" do
+    buffer = ''.encode(Encoding::ISO_8859_1)
+    @io.read(nil, buffer)
+
+    buffer.encoding.should_not == Encoding::ISO_8859_1
+  end
+
   it "returns the given buffer" do
     buf = +""
 

spec/ruby/core/io/readpartial_spec.rb

@@ -62,7 +62,7 @@
     buffer = +"existing content"
     @wr.write("hello world")
     @wr.close
-    @rd.readpartial(11, buffer)
+    @rd.readpartial(11, buffer).should.equal?(buffer)
     buffer.should == "hello world"
   end
 
@@ -106,6 +106,7 @@
     @wr.write("abc")
     @wr.close
     @rd.readpartial(10, buffer)
+
     buffer.encoding.should == Encoding::ISO_8859_1
   end
 end

spec/ruby/core/io/sysread_spec.rb

@@ -97,7 +97,7 @@
 
   it "discards the existing buffer content upon successful read" do
     buffer = +"existing content"
-    @file.sysread(11, buffer)
+    @file.sysread(11, buffer).should.equal?(buffer)
     buffer.should == "01234567890"
   end
 
@@ -107,6 +107,13 @@
     -> { @file.sysread(1, buffer) }.should raise_error(EOFError)
     buffer.should be_empty
   end
+
+  it "preserves the encoding of the given buffer" do
+    buffer = ''.encode(Encoding::ISO_8859_1)
+    string = @file.sysread(10, buffer)
+
+    buffer.encoding.should == Encoding::ISO_8859_1
+  end
 end
 
 describe "IO#sysread" do

spec/tags/core/io/pread_tags.txt

@@ -18,3 +18,4 @@ fails:IO#pread raises TypeError if offset is not an Integer and cannot be coerce
 fails:IO#pread raises ArgumentError for negative values of maxlen
 fails:IO#pread raised Errno::EINVAL for negative values of offset
 fails:IO#pread raises TypeError if the buffer is not a String and cannot be coerced into String
+fails:IO#pread preserves the encoding of the given buffer

spec/tags/core/io/read_nonblock_tags.txt

@@ -1,3 +1,2 @@
 fails:IO#read_nonblock raises an exception after ungetc with data in the buffer and character conversion enabled
 fails:IO#read_nonblock discards the existing buffer content upon error
-fails:IO#read_nonblock preserves the encoding of the given buffer

spec/tags/core/io/readpartial_tags.txt

@@ -1,4 +1,2 @@
 fails:BasicSocket#read_nonblock using IPv4 does not set the IO in nonblock mode
 fails:BasicSocket#read_nonblock using IPv6 does not set the IO in nonblock mode
-fails:BasicSocket#read_nonblock using IPv4 replaces the content of the provided buffer without changing its encoding
-fails:BasicSocket#read_nonblock using IPv6 replaces the content of the provided buffer without changing its encoding

spec/tags/library/socket/basicsocket/read_nonblock_tags.txt

@@ -1673,6 +1673,8 @@ def read(length = nil, buffer = nil)
       str = IO.read_encode self, read_all
       return str unless buffer
 
+      # intentionally don't preserve buffer encoding
+      # see https://bugs.ruby-lang.org/issues/20416
       return buffer.replace(str)
     end
 
@@ -1753,10 +1755,14 @@ def read_nonblock(size, buffer = nil, exception: true)
     str = Truffle::POSIX.read_string_nonblock(self, size, exception)
 
     case str
-    when Symbol
+    when Symbol # :wait_readable in case of EAGAIN_ERRNO
       str
     when String
-      buffer ? buffer.replace(str) : str
+      if buffer
+        buffer.replace str.force_encoding(buffer.encoding)
+      else
+        str
+      end
     else # EOF
       if exception
         raise EOFError, 'end of file reached'
@@ -1871,8 +1877,7 @@ def readpartial(size, buffer = nil)
         raise EOFError if Primitive.nil? data
       end
 
-      buffer.replace(data)
-      buffer
+      buffer.replace data.force_encoding(buffer.encoding)
     else
       return +'' if size == 0
 
@@ -2196,20 +2201,23 @@ def sync=(v)
   #
   #  @todo  Improve reading into provided buffer.
   #
-  def sysread(number_of_bytes, buffer = undefined)
+  def sysread(number_of_bytes, buffer = nil)
     ensure_open_and_readable
     flush
     raise IOError unless @ibuffer.empty?
 
+    buffer = StringValue(buffer) if buffer
+
     str, errno = Truffle::POSIX.read_string(self, number_of_bytes)
     Errno.handle_errno(errno) unless errno == 0
 
     raise EOFError if Primitive.nil? str
 
-    unless Primitive.undefined? buffer
-      StringValue(buffer).replace str
+    if buffer
+      buffer.replace str.force_encoding(buffer.encoding)
+    else
+      str
     end
-    str
   end
 
   ##