aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorAaron Kromer <aaron.kromer@gmail.com>2018-07-22 04:00:40 -0400
committerKasper Timm Hansen <kaspth@gmail.com>2018-07-22 10:00:40 +0200
commit20543c049625784a91944efdebc1d0c397406f21 (patch)
treeef7dee42b0815dfafdfee1be0d7ad511f31468f4 /actionpack
parent76f22e19d6793c24a570f413e4f6b7b9b5416bf7 (diff)
downloadrails-20543c049625784a91944efdebc1d0c397406f21.tar.gz
rails-20543c049625784a91944efdebc1d0c397406f21.tar.bz2
rails-20543c049625784a91944efdebc1d0c397406f21.zip
Add implicit to path conversion to uploaded file (#28676)
* Add implicit to path conversion to uploaded file Ruby has a few implicit conversion protocols (e.g. `to_hash`, `to_str`, `to_path`, etc.). These are considered implicit conversion protocols because in certain instances Ruby (MRI core objects) will check if an argument responds to the appropriate protocol and automatically convert it when it does; this is why you can provide a `Pathname` instance into `File.read` without having to explicitly call `to_s`. ```ruby a_file_path = 'some/path/file.ext' File.write a_file_path, 'String Path Content' File.read a_file_path a_pathname = Pathname(a_file_path) File.write core_file, 'Pathname Content' File.read a_file_path core_file = File.new(a_pathname) File.write core_file, 'File Content' File.read core_file tmp_file = Tempfile.new('example') File.write tmp_file, 'Tempfile Content' File.read tmp_file ``` So how does an uploaded file work in such cases? ```ruby tmp_file = Tempfile.new('example') File.write tmp_file, 'Uploaded Content' uploaded_file = ActionDispatch::Http::UploadedFile.new(tempfile: tmp_file) File.read uploaded_file ``` It fails with a `TypeError`: no implicit conversion of ActionDispatch::Http::UploadedFile into String In order to make an uploaded file work it must be explicitly converted to a file path using `path`. ```ruby File.read uploaded_file.path ``` This requires any code that expects path/file like objects to either special case an uploaded file, re-implement the path conversion protocol to use `path`, or forces the developer to explicitly cast uploaded files to paths. This last option can sometimes be difficult to do when such calls are deep within the inner workings of libraries. Since an uploaded file already has a path it makes sense to implement the implicit "path" conversion protocol (just like `File` and `Tempfile`). This change allows uploaded file content to be treated more closely to regular file content, without requiring any special case handling or explicit conversion for common file utilities. * Note uploaded file path delegation in CHANGELOG
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md10
-rw-r--r--actionpack/lib/action_dispatch/http/upload.rb5
-rw-r--r--actionpack/test/dispatch/uploaded_file_test.rb6
3 files changed, 21 insertions, 0 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 7645b2b0e7..f7e9104627 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,13 @@
+* `ActionDispatch::Http::UploadedFile` now delegates `to_path` to its tempfile.
+
+ This allows uploaded file objects to be passed directly to `File.read`
+ without raising a `TypeError`:
+
+ uploaded_file = ActionDispatch::Http::UploadedFile.new(tempfile: tmp_file)
+ File.read(uploaded_file)
+
+ *Aaron Kromer*
+
* Pass along arguments to underlying `get` method in `follow_redirect!`
Now all arguments passed to `follow_redirect!` are passed to the underlying
diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb
index 0b162dc7f1..827f022ca2 100644
--- a/actionpack/lib/action_dispatch/http/upload.rb
+++ b/actionpack/lib/action_dispatch/http/upload.rb
@@ -65,6 +65,11 @@ module ActionDispatch
@tempfile.path
end
+ # Shortcut for +tempfile.to_path+.
+ def to_path
+ @tempfile.to_path
+ end
+
# Shortcut for +tempfile.rewind+.
def rewind
@tempfile.rewind
diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb
index 5a584b12e5..21169fcb5c 100644
--- a/actionpack/test/dispatch/uploaded_file_test.rb
+++ b/actionpack/test/dispatch/uploaded_file_test.rb
@@ -103,6 +103,12 @@ module ActionDispatch
assert_predicate 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
+ end
+
def test_respond_to?
tf = Class.new { def read; yield end }
uf = Http::UploadedFile.new(tempfile: tf.new)