Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Raise ProtectedAttributeAssignmentError in development and test envir…

…onments when mass-assigning to an attr_protected attribute. Closes #9699.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7777 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 84a14f262031d5081e34559bb1ba52e75b05afb4 1 parent d9f3c43
@jeremy jeremy authored
View
2  activerecord/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Raise ProtectedAttributeAssignmentError in development and test environments when mass-assigning to an attr_protected attribute. #9699 [Henrik N]
+
* MySQL: speedup date/time parsing. [Jeremy Kemper]
* Fix calling .clear on a has_many :dependent=>:delete_all association. [tarmo]
View
48 activerecord/lib/active_record/base.rb
@@ -35,10 +35,11 @@ class ReadOnlyRecord < ActiveRecordError #:nodoc:
end
class Rollback < ActiveRecordError #:nodoc:
end
-
+ class ProtectedAttributeAssignmentError < ActiveRecordError #:nodoc:
+ end
class DangerousAttributeError < ActiveRecordError #:nodoc:
end
-
+
# Raised when you've tried to access a column, which wasn't
# loaded by your finder. Typically this is because :select
# has been specified
@@ -327,13 +328,13 @@ def self.reset_subclasses #:nodoc:
cattr_accessor :table_name_suffix, :instance_writer => false
@@table_name_suffix = ""
- # Indicates whether or not table names should be the pluralized versions of the corresponding class names.
+ # Indicates whether table names should be the pluralized versions of the corresponding class names.
# If true, the default table name for a +Product+ class will be +products+. If false, it would just be +product+.
# See table_name for the full rules on table/class naming. This is true, by default.
cattr_accessor :pluralize_table_names, :instance_writer => false
@@pluralize_table_names = true
- # Determines whether or not to use ANSI codes to colorize the logging statements committed by the connection adapter. These colors
+ # Determines whether to use ANSI codes to colorize the logging statements committed by the connection adapter. These colors
# make it much easier to overview things during debugging (when used through a reader like +tail+ and on a black background), but
# may complicate matters if you use software like syslog. This is true, by default.
cattr_accessor :colorize_logging, :instance_writer => false
@@ -344,7 +345,7 @@ def self.reset_subclasses #:nodoc:
cattr_accessor :default_timezone, :instance_writer => false
@@default_timezone = :local
- # Determines whether or not to use a connection for each thread, or a single shared connection for all threads.
+ # Determines whether to use a connection for each thread, or a single shared connection for all threads.
# Defaults to false. Set to true if you're writing a threaded application.
cattr_accessor :allow_concurrency, :instance_writer => false
@@allow_concurrency = false
@@ -358,6 +359,11 @@ def self.reset_subclasses #:nodoc:
cattr_accessor :schema_format , :instance_writer => false
@@schema_format = :ruby
+ # Determines whether to raise an exception on mass-assignment to protected
+ # attribute. Defaults to true.
+ cattr_accessor :whiny_protected_attributes, :instance_writer => false
+ @@whiny_protected_attributes = true
+
class << self # Class methods
# Find operates with three different retrieval approaches:
#
@@ -2022,17 +2028,31 @@ def convert_number_column_value(value)
end
def remove_attributes_protected_from_mass_assignment(attributes)
- if self.class.accessible_attributes.nil? && self.class.protected_attributes.nil?
- attributes.reject { |key, value| attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) }
- elsif self.class.protected_attributes.nil?
- attributes.reject { |key, value| !self.class.accessible_attributes.include?(key.gsub(/\(.+/, "").intern) || attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) }
- elsif self.class.accessible_attributes.nil?
- attributes.reject { |key, value| self.class.protected_attributes.include?(key.gsub(/\(.+/,"").intern) || attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) }
- else
- raise "Declare either attr_protected or attr_accessible for #{self.class}, but not both."
+ safe_attributes =
+ if self.class.accessible_attributes.nil? && self.class.protected_attributes.nil?
+ attributes.reject { |key, value| attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) }
+ elsif self.class.protected_attributes.nil?
+ attributes.reject { |key, value| !self.class.accessible_attributes.include?(key.gsub(/\(.+/, "").intern) || attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) }
+ elsif self.class.accessible_attributes.nil?
+ attributes.reject { |key, value| self.class.protected_attributes.include?(key.gsub(/\(.+/,"").intern) || attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) }
+ else
+ raise "Declare either attr_protected or attr_accessible for #{self.class}, but not both."
+ end
+
+ removed_attributes = attributes.keys - safe_attributes.keys
+
+ if removed_attributes.any?
+ error_message = "Can't mass-assign these protected attributes: #{removed_attributes.join(', ')}"
+ if self.class.whiny_protected_attributes
+ raise ProtectedAttributeAssignmentError, error_message
+ else
+ logger.error error_message
+ end
end
+
+ safe_attributes
end
-
+
# Removes attributes which have been marked as readonly.
def remove_readonly_attributes(attributes)
unless self.class.readonly_attributes.nil?
View
18 activerecord/test/base_test.rb
@@ -67,6 +67,13 @@ class TopicWithProtectedContentAndAccessibleAuthorName < ActiveRecord::Base
class BasicsTest < Test::Unit::TestCase
fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics
+ # whiny_protected_attributes is turned off since several tests were
+ # not written with it in mind, and would otherwise raise exceptions
+ # as an irrelevant side-effect.
+ def setup
+ ActiveRecord::Base.whiny_protected_attributes = false
+ end
+
def test_table_exists
assert !NonExistentTable.table_exists?
assert Topic.table_exists?
@@ -860,6 +867,17 @@ def test_mass_assignment_protection_inheritance
assert_equal [ :name, :address, :phone_number ], TightDescendant.accessible_attributes
end
+ def test_whiny_protected_attributes
+ ActiveRecord::Base.whiny_protected_attributes = true
+ assert_raise(ActiveRecord::ProtectedAttributeAssignmentError) do
+ LoosePerson.create!(:administrator => true)
+ end
+ ActiveRecord::Base.whiny_protected_attributes = false
+ assert_nothing_raised do
+ LoosePerson.create!(:administrator => true)
+ end
+ end
+
def test_readonly_attributes
assert_equal [ :title ], ReadonlyTitlePost.readonly_attributes
View
3  railties/environments/production.rb
@@ -16,3 +16,6 @@
# Disable delivery errors, bad email addresses will be ignored
# config.action_mailer.raise_delivery_errors = false
+
+# Disable raising errors when mass-assigning to a protected attribute
+config.whiny_protected_attributes = false
View
18 railties/lib/initializer.rb
@@ -72,6 +72,7 @@ def initialize(configuration)
# * #initialize_framework_views
# * #initialize_dependency_mechanism
# * #initialize_whiny_nils
+ # * #initialize_whiny_protected_attributes
# * #initialize_temporary_directories
# * #initialize_framework_settings
# * #add_support_load_paths
@@ -95,6 +96,7 @@ def process
initialize_framework_views
initialize_dependency_mechanism
initialize_whiny_nils
+ initialize_whiny_protected_attributes
initialize_temporary_directories
initialize_framework_settings
@@ -298,6 +300,12 @@ def initialize_dependency_mechanism
def initialize_whiny_nils
require('active_support/whiny_nil') if configuration.whiny_nils
end
+
+ # Sets +ActiveRecord::Base#whiny_protected_attributes+ which determines whether to
+ # raise on mass-assigning attributes protected with +attr_protected+/+attr_accessible+.
+ def initialize_whiny_protected_attributes
+ ActiveRecord::Base.whiny_protected_attributes = configuration.whiny_protected_attributes
+ end
def initialize_temporary_directories
if configuration.frameworks.include?(:action_controller)
@@ -423,6 +431,11 @@ class Configuration
# Set to +true+ if you want to be warned (noisily) when you try to invoke
# any method of +nil+. Set to +false+ for the standard Ruby behavior.
attr_accessor :whiny_nils
+
+ # The default value of +true+ means an exception will be raised on attempts
+ # to mass-assign to protected attributes. Set to +false+ to discard them
+ # without raising (an error will be logged instead).
+ attr_accessor :whiny_protected_attributes
# The list of plugins to load. If this is set to <tt>nil</tt>, all plugins will
# be loaded. If this is set to <tt>[]</tt>, no plugins will be loaded. Otherwise,
@@ -471,6 +484,7 @@ def initialize
self.controller_paths = default_controller_paths
self.cache_classes = default_cache_classes
self.whiny_nils = default_whiny_nils
+ self.whiny_protected_attributes = default_whiny_protected_attributes
self.plugins = default_plugins
self.plugin_paths = default_plugin_paths
self.plugin_locators = default_plugin_locators
@@ -630,6 +644,10 @@ def default_cache_classes
def default_whiny_nils
false
end
+
+ def default_whiny_protected_attributes
+ true
+ end
def default_plugins
nil
Please sign in to comment.
Something went wrong with that request. Please try again.