diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 48 | ||||
| -rwxr-xr-x | activerecord/test/base_test.rb | 18 | ||||
| -rw-r--r-- | railties/environments/production.rb | 3 | ||||
| -rw-r--r-- | railties/lib/initializer.rb | 18 |
5 files changed, 75 insertions, 14 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 391c1273f3..d75e719d4e 100644 --- a/activerecord/CHANGELOG +++ b/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] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 068d9a5c45..8b562e2ffb 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -35,10 +35,11 @@ module ActiveRecord #: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 @@ module ActiveRecord #: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 @@ module ActiveRecord #: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 @@ module ActiveRecord #: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 @@ module ActiveRecord #:nodoc: 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? diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb index 53ef4a5e86..650d87f91f 100755 --- a/activerecord/test/base_test.rb +++ b/activerecord/test/base_test.rb @@ -67,6 +67,13 @@ end 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 @@ class BasicsTest < Test::Unit::TestCase 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 diff --git a/railties/environments/production.rb b/railties/environments/production.rb index cb295b83f1..415a058f4c 100644 --- a/railties/environments/production.rb +++ b/railties/environments/production.rb @@ -16,3 +16,6 @@ config.action_controller.perform_caching = true # 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 diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 0256ba96b2..b2a405d268 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -72,6 +72,7 @@ module Rails # * #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 @@ module Rails initialize_framework_views initialize_dependency_mechanism initialize_whiny_nils + initialize_whiny_protected_attributes initialize_temporary_directories initialize_framework_settings @@ -298,6 +300,12 @@ module Rails 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 @@ module Rails # 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 @@ module Rails 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 @@ module Rails def default_whiny_nils false end + + def default_whiny_protected_attributes + true + end def default_plugins nil |