Skip to content

Commit

Permalink
Ordering uses information from relfection instead of hash, fixes bug …
Browse files Browse the repository at this point in the history
…for alternate table names
  • Loading branch information
binarylogic committed Sep 19, 2008
1 parent 2f701cf commit 744bad1
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 9 deletions.
12 changes: 7 additions & 5 deletions lib/searchgasm/search/ordering.rb
Expand Up @@ -115,18 +115,20 @@ def order_by_includes

private
def order_by_to_order(order_by, order_as, alt_klass = nil, new_includes = [])
table_name = (alt_klass || klass).table_name
k = alt_klass || klass
table_name = k.table_name
sql_parts = []

case order_by
when Array
order_by.each { |part| sql_parts << order_by_to_order(part, order_as) }
when Hash
raise(ArgumentError, "when passing a hash to order_by you must only have 1 key: {:user_group => :name} not {:user_group => :name, :user_group => :id}. The latter should be [{:user_group => :name}, {:user_group => :id}]") if order_by.keys.size != 1
k = order_by.keys.first
v = order_by.values.first
new_includes << k.to_sym
sql_parts << order_by_to_order(v, order_as, eval(k.to_s.classify), new_includes) # using eval, better performance, protection makes sure nothing fishy goes on here
key = order_by.keys.first
reflection = k.reflect_on_association(key.to_sym)
value = order_by.values.first
new_includes << key.to_sym
sql_parts << order_by_to_order(value, order_as, reflection.klass, new_includes) # using eval, better performance, protection makes sure nothing fishy goes on here
when Symbol, String
new_include = build_order_by_includes(new_includes)
self.order_by_includes << new_include if new_include
Expand Down
9 changes: 5 additions & 4 deletions lib/searchgasm/search/protection.rb
Expand Up @@ -26,7 +26,7 @@ module Protection

VULNERABLE_FIND_OPTIONS = Base::AR_FIND_OPTIONS - SAFE_OPTIONS

VULERNABLE_CALCULATIONS_OPTIONS = Base::AR_CALCULATIONS_OPTIONS - SAFE_OPTIONS
VULNERABLE_CALCULATIONS_OPTIONS = Base::AR_CALCULATIONS_OPTIONS - SAFE_OPTIONS

# Options that are not allowed, at all, when protecting against SQL injections
VULNERABLE_OPTIONS = Base::OPTIONS - SAFE_OPTIONS
Expand Down Expand Up @@ -63,12 +63,13 @@ def order_by_safe?(order_by, alt_klass = nil)

k = alt_klass || klass
column_names = k.column_names

[order_by].flatten.each do |column|
case column
when Hash
return false unless k.reflect_on_association(column.keys.first.to_sym)
return false unless order_by_safe?(column.values.first, column.keys.first.to_s.classify.constantize)
reflection = k.reflect_on_association(column.keys.first.to_sym)
return false unless reflection
return false unless order_by_safe?(column.values.first, reflection.klass)
when Array
return false unless order_by_safe?(column)
else
Expand Down
5 changes: 5 additions & 0 deletions test/test_active_record_associations.rb
Expand Up @@ -58,4 +58,9 @@ def test_has_many_through
def test_habtm

end

def test_alternate_class_name
search = Account.find(1).admin.new_search
assert_equal User, search.klass
end
end
2 changes: 2 additions & 0 deletions test/test_helper.rb
Expand Up @@ -9,6 +9,7 @@
ActiveRecord::Base.establish_connection(:adapter => "sqlite3", :dbfile => ":memory:")

class Account < ActiveRecord::Base
has_one :admin, :class_name => "User", :conditions => {:first_name => "Ben"}
has_many :users, :dependent => :destroy
has_many :orders, :through => :users
end
Expand All @@ -17,6 +18,7 @@ class User < ActiveRecord::Base
acts_as_tree
belongs_to :account
has_many :orders, :dependent => :destroy
has_one :cool_order, :class_name => "Order", :conditions => {:total => 100}
end

class Order < ActiveRecord::Base
Expand Down

0 comments on commit 744bad1

Please sign in to comment.