Please provide a formal explanation of Rails 3.x clobbering behavior

rubyonrails-core
#1

Hi,

I’m sure you’re well aware of the email incident that GitHub experienced when upgrading to Rails 3.2.13

https://github.com/blog/1440-today-s-email-incident

A number of regressions were resolved, though some are likely to remain open to avoid symbol attacks while maintaining some semblance of backwards compatibility. There is quite a bit of discussion on issue #9813:

https://github.com/rails/rails/issues/9813

I’ve attempted to test the issue GitHub blogged about against multiple versions of Rails using Appraisal:

http://stackoverflow.com/questions/17867202/why-am-i-unable-to-reproduce-the-github-email-incident-issue

And we are currently doing a query-by-query comparison of Rails 3.2.12 and Rails 3.2.14 by logging all queries generated during a test suite run:

https://gist.github.com/nathany/6091516

Still we’re a little uncomfortable rolling 3.2.14 into production due to the crashes we had with 3.2.13, and I personally don’t feel like I fully understand all the issues.

I would really like to see a formal response to the GitHub email incident, preferably on the Rails blog. What is the clobbering behaviour? What actions cause an ActiveRelation merge? What should we avoid or watch out for?

Thanks,

Nathan.

#2

Agreed, it’s all a bit too spooky right now. I started compiling a table of different queries and their behaviour (clobber vs. append) between 3.2.12 and 3.2.14. I’m not totally there yet, but I’ll put it up when I think I have a good understanding so we can know exactly what tests we need to write. Here are two failing ones to start, based on pre-3.2.13 as the standard:

diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index f14eee2..93e0b61 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -378,6 +378,10 @@ class RelationTest < ActiveRecord::TestCase
assert_equal Developer.find_all_by_name('David').map(&:id).sort, developers
end
+ def test_default_scope_with_conditions_hash_conflict
+ assert_equal [], DeveloperCalledJamis.where(name: 'David').to_a
+ end