From 4506dd2f07be824fd7e0eb6165c29994aeb1bfcd Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Thu, 30 Jan 2014 22:58:00 -0200 Subject: [PATCH] Associations now raise `ArgumentError` on name conflicts. Dangerous association names conflicts include instance or class methods already defined by `ActiveRecord::Base`. --- activerecord/CHANGELOG.md | 16 ++++++++++++++++ .../associations/builder/association.rb | 6 ++++++ .../associations/belongs_to_associations_test.rb | 10 ++++++++++ .../associations/has_many_associations_test.rb | 10 ++++++++++ .../associations/has_one_associations_test.rb | 10 ++++++++++ 5 files changed, 52 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a6400a169b..f3322dd7f6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Associations now raise `ArgumentError` on name conflicts. + + Dangerous association names conflicts include instance or class methods already + defined by `ActiveRecord::Base`. + + Example: + + class Car < ActiveRecord::Base + has_many :errors + end + # Will raise ArgumentError. + + Fixes #13217. + + *Lauro Caetano* + * Fix regressions on `select_*` methods. When `select_*` methods receive a `Relation` object, they should be able to get the arel/binds from it. Also fix regressions on select_rows that was ignoring the binds. diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 3911d1b520..f085fd1cfd 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -26,6 +26,12 @@ module ActiveRecord::Associations::Builder attr_reader :name, :scope, :options def self.build(model, name, scope, options, &block) + if model.dangerous_attribute_method?(name) + raise ArgumentError, "You tried to define an association named #{name} on the model #{model.name}, but " \ + "this will conflict with a method #{name} already defined by Active Record. " \ + "Please choose a different association name." + end + builder = create_builder model, name, scope, options, &block reflection = builder.build(model) define_accessors model, reflection diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 2283ba66db..9340bc0a83 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -853,4 +853,14 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert post.save assert_equal post.author_id, author2.id end + + test 'dangerous association name raises ArgumentError' do + [:errors, 'errors', :save, 'save'].each do |name| + assert_raises(ArgumentError, "Association #{name} should not be allowed") do + Class.new(ActiveRecord::Base) do + belongs_to name + end + end + end + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index cf1e50890e..a86fb15719 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1820,4 +1820,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase topic.approved_replies.create! end end + + test 'dangerous association name raises ArgumentError' do + [:errors, 'errors', :save, 'save'].each do |name| + assert_raises(ArgumentError, "Association #{name} should not be allowed") do + Class.new(ActiveRecord::Base) do + has_many name + end + end + end + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index d4edef03d6..a4650ccdf2 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -564,4 +564,14 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end end end + + test 'dangerous association name raises ArgumentError' do + [:errors, 'errors', :save, 'save'].each do |name| + assert_raises(ArgumentError, "Association #{name} should not be allowed") do + Class.new(ActiveRecord::Base) do + has_one name + end + end + end + end end