From e8248124210a088280216c8099bc738d317f5332 Mon Sep 17 00:00:00 2001 From: Dan Barry Date: Wed, 16 Apr 2008 12:12:50 -0500 Subject: [PATCH] bounded parameters in find(...:from) --- activerecord/lib/active_record/associations.rb | 2 +- activerecord/lib/active_record/base.rb | 4 ++-- activerecord/test/cases/associations/eager_test.rb | 5 +++++ activerecord/test/cases/finder_test.rb | 9 +++++++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7d27b06..5b17888 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1407,7 +1407,7 @@ module ActiveRecord def construct_finder_sql_with_included_associations(options, join_dependency) scope = scope(:find) - sql = "SELECT #{column_aliases(join_dependency)} FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} " + sql = "SELECT #{column_aliases(join_dependency)} FROM #{(scope && sanitize_sql(scope[:from])) || sanitize_sql(options[:from]) || quoted_table_name} " sql << join_dependency.join_associations.collect{|join| join.association_join }.join add_joins!(sql, options, scope) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 8bef5ed..e0d5da6 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -463,7 +463,7 @@ module ActiveRecord #:nodoc: # * :select: By default, this is * as in SELECT * FROM, but can be changed if you, for example, want to do a join but not # include the joined columns. # * :from: By default, this is the table name of the class, but can be changed to an alternate table name (or even the name - # of a database view). + # of a database view). It can accept an array so that tainted parameters to database functions can be sanitized (i.e. ["posts_for(?) as posts", category_id]) # * :readonly: Mark the returned records read-only so they cannot be saved or updated. # * :lock: An SQL fragment like "FOR UPDATE" or "LOCK IN SHARE MODE". # :lock => true gives connection's default exclusive lock, usually "FOR UPDATE". @@ -1435,7 +1435,7 @@ module ActiveRecord #:nodoc: def construct_finder_sql(options) scope = scope(:find) sql = "SELECT #{(scope && scope[:select]) || options[:select] || (options[:joins] && quoted_table_name + '.*') || '*'} " - sql << "FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} " + sql << "FROM #{(scope && sanitize_sql(scope[:from])) || sanitize_sql(options[:from]) || quoted_table_name} " add_joins!(sql, options, scope) add_conditions!(sql, options[:conditions], scope) diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 0bc3454..854c92c 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -164,6 +164,11 @@ class EagerAssociationTest < ActiveRecord::TestCase end end + def test_eager_association_loading_with_belongs_to_and_from_array + post = Post.find(:first) + assert_equal Comment.find(:all, :include => :post, :conditions => ['posts.id = ?', post.id]), Comment.find(:all, :include => :post, :from => ['comments INNER JOIN SUBSTR(?, 2) baz ON true', 'foobar'], :conditions => ['posts.id = ? AND baz = ?', post.id, 'oobar']) + end + def test_eager_association_loading_with_belongs_to_and_order_string_with_unquoted_table_name assert_nothing_raised do Comment.find(:all, :include => :post, :order => 'posts.id') diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index b7f87fe..f7a7242 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -798,6 +798,15 @@ class FinderTest < ActiveRecord::TestCase assert developer_names.include?('Jamis') end + def test_find_all_using_array_from + assert_equal Developer.find(:all), Developer.find(:all, :from => ['developers, SUBSTR(?, 2) AS baz', 'foobar'], :conditions => ['baz = ?', 'oobar']) + end + + def test_find_all_with_associations_using_array_from + project = Project.find(:first) + assert_equal Developer.find(:all, :include => :projects, :conditions => ['projects.id = ?', project.id]), Developer.find(:all, :include => :projects, :from => ['developers INNER JOIN SUBSTR(?, 2) baz ON true', 'foobar'], :conditions => ['projects.id = ? AND baz = ?', project.id, 'oobar']) + end + def test_joins_dont_clobber_id first = Firm.find( :first, -- 1.5.4.2