Merged pull request #308 from joshk/assign_attributes.

deprecated the use of guard_protected_attributes with attributes= in AR

3 files changed, 22 insertions, 11 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4512e8c8ad..04c12f86b6 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1621,11 +1621,11 @@ end
       # Allows you to set all the attributes at once by passing in a hash with keys
       # matching the attribute names (which again matches the column names).
       #
-      # If +guard_protected_attributes+ is true (the default), then sensitive
-      # attributes can be protected from this form of mass-assignment by using
-      # the +attr_protected+ macro. Or you can alternatively specify which
-      # attributes *can* be accessed with the +attr_accessible+ macro. Then all the
-      # attributes not included in that won't be allowed to be mass-assigned.
+      # If any attributes are protected by either +attr_protected+ or
+      # +attr_accessible+ then only settable attributes will be assigned.
+      #
+      # The +guard_protected_attributes+ argument is now deprecated, use
+      # the +assign_attributes+ method if you want to bypass mass-assignment security.
       #
       #   class User < ActiveRecord::Base
       #     attr_protected :is_admin
@@ -1635,11 +1635,16 @@ end
       #   user.attributes = { :username => 'Phusion', :is_admin => true }
       #   user.username   # => "Phusion"
       #   user.is_admin?  # => false
-      #
-      #   user.send(:attributes=, { :username => 'Phusion', :is_admin => true }, false)
-      #   user.is_admin?  # => true
-      def attributes=(new_attributes, guard_protected_attributes = true)
+      def attributes=(new_attributes, guard_protected_attributes = nil)
+        unless guard_protected_attributes.nil?
+          message = "the use of 'guard_protected_attributes' will be removed from the next major release of rails, " +
+                    "if you want to bypass mass-assignment security then look into using assign_attributes"
+          ActiveSupport::Deprecation.warn(message)
+        end
+
         return unless new_attributes.is_a?(Hash)
+
+        guard_protected_attributes ||= true
         if guard_protected_attributes
           assign_attributes(new_attributes)
         else
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index a3d4b7f45a..57c9921ea8 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -279,8 +279,8 @@ module ActiveRecord
 
       unless record
         record = @klass.new do |r|
-          r.send(:attributes=, protected_attributes_for_create, true) unless protected_attributes_for_create.empty?
-          r.send(:attributes=, unprotected_attributes_for_create, false) unless unprotected_attributes_for_create.empty?
+          r.assign_attributes(protected_attributes_for_create)
+          r.assign_attributes(unprotected_attributes_for_create, :without_protection => true)
         end
         yield(record) if block_given?
         record.save if match.instantiator == :create
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index ef833857ce..5ee3b2d776 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -489,6 +489,12 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal 'value2', weird.send('a$b')
   end
 
+  def test_attributes_guard_protected_attributes_is_deprecated
+    attributes = { "title" => "An amazing title" }
+    topic = Topic.new
+    assert_deprecated { topic.send(:attributes=, attributes, false) }
+  end
+
   def test_multiparameter_attributes_on_date
     attributes = { "last_read(1i)" => "2004", "last_read(2i)" => "6", "last_read(3i)" => "24" }
     topic = Topic.find(1)