This project is archived and is in readonly mode.
Empty file uploads should not come through as empty Tempfiles
Reported by Mislav | January 20th, 2009 @ 05:18 PM
We're using Paperclip for file uploads and recent edge Rails rendered our user profile forms unusable.
File uploads don't break our application when there was an actual file upload; what breaks Paperclip is the case when nothing was selected in the file input. The form is still sent with multipart encoding and parsed by Rack, which creates a Tempfile regardless of whether some data was received or not.
The result of Rack processing a single file field is a hash with these keys: {:filename, :type, :name, :tempfile, :head}.
Rails further processes this in ActionController::UrlEncodedPairParser.get_typed_value. When it sees the above formatted hash, it replaces it with the Tempfile object it references and applies other metadata, like filename, as properties of this object.
In short, when a "user[avatar]" file field was sent empty, older Rails version would receive nothing:
params[:user][:avatar] # => nil
Now a Tempfile is received in any case:
params[:user][:avatar] # => #<File:/tmp/RackMultipart.xxxyyy>
So naturally Paperclip thinks a file was uploaded and explodes
because this object has nil value original_filename
and a size of 0.
A solution is to check for Tempfile size and not let it through if it's zero.
Comments and changes to this ticket
-
josh January 20th, 2009 @ 05:31 PM
- Milestone cleared.
- State changed from new to open
- Assigned user set to josh
-
Xavier Noria January 20th, 2009 @ 05:53 PM
An uploaded file could have size 0... Wouldn't be possible to test something else like value[:filename].nil? ?
Not really into it so don't know if it is a valid alternative indeed.
-
Mislav January 20th, 2009 @ 06:38 PM
- Assigned user cleared.
Theoretically it could, but what good is storing a 0-byte file? I say ignore it
-
Repository January 20th, 2009 @ 06:40 PM
- State changed from open to resolved
(from [01f06fc7f4dda52035d5a2273d402d8555a897a5]) Don't let empty Tempfiles come through as uploaded files [#1785 state:resolved]
Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...
-
Xavier Noria January 20th, 2009 @ 06:45 PM
I think it doesn't matter if you cannot come up with a use case if you can be correct by the same price.
The patch should test whether a file was uploaded, and it doesn't test that. That's my point. If it could it should.
-
josh January 20th, 2009 @ 08:49 PM
- Assigned user set to josh
I created the test fixture by dumping the raw POST body from an empty file upload in Safari.
http://github.com/rails/rails/bl...
Am I missing something else?
-
Mislav January 20th, 2009 @ 10:54 PM
Xavier, would you say that this is a better test for when a file was not uploaded?
if value.has_key?(:tempfile) and not (value[:tempfile].size == 0 and value[:filename].blank?)
This check allows 0-sized files to be uploaded if they have a filename.
-
josh January 21st, 2009 @ 12:05 AM
- State changed from resolved to open
Now I'm confused? Xavier, I guess you were referring the to test conditional not test fixture :)
After some more testing, it seems like we could just check for value[:filename].blank? This always seems to be the case when the user does not select a file. This should allow users to upload a text file with no contents.
-
Xavier Noria January 21st, 2009 @ 12:33 AM
Exactly, my untested guess is that to test if the file was uploaded you just check value[:filename].blank?.
If that's correct checking the size would not be necessary.
-
josh January 21st, 2009 @ 02:36 AM
- State changed from open to resolved
I think this is the fix we wanted
-
MatthewRudy January 21st, 2009 @ 10:14 AM
Great
Thanks.
I hit this problem on Monday, but didn't have time to debug it.
Nice one.
-
Thijs February 14th, 2009 @ 05:11 PM
I'm stil seeing the first reported behaviour with Paperclip master and Rails master. Am I missing something?
-
Mislav February 15th, 2009 @ 11:46 AM
I didn't experience it anymore after the fix.
The best bet is to test this in your browser: submit a form with empty file field and inspect the parameters you've got in the controller. Is it a Tempfile? If so, can you inspect its properties like
original_filename
andsize
?
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 1785 Empty file uploads should not come through as empty Tempfiles (from [01f06fc7f4dda52035d5a2273d402d8555a897a5]) Don't l...