From 3dd18a6c585724db36bee3cdd49aeb650984998e Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Mon, 2 Oct 2023 15:01:49 +0200 Subject: [PATCH 1/2] Fix IO.copy_stream with a Tempfile destination * Fixes https://github.com/oracle/truffleruby/issues/3280 --- CHANGELOG.md | 1 + spec/ruby/core/io/copy_stream_spec.rb | 33 ++++++++++++++++++++++----- src/main/ruby/truffleruby/core/io.rb | 26 +++++++++------------ 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b693c411d55..296aca81c2d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ New features: Bug fixes: * Fix `rb_enc_left_char_head()` so it is not always `ArgumentError` (#3267, @eregon). +* Fix `IO.copy_stream` with a `Tempfile` destination (#3280, @eregon). Compatibility: diff --git a/spec/ruby/core/io/copy_stream_spec.rb b/spec/ruby/core/io/copy_stream_spec.rb index df9c5c7390e4..ffa2ea992ce4 100644 --- a/spec/ruby/core/io/copy_stream_spec.rb +++ b/spec/ruby/core/io/copy_stream_spec.rb @@ -69,9 +69,12 @@ end it "raises an IOError if the destination IO is not open for writing" do - @to_io.close - @to_io = new_io @to_name, "r" - -> { IO.copy_stream @object.from, @to_io }.should raise_error(IOError) + to_io = new_io __FILE__, "r" + begin + -> { IO.copy_stream @object.from, to_io }.should raise_error(IOError) + ensure + to_io.close + end end it "does not close the destination IO" do @@ -109,7 +112,8 @@ end after :each do - rm_r @to_name, @from_bigfile + rm_r @to_name if @to_name + rm_r @from_bigfile end describe "from an IO" do @@ -164,6 +168,25 @@ it_behaves_like :io_copy_stream_to_io, nil, IOSpecs::CopyStream it_behaves_like :io_copy_stream_to_io_with_offset, nil, IOSpecs::CopyStream end + + describe "to a Tempfile" do + before :all do + require 'tempfile' + end + + before :each do + @to_io = Tempfile.new("rubyspec_copy_stream", encoding: Encoding::BINARY, mode: File::RDONLY) + @to_name = @to_io.path + end + + after :each do + @to_io.close! + @to_name = nil # do not rm_r it, already done by Tempfile#close! + end + + it_behaves_like :io_copy_stream_to_io, nil, IOSpecs::CopyStream + it_behaves_like :io_copy_stream_to_io_with_offset, nil, IOSpecs::CopyStream + end end describe "from a file name" do @@ -277,10 +300,8 @@ @io.should_not_receive(:pos) IO.copy_stream(@io, @to_name) end - end - describe "with a destination that does partial reads" do before do @from_out, @from_in = IO.pipe diff --git a/src/main/ruby/truffleruby/core/io.rb b/src/main/ruby/truffleruby/core/io.rb index 5a25c0338f6c..a29b0b634719 100644 --- a/src/main/ruby/truffleruby/core/io.rb +++ b/src/main/ruby/truffleruby/core/io.rb @@ -356,24 +356,20 @@ def initialize(from, to, length, offset) @method = read_method @from end + # From copy_stream_body in io.c in CRuby + # The first element is true if obj can be used as an IO directly def to_io(obj, mode) - if Primitive.is_a?(obj, IO) - flag = true - io = obj + has_to_path = obj.respond_to? :to_path + io = (Primitive.is_a?(obj, IO) || has_to_path) && IO.try_convert(obj) + if io + [true, io] + elsif Primitive.is_a?(obj, String) or has_to_path + path = Truffle::Type.coerce_to obj, String, :to_path + io = File.open path, mode + [false, io] else - flag = false - - if Primitive.is_a?(obj, String) - io = File.open obj, mode - elsif obj.respond_to? :to_path - path = Truffle::Type.coerce_to obj, String, :to_path - io = File.open path, mode - else - io = obj - end + [false, obj] end - - [flag, io] end def read_method(obj) From 60b4cfdf1b166540a624ebe65b67819efac3f305 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Mon, 2 Oct 2023 16:30:49 +0200 Subject: [PATCH 2/2] Refactor logic to be closer to CRuby's logic for IO.copy_stream --- src/main/ruby/truffleruby/core/io.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/ruby/truffleruby/core/io.rb b/src/main/ruby/truffleruby/core/io.rb index a29b0b634719..ca300fbf6293 100644 --- a/src/main/ruby/truffleruby/core/io.rb +++ b/src/main/ruby/truffleruby/core/io.rb @@ -359,16 +359,16 @@ def initialize(from, to, length, offset) # From copy_stream_body in io.c in CRuby # The first element is true if obj can be used as an IO directly def to_io(obj, mode) - has_to_path = obj.respond_to? :to_path - io = (Primitive.is_a?(obj, IO) || has_to_path) && IO.try_convert(obj) - if io + unless Primitive.is_a?(obj, IO) || Primitive.is_a?(obj, String) || obj.respond_to?(:to_path) + return [false, obj] + end + + if io = IO.try_convert(obj) [true, io] - elsif Primitive.is_a?(obj, String) or has_to_path + else path = Truffle::Type.coerce_to obj, String, :to_path io = File.open path, mode [false, io] - else - [false, obj] end end