Skip to content

Commit

Permalink
Merge branch '3-1-stable-sec' into 3-1-stable
Browse files Browse the repository at this point in the history
* 3-1-stable-sec:
  Strip [nil] from parameters hash. Thanks to Ben Murphy for reporting this!
  predicate builder should not recurse for determining where columns. Thanks to Ben Murphy for reporting this
  • Loading branch information
tenderlove committed May 31, 2012
2 parents 2f42815 + 5b83bbf commit aa6e56b
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 5 deletions.
22 changes: 22 additions & 0 deletions actionpack/lib/action_dispatch/http/request.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -267,6 +267,28 @@ def local?
LOCALHOST.any? { |local_ip| local_ip === remote_addr && local_ip === remote_ip } LOCALHOST.any? { |local_ip| local_ip === remote_addr && local_ip === remote_ip }
end end


protected

# Remove nils from the params hash
def deep_munge(hash)
hash.each_value do |v|
case v
when Array
v.grep(Hash) { |x| deep_munge(x) }
when Hash
deep_munge(v)
end
end

keys = hash.keys.find_all { |k| hash[k] == [nil] }
keys.each { |k| hash[k] = nil }
hash
end

def parse_query(qs)
deep_munge(super)
end

private private


def check_method(name) def check_method(name)
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -81,7 +81,12 @@ def teardown
end end


test "query string without equal" do test "query string without equal" do
assert_parses({ "action" => nil }, "action") assert_parses({"action" => nil}, "action")
assert_parses({"action" => {"foo" => nil}}, "action[foo]")
assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar]")
assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar][]")
assert_parses({"action" => {"foo" => nil}}, "action[foo][]")
assert_parses({"action"=>{"foo"=>[{"bar"=>nil}]}}, "action[foo][][bar]")
end end


test "query string with empty key" do test "query string with empty key" do
Expand Down
17 changes: 16 additions & 1 deletion activerecord/lib/active_record/associations/association_scope.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def add_constraints(scope)


conditions.each do |condition| conditions.each do |condition|
if options[:through] && condition.is_a?(Hash) if options[:through] && condition.is_a?(Hash)
condition = { table.name => condition } condition = disambiguate_condition(table, condition)
end end


scope = scope.where(interpolate(condition)) scope = scope.where(interpolate(condition))
Expand Down Expand Up @@ -126,6 +126,21 @@ def table_name_for(reflection)
end end
end end


def disambiguate_condition(table, condition)
if condition.is_a?(Hash)
Hash[
condition.map do |k, v|
if v.is_a?(Hash)
[k, v]
else
[table.table_alias || table.name, { k => v }]
end
end
]
else
condition
end
end
end end
end end
end end
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/predicate_builder.rb
Original file line number Original file line Diff line number Diff line change
@@ -1,16 +1,16 @@
module ActiveRecord module ActiveRecord
class PredicateBuilder # :nodoc: class PredicateBuilder # :nodoc:
def self.build_from_hash(engine, attributes, default_table) def self.build_from_hash(engine, attributes, default_table, check_column = true)
predicates = attributes.map do |column, value| predicates = attributes.map do |column, value|
table = default_table table = default_table


if value.is_a?(Hash) if value.is_a?(Hash)
table = Arel::Table.new(column, engine) table = Arel::Table.new(column, engine)
build_from_hash(engine, value, table) build_from_hash(engine, value, table, false)
else else
column = column.to_s column = column.to_s


if column.include?('.') if check_column && column.include?('.')
table_name, column = column.split('.', 2) table_name, column = column.split('.', 2)
table = Arel::Table.new(table_name, engine) table = Arel::Table.new(table_name, engine)
end end
Expand Down
19 changes: 19 additions & 0 deletions activerecord/test/cases/relation/where_test.rb
Original file line number Original file line Diff line number Diff line change
@@ -0,0 +1,19 @@
require "cases/helper"
require 'models/post'

module ActiveRecord
class WhereTest < ActiveRecord::TestCase
fixtures :posts

def test_where_error
assert_raises(ActiveRecord::StatementInvalid) do
Post.where(:id => { 'posts.author_id' => 10 }).first
end
end

def test_where_with_table_name
post = Post.first
assert_equal post, Post.where(:posts => { 'id' => post.id }).first
end
end
end

0 comments on commit aa6e56b

Please sign in to comment.