Permalink
Browse files

Added bind-style variable interpolation for the condition arrays that…

… uses the adapter's quote method [Michael Koziarski]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@56 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 5e3eaff commit 3e7d191e6450a3050976c735b0efc11b8a0aee93 @dhh dhh committed Dec 7, 2004
Showing with 32 additions and 11 deletions.
  1. +10 −0 activerecord/CHANGELOG
  2. +15 −11 activerecord/lib/active_record/base.rb
  3. +7 −0 activerecord/test/finder_test.rb
View
@@ -1,5 +1,15 @@
*CVS*
+* Added bind-style variable interpolation for the condition arrays that uses the adapter's quote method [Michael Koziarski]
+
+ Before:
+ find_first([ "user_name = '%s' AND password = '%s'", user_name, password ])]
+ find_first([ "firm_id = %s", firm_id ])] # unsafe!
+
+ After:
+ find_first([ "user_name = ? AND password = ?", user_name, password ])]
+ find_first([ "firm_id = ?", firm_id ])]
+
* Added CSV format for fixtures #272 [what-a-day]. (See the new and expanded documentation on fixtures for more information)
* Fixed fixtures using primary key fields called something else than "id" [dave]
@@ -67,7 +67,7 @@ class StatementInvalid < ActiveRecordError #:nodoc:
# end
#
# def self.authenticate_safely(user_name, password)
- # find_first([ "user_name = '%s' AND password = '%s'", user_name, password ])
+ # find_first([ "user_name = ? AND password = ?", user_name, password ])
# end
# end
#
@@ -76,10 +76,6 @@ class StatementInvalid < ActiveRecordError #:nodoc:
# on the other hand, will sanitize the <tt>user_name</tt> and +password+ before inserting them in the query, which will ensure that
# an attacker can't escape the query and fake the login (or worse).
#
- # Beware, that the approach used in <tt>authenticate_unsafely</tt> is basically just a wrapped call to sprintf. This means that you
- # still have to quote when using %s or use %d instead. So find_first([ "firm_id = %s", firm_id ]) is _not_ safe while both
- # find_first([ "firm_id = '%s'", firm_id ]) and find_first([ "firm_id = %d", firm_id ]) are.
- #
# == Overwriting default accessors
#
# All column values are automatically available through basic accessors on the Active Record object, but some times you
@@ -636,13 +632,21 @@ def class_name_of_active_record_descendant(klass)
# Accepts either a condition array or string. The string is returned untouched, but the array has each of
# the condition values sanitized.
def sanitize_conditions(conditions)
- if Array === conditions
- statement, values = conditions[0], conditions[1..-1]
- values.collect! { |value| sanitize(value) }
- conditions = statement % values
+ return conditions unless conditions.is_a?(Array)
+
+ statement, values = conditions[0], conditions[1..-1]
+
+ statement =~ /\?/ ?
+ replace_bind_variables(statement, values) :
+ statement % values.collect { |value| sanitize(value) }
+ end
+
+ def replace_bind_variables(statement, values)
+ while statement =~ /\?/
+ statement.sub!(/\?/, connection.quote(values.shift))
end
-
- return conditions
+
+ return statement
end
end
@@ -60,6 +60,13 @@ def test_condition_interpolation
assert_kind_of Time, Topic.find_first(["id = %d", 1]).written_on
end
+ def test_bind_variables
+ assert_kind_of Firm, Company.find_first(["name = ?", "37signals"])
+ 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
+ end
+
def test_string_sanitation
assert_equal "something '' 1=1", ActiveRecord::Base.sanitize("something ' 1=1")
assert_equal "something select table", ActiveRecord::Base.sanitize("something; select table")

0 comments on commit 3e7d191

Please sign in to comment.