Merge pull request #35382 from janko/restore-io-copy-stream-compatibility-with-uploaded-file

Restore ActionDispatch::Http::UploadedFile compatibility with IO.copy_stream

2 files changed, 62 insertions, 48 deletions

diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb
index 827f022ca2..0da8f5c14e 100644
--- a/actionpack/lib/action_dispatch/http/upload.rb
+++ b/actionpack/lib/action_dispatch/http/upload.rb
@@ -20,7 +20,6 @@ module ActionDispatch
       # A +Tempfile+ object with the actual uploaded file. Note that some of
       # its interface is available directly.
       attr_accessor :tempfile
-      alias :to_io :tempfile
 
       # A string with the headers of the multipart request.
       attr_accessor :headers
@@ -84,6 +83,10 @@ module ActionDispatch
       def eof?
         @tempfile.eof?
       end
+
+      def to_io
+        @tempfile.to_io
+      end
     end
   end
 end
diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb
index 21169fcb5c..03e5274541 100644
--- a/actionpack/test/dispatch/uploaded_file_test.rb
+++ b/actionpack/test/dispatch/uploaded_file_test.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 require "abstract_unit"
+require "tempfile"
+require "stringio"
 
 module ActionDispatch
   class UploadedFileTest < ActiveSupport::TestCase
@@ -11,109 +13,118 @@ module ActionDispatch
     end
 
     def test_original_filename
-      uf = Http::UploadedFile.new(filename: "foo", tempfile: Object.new)
+      uf = Http::UploadedFile.new(filename: "foo", tempfile: Tempfile.new)
       assert_equal "foo", uf.original_filename
     end
 
     def test_filename_is_different_object
       file_str = "foo"
-      uf = Http::UploadedFile.new(filename: file_str, tempfile: Object.new)
+      uf = Http::UploadedFile.new(filename: file_str, tempfile: Tempfile.new)
       assert_not_equal file_str.object_id, uf.original_filename.object_id
     end
 
     def test_filename_should_be_in_utf_8
-      uf = Http::UploadedFile.new(filename: "foo", tempfile: Object.new)
+      uf = Http::UploadedFile.new(filename: "foo", tempfile: Tempfile.new)
       assert_equal "UTF-8", uf.original_filename.encoding.to_s
     end
 
     def test_filename_should_always_be_in_utf_8
       uf = Http::UploadedFile.new(filename: "foo".encode(Encoding::SHIFT_JIS),
-                                  tempfile: Object.new)
+                                  tempfile: Tempfile.new)
       assert_equal "UTF-8", uf.original_filename.encoding.to_s
     end
 
     def test_content_type
-      uf = Http::UploadedFile.new(type: "foo", tempfile: Object.new)
+      uf = Http::UploadedFile.new(type: "foo", tempfile: Tempfile.new)
       assert_equal "foo", uf.content_type
     end
 
     def test_headers
-      uf = Http::UploadedFile.new(head: "foo", tempfile: Object.new)
+      uf = Http::UploadedFile.new(head: "foo", tempfile: Tempfile.new)
       assert_equal "foo", uf.headers
     end
 
     def test_tempfile
-      uf = Http::UploadedFile.new(tempfile: "foo")
-      assert_equal "foo", uf.tempfile
+      tf = Tempfile.new
+      uf = Http::UploadedFile.new(tempfile: tf)
+      assert_equal tf, uf.tempfile
     end
 
-    def test_to_io_returns_the_tempfile
-      tf = Object.new
+    def test_to_io_returns_file
+      tf = Tempfile.new
       uf = Http::UploadedFile.new(tempfile: tf)
-      assert_equal tf, uf.to_io
+      assert_equal tf.to_io, uf.to_io
     end
 
     def test_delegates_path_to_tempfile
-      tf = Class.new { def path; "thunderhorse" end }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_equal "thunderhorse", uf.path
+      tf = Tempfile.new
+      uf = Http::UploadedFile.new(tempfile: tf)
+      assert_equal tf.path, uf.path
     end
 
     def test_delegates_open_to_tempfile
-      tf = Class.new { def open; "thunderhorse" end }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_equal "thunderhorse", uf.open
+      tf = Tempfile.new
+      tf.close
+      uf = Http::UploadedFile.new(tempfile: tf)
+      assert_equal tf, uf.open
+      assert_not tf.closed?
     end
 
     def test_delegates_close_to_tempfile
-      tf = Class.new { def close(unlink_now = false); "thunderhorse" end }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_equal "thunderhorse", uf.close
+      tf = Tempfile.new
+      uf = Http::UploadedFile.new(tempfile: tf)
+      uf.close
+      assert tf.closed?
     end
 
     def test_close_accepts_parameter
-      tf = Class.new { def close(unlink_now = false); "thunderhorse: #{unlink_now}" end }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_equal "thunderhorse: true", uf.close(true)
+      tf = Tempfile.new
+      uf = Http::UploadedFile.new(tempfile: tf)
+      uf.close(true)
+      assert tf.closed?
+      assert_nil tf.path
     end
 
     def test_delegates_read_to_tempfile
-      tf = Class.new { def read(length = nil, buffer = nil); "thunderhorse" end }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
+      tf = Tempfile.new
+      tf << "thunderhorse"
+      tf.rewind
+      uf = Http::UploadedFile.new(tempfile: tf)
       assert_equal "thunderhorse", uf.read
     end
 
     def test_delegates_read_to_tempfile_with_params
-      tf = Class.new { def read(length = nil, buffer = nil); [length, buffer] end }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_equal %w{ thunder horse }, uf.read(*%w{ thunder horse })
-    end
-
-    def test_delegate_respects_respond_to?
-      tf = Class.new { def read; yield end; private :read }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_raises(NoMethodError) do
-        uf.read
-      end
+      tf = Tempfile.new
+      tf << "thunderhorse"
+      tf.rewind
+      uf = Http::UploadedFile.new(tempfile: tf)
+      assert_equal "thunder", uf.read(7)
+      assert_equal "horse",   uf.read(5, String.new)
     end
 
     def test_delegate_eof_to_tempfile
-      tf = Class.new { def eof?; true end; }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_predicate uf, :eof?
+      tf = Tempfile.new
+      tf << "thunderhorse"
+      uf = Http::UploadedFile.new(tempfile: tf)
+      assert_equal true, uf.eof?
+      tf.rewind
+      assert_equal false, uf.eof?
     end
 
     def test_delegate_to_path_to_tempfile
-      tf = Class.new { def to_path; "/any/file/path" end; }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_equal "/any/file/path", uf.to_path
+      tf = Tempfile.new
+      uf = Http::UploadedFile.new(tempfile: tf)
+      assert_equal tf.to_path, uf.to_path
     end
 
-    def test_respond_to?
-      tf = Class.new { def read; yield end }
-      uf = Http::UploadedFile.new(tempfile: tf.new)
-      assert_respond_to uf, :headers
-      assert_respond_to uf, :read
+    def test_io_copy_stream
+      tf = Tempfile.new
+      tf << "thunderhorse"
+      tf.rewind
+      uf = Http::UploadedFile.new(tempfile: tf)
+      result = StringIO.new
+      IO.copy_stream(uf, result)
+      assert_equal "thunderhorse", result.string
     end
   end
 end