From 4506dd2f07be824fd7e0eb6165c29994aeb1bfcd Mon Sep 17 00:00:00 2001
From: Lauro Caetano <laurocaetano1@gmail.com>
Date: Thu, 30 Jan 2014 22:58:00 -0200
Subject: 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 ++++++++++++++++
 .../active_record/associations/builder/association.rb    |  6 ++++++
 .../cases/associations/belongs_to_associations_test.rb   | 10 ++++++++++
 .../cases/associations/has_many_associations_test.rb     | 10 ++++++++++
 .../test/cases/associations/has_one_associations_test.rb | 10 ++++++++++
 5 files changed, 52 insertions(+)

(limited to 'activerecord')

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
-- 
cgit v1.2.3