You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Attaching an unlinked Tempfile using Attacher#attach fails with a TypeError.
Expected behavior
attach should attach the Tempfile.
Actual behavior
In extract_filename, Shrine passes the Tempfile's path, which is nil, to File::basename. basename fails because it doesn't expect nil.
When a Tempfile is unlinked, its path is set to nil, but its contents remain accessible. This is valid on POSIX systems and it's what Puma does for request bodies beyond a certain size. We're trying to attach request.body in an API handler.
For now, I've monkeypatched extract_filename like this. It seems to work, but I'm not sure if it's the optimal solution.
Simplest self-contained example code to demonstrate issue
require"sequel"require"shrine"require"shrine/storage/memory"require"down"require"bundler/setup"# if you want to debug shrine locallyrequire'minitest/autorun'# if you wanna use minitest# require 'byebug' ## if you're using byebug# byebugShrine.storages={cache: Shrine::Storage::Memory.new,store: Shrine::Storage::Memory.new,}Shrine.plugin:sequelclassMyUploader < Shrine# plugins and uploading logicendDB=Sequel.sqlite# SQLite memory databaseDB.create_table:postsdoprimary_key:idString:image_dataendclassPost < Sequel::ModelincludeMyUploader::Attachment(:image)endclassPostTest < Minitest::Testdeftest_linked_tempfile# This works.t=Tempfile.newattacher=MyUploader::Attacher.newattacher.attach(t)enddeftest_unlinked_tempfile# This fails.t=Tempfile.newt.unlinkattacher=MyUploader::Attacher.newattacher.attach(t)endend
PostTest#test_unlinked_tempfile:
TypeError: no implicit conversion of nil into String
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine.rb:256:in `basename'
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine.rb:256:in `extract_filename'
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine.rb:230:in `extract_metadata'
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine.rb:288:in `get_metadata'
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine.rb:207:in `upload'
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine.rb:108:in `upload'
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine/attacher.rb:182:in `upload'
/Users/jobe/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/shrine-3.4.0/lib/shrine/attacher.rb:117:in `attach'
sequel_template.rb:46:in `test_unlinked_tempfile'
System configuration
Ruby version: Reproduced on 2.7.5
Shrine version: 3.4.0
The text was updated successfully, but these errors were encountered:
That solution makes sense to me as a PR to shrine ... I think shrine can handle having no extracted filename already? @janko , good PR? @jonasoberschweiber , if the maintainers like it, are you interested in making a PR with a test?
Brief Description
Attaching an unlinked Tempfile using
Attacher#attach
fails with a TypeError.Expected behavior
attach
should attach the Tempfile.Actual behavior
In
extract_filename
, Shrine passes the Tempfile'spath
, which is nil, toFile::basename
.basename
fails because it doesn't expect nil.When a Tempfile is unlinked, its
path
is set to nil, but its contents remain accessible. This is valid on POSIX systems and it's what Puma does for request bodies beyond a certain size. We're trying to attachrequest.body
in an API handler.For now, I've monkeypatched
extract_filename
like this. It seems to work, but I'm not sure if it's the optimal solution.Simplest self-contained example code to demonstrate issue
System configuration
Ruby version: Reproduced on 2.7.5
Shrine version: 3.4.0
The text was updated successfully, but these errors were encountered: