Added named bind-style variable interpolation #281 [Michael Koziarski]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@78 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 60 insertions, 6 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index f4119046d5..ca16757ae7 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -21,6 +21,10 @@
 
 * Fixed the Inflector to handle the movie/movies pair correctly #261 [Scott Baron]
 
+* Added named bind-style variable interpolation #281 [Michael Koziarski]. Example:
+
+    Person.find(["id = :id and first_name = :first_name", { :id => 5, :first_name = "bob' or 1=1" }])
+
 * Added bind-style variable interpolation for the condition arrays that uses the adapter's quote method [Michael Koziarski]
 
   Before:
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index b876f83abb..cc76204dc0 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -26,6 +26,8 @@ module ActiveRecord #:nodoc:
   end
   class StatementInvalid < ActiveRecordError #:nodoc:
   end
+  class PreparedStatementInvalid < ActiveRecordError #:nodoc:
+  end
 
   # Active Record objects doesn't specify their attributes directly, but rather infer them from the table definition with
   # which they're linked. Adding, removing, and changing attributes and their type is done directly in the database. Any change
@@ -642,19 +644,46 @@ module ActiveRecord #:nodoc:
         def sanitize_conditions(conditions)
           return conditions unless conditions.is_a?(Array)
 
-          statement, values = conditions[0], conditions[1..-1]
+          statement, *values = conditions
 
-          statement =~ /\?/ ?
-            replace_bind_variables(statement, values) :
+          if values[0].is_a?(Hash) && statement =~ /:\w+/
+            replace_named_bind_variables(statement, values[0])
+          elsif statement =~ /\?/ 
+            replace_bind_variables(statement, values)
+          else
             statement % values.collect { |value| connection.quote_string(value.to_s) }
+          end
         end
 
         def replace_bind_variables(statement, values)
-          while statement =~ /\?/
+          orig_statement = statement.clone
+          expected_number_of_variables = statement.count('?')
+          provided_number_of_variables = values.size
+
+          unless expected_number_of_variables == provided_number_of_variables
+            raise PreparedStatementInvalid, "wrong number of bind variables (#{provided_number_of_variables} for #{expected_number_of_variables})"
+          end
+
+          until values.empty?
             statement.sub!(/\?/, connection.quote(values.shift))
           end
-	
-  		    return statement
+          
+          statement.gsub('?') { |all, match| connection.quote(values.shift) }
+        end
+
+        def replace_named_bind_variables(statement, values_hash)
+          orig_statement = statement.clone
+          values_hash.keys.each do |k|
+            if statement.sub!(/:#{k.id2name}/, connection.quote(values_hash.delete(k))).nil?
+              raise PreparedStatementInvalid, ":#{k} is not a variable in [#{orig_statement}]"
+            end
+          end
+
+          if statement =~ /(:\w+)/
+            raise PreparedStatementInvalid, "No value provided for #{$1} in [#{orig_statement}]"
+          end
+
+          return statement
         end
     end
 
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index 721ad76d56..ff0ab56909 100755
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -88,7 +88,28 @@ class FinderTest < Test::Unit::TestCase
     assert_nil Company.find_first(["name = ?", "37signals!"])
     assert_nil Company.find_first(["name = ?", "37signals!' OR 1=1"])
     assert_kind_of Time, Topic.find_first(["id = ?", 1]).written_on
+    assert_raises(ActiveRecord::PreparedStatementInvalid) {
+      Company.find_first(["id=? AND name = ?", 2])
+    }
+    assert_raises(ActiveRecord::PreparedStatementInvalid) {
+	   Company.find_first(["id=?", 2, 3, 4])
+    }
+  end
+
+  def test_named_bind_variables
+    assert_kind_of Firm, Company.find_first(["name = :name", { :name => "37signals" }])
+    assert_nil Company.find_first(["name = :name", { :name => "37signals!" }])
+    assert_nil Company.find_first(["name = :name", { :name => "37signals!' OR 1=1" }])
+    assert_kind_of Time, Topic.find_first(["id = :id", { :id => 1 }]).written_on
+    assert_raises(ActiveRecord::PreparedStatementInvalid) {
+      Company.find_first(["id=:id and name=:name", { :id=>3 }])
+    }
+    assert_raises(ActiveRecord::PreparedStatementInvalid) {
+      Company.find_first(["id=:id", { :id=>3, :name=>"37signals!" }])
+    }
   end
+
+  
 	
   def test_string_sanitation
     assert_not_equal "'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1")