We build Web & Mobile Applications.
I’ve said it before and I’ll say it again: attachment_fu is a great Rails plugin! If you’ve never used it before then Mike Clark’s tutorial is the place to start. However there is one problem with attachment_fu that has been bugging me lately and it relates to mass-assignment.
Consider the following migration, model, view and controller:
class CreateMugshots < ActiveRecord::Migration
def self.up
create_table :mugshots do |t|
t.column :parent_id, :integer
t.column :content_type, :string
t.column :filename, :string
t.column :thumbnail, :string
t.column :size, :integer
t.column :width, :integer
t.column :height, :integer
end
end
def self.down
drop_table :mugshots
end
end
class Mugshot < ActiveRecord::Base
has_attachment :content_type => :image,
:storage => :file_system,
:max_size => 500.kilobytes,
:resize_to => '320x200>',
:thumbnails => { :thumb => '100x100>' }
validates_as_attachment
end
<%= error_messages_for :mugshot %>
<% form_for(:mugshot, :url => mugshots_path,
:html => { :multipart => true }) do |f| -%>
<p>
<label for="mugshot_uploaded_data">Upload A Mugshot:</label>
<%= f.file_field :uploaded_data %>
</p>
<p>
<%= submit_tag 'Create' %>
</p>
<% end -%>
class MugshotsController < ApplicationController
def new
@mugshot = Mugshot.new
end
def create
@mugshot = Mugshot.new(params[:mugshot])
if @mugshot.save
flash[:notice] = 'Mugshot was successfully created.'
redirect_to mugshot_url(@mugshot)
else
render :action => :new
end
end
end
The first thing you might think is that I’ve ripped-off re-used Mike’s code. The second thing you might think is to ask yourself what would happen if we changed the view to look like this:
<%= error_messages_for :mugshot %>
<% form_for(:mugshot, :url => mugshots_path,
:html => { :multipart => true }) do |f| -%>
<p>
<label for="mugshot_uploaded_data">Upload A Mugshot:</label>
<%= f.file_field :uploaded_data %>
</p>
<p>
<label for="mugshot_parent_id">Parent ID:</label>
<%= f.text_field :parent_id %>
</p>
<p>
<label for="mugshot_thumbnail">Thumbnail:</label>
<%= f.text_field :thumbnail %>
</p>
<p>
<%= submit_tag 'Create' %>
</p>
<% end -%>
The answer is simple: when the form is submitted the user entered values for parent_id and thumbnail will be stored in the database. If in our model we hadn’t limited the attachment content types (using :content_type => :image) then it would also have been possible to allow the user to enter a value in the form for that too.
Now in the real world you won’t want your users entering data for these attributes so you naturally won’t have form fields for them. The problem is that if some loser is determined to cause you headaches they’ll be able to take advantage of this loop-hole regardless of what fields you put on your form.
If you’ve been using Rails for a while or have recently watched Ryan Bates’ Hackers love mass assignment RailsCast then you’ll probably be thinking it’s time to add attr_accessible to your model so that only the uploaded_data attribute can be set by mass-assignment, like so:
class Mugshot < ActiveRecord::Base
has_attachment :content_type => :image,
:storage => :file_system,
:max_size => 500.kilobytes,
:resize_to => '320x200>',
:thumbnails => { :thumb => '100x100>' }
validates_as_attachment
attr_accessible :uploaded_data
end
If you now try submitting a new mugshot you’ll see this:
ActiveRecord::RecordInvalid in MugshotsController#create
Validation failed: Content type is not included in the list, Content type can't be blank, Size is not included in the list, Size can't be blank, Filename can't be blank
The stack trace that accompanies the error message shows the problem is occurring in the create_or_update_thumbnail method within the attachment_fu code. If you open up the source code you’ll see the method is defined like this:
def create_or_update_thumbnail(temp_file, file_name_suffix, *size)
thumbnailable? || raise(ThumbnailError.new("Can't create a thumbnail if the content type is not an image or there is no parent_id column"))
returning find_or_initialize_thumbnail(file_name_suffix) do |thumb|
thumb.attributes = {
:content_type => content_type,
:filename => thumbnail_name_for(file_name_suffix),
:temp_path => temp_file,
:thumbnail_resize_options => size
}
callback_with_args :before_thumbnail_saved, thumb
thumb.save!
end
end
See the line that starts with thumb.attributes? That’s mass-assignment, the very thing we’ve just prevented with our attr_accessible line in the model.
This is something that can take a while to sink in when you first start using Rails: if you use attr_accessible (or attr_protected) to prevent mass-assignment you need to remember that it doesn’t just apply when you pass the params hash to a constructor or to the update_attributes method in your controller, it applies to all mass-assignments.
The good news is we can easily fix attachment_fu to work with attr_accesible like this:
def create_or_update_thumbnail(temp_file, file_name_suffix, *size)
thumbnailable? || raise(ThumbnailError.new("Can't create a thumbnail if the content type is not an image or there is no parent_id column"))
returning find_or_initialize_thumbnail(file_name_suffix) do |thumb|
thumb.content_type = content_type
thumb.filename = thumbnail_name_for(file_name_suffix)
thumb.temp_path = temp_file
thumb.thumbnail_resize_options = size
callback_with_args :before_thumbnail_saved, thumb
thumb.save!
end
end
You can download a patch for revision 3145 of attachment_fu (don’t forget to restart your server after applying it) and I’ve also submitted a ticket to the attachment_fu tracker.
This change will not work if you’re not on Rails 2.0 or newer. This is because the dynamic ActiveRecord finders used by attachment_fu (such as find_or_initialize_by_thumbnail_and_parent_id) use mass-assignment too. This results in a nasty situation where attachment_fu gets stuck in a loop trying to create thumbnails of thumbnails. This was fixed in Rails changeset 7826.
For completeness I should also mention another approach you could take instead of using attr_accessible: you could choose to sanitise your params hash instead, manually filtering out values that you don’t want to assign to an object. If this sounds more like your cup of tea take a look at this blog entry which contains some alternative approaches to handling mass-assignment.