Switching to Rack possibly breaks user code dealing with file uploads

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.

Do you think this should be handled in Rails or Rack?

In the meantime, I’ve monkeypatched our app: http://gist.github.com/49519

I'm trying to make Rails 2.3 w/ Rack as backwords compat with the CGI
API. So this is definitely something we need to fix in Rails.

Please create a ticket on LH with some tests (so we can catch this
issue if it happens again) and I'll pull it in ASAP.

Thanks for finding this.

In my workaround I detect there was no file if the filename property is blank. Are there cases a valid file could be uploaded without an original filename? Is a better check to see if the size of Tempfile is 0, also?

I think a filename is always provided. Could probably check
params[:tempfile].length as you suggested too.

hrm, maybe this is a Rack issue too :slight_smile:
I don't think Rack's multipart parser should be creating empty tempfiles.

Ticket created http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1785-empty-file-uploads-should-not-come-through-as-empty-tempfiles

I would like to ask someone to create a failing tests for this. I didn’t figure out how to simulate a browser submitting an empty form field – I don’t even know what it sends. An empty byte stream with Content-Type: application/octet-stream; Content-Transfer-Encoding: binary? I tried, but ordinary String comes through instead of Tempfile.

Applied the fix to Rails.

I also submitted this patch to Rack:
http://rack.lighthouseapp.com/projects/22435/tickets/