We build Web & Mobile Applications.

< All Articles

Hackers love attachment_fu?

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:

Migration: 001_create_mugshots.rb

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

Model: mugshot.rb

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

View: new.html.erb

<%= 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 -%>

Controller: mugshots_controller.rb

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.

Important!

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.

Updated on 07 February 2019
First published by Rob Anderton on 03 March 2008
© Rob Anderton 2019
"Hackers love attachment_fu?" by Rob Anderton at TheWebFellas is licensed under a Creative Commons Attribution 4.0 International License.