diff options
Diffstat (limited to 'activerecord')
6 files changed, 14 insertions, 13 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 573b7512a9..c0bef5fb46 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* Change behaviour with empty array in where clause, + the SQL generated when when were passed an empty array was insecure in some cases + + Roberto Miranda + * Raise ArgumentError instead of generate invalid SQL when empty hash is used in where clause value Roberto Miranda diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index d1458f30ba..2099effbeb 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -136,7 +136,7 @@ module ActiveRecord records = load_target if records == :all scope = through_association.scope - scope.where! construct_join_attributes(*records) + scope.where! construct_join_attributes(*records) unless records.empty? case method when :destroy diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 24cbae83de..db2588e725 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -17,6 +17,8 @@ module ActiveRecord queries.concat expand(association && association.klass, table, k, v) end end + elsif value.is_a?(Array) && value.empty? + raise ArgumentError, "Condition value in SQL clause can't be an empty array" else column = column.to_s diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index a9fa107749..683e4fbd97 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -808,15 +808,6 @@ class FinderTest < ActiveRecord::TestCase assert_equal [2, 1].sort, client_of.compact.sort end - def test_find_with_nil_inside_set_passed_for_attribute - client_of = Company.all.merge!( - :where => { :client_of => [nil] }, - :order => 'client_of DESC' - ).map { |x| x.client_of } - - assert_equal [], client_of.compact - end - def test_with_limiting_with_custom_select posts = Post.references(:authors).merge( :includes => :author, :select => ' posts.*, authors.id as "author_id"', diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index 8c8f4267a9..ccbfcc0549 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -98,7 +98,9 @@ module ActiveRecord end def test_where_with_table_name_and_empty_array - assert_equal 0, Post.where(:id => []).count + assert_raises(ArgumentError) do + Post.where(:id => []) + end end def test_where_with_empty_hash_and_no_foreign_key diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 379c0c0758..981aa14e98 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -515,8 +515,9 @@ class RelationTest < ActiveRecord::TestCase end def test_find_in_empty_array - authors = Author.all.where(:id => []) - assert authors.to_a.blank? + assert_raises(ArgumentError) do + Author.all.where(:id => []) + end end def test_where_with_ar_object |