Skip to content

Commit

Permalink
Restore UploadedFile compatibility with IO.copy_stream
Browse files Browse the repository at this point in the history
In #28676 the `#to_path` method was
added to `ActionDispatch::Http::UploadedFile`. This broke usage with
`IO.copy_stream`:

  source = ActionDispatch::Http::UploadedFile.new(...)
  IO.copy_stream(source, destination)
  # ~> TypeError: can't convert ActionDispatch::Http::UploadedFile to IO (ActionDispatch::Http::UploadedFile#to_io gives Tempfile)

Normally `IO.copy_stream` just calls `#read` on the source object.
However, when `#to_path` is defined, `IO.copy_stream` calls `#to_io` in
order to retrieve the raw `File` object. In that case it trips up,
because `ActionDispatch::Http::UploadedFile#to_io` returned a `Tempfile`
object, which is not an `IO` subclass.

We fix this by having `#to_io` return an actual `File` object.
  • Loading branch information
janko committed Feb 23, 2019
1 parent 9118a8f commit 32b2942
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
5 changes: 4 additions & 1 deletion actionpack/lib/action_dispatch/http/upload.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class UploadedFile
# 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
Expand Down Expand Up @@ -84,6 +83,10 @@ def size
def eof?
@tempfile.eof?
end

def to_io
@tempfile.to_io
end
end
end
end
15 changes: 13 additions & 2 deletions actionpack/test/dispatch/uploaded_file_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "abstract_unit"
require "tempfile"
require "stringio"

module ActionDispatch
class UploadedFileTest < ActiveSupport::TestCase
Expand Down Expand Up @@ -49,10 +50,10 @@ def test_tempfile
assert_equal tf, uf.tempfile
end

def test_to_io_returns_tempfile
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
Expand Down Expand Up @@ -115,5 +116,15 @@ def test_delegate_to_path_to_tempfile
uf = Http::UploadedFile.new(tempfile: tf)
assert_equal tf.to_path, uf.to_path
end

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

1 comment on commit 32b2942

@joevandyk
Copy link
Contributor

@joevandyk joevandyk commented on 32b2942 Jul 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what it's worth, this also fixes this error in Rails 5.2:

uploaded_file = ActionDispatch::Http::UploadedFile.new(tempfile: Tempfile.new)

File.size(uploaded_file)

# TypeError (can't convert ActionDispatch::Http::UploadedFile to IO # # ## 
# (ActionDispatch::Http::UploadedFile#to_io gives Tempfile))

(the aws-sdk-s3 gem uses File.size - so trying to upload a ActionDispatch::Http::UploadedFile file to s3 generated this error)

Please sign in to comment.