Permalink
Browse files

Fixed value quoting in all generated SQL statements, so that integers…

… are not surrounded in quotes and that all sanitation are happening through the database's own quoting routine. This should hopefully make it lots easier for new adapters that doesn't accept '1' for integer columns.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@70 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 8a40c6b commit 49403831fc90a9d0d6955bab2ae6f7833be3c0ba @dhh dhh committed Dec 7, 2004
View
@@ -1,5 +1,9 @@
*CVS*
+* Fixed value quoting in all generated SQL statements, so that integers are not surrounded in quotes and that all sanitation are happening
+ through the database's own quoting routine. This should hopefully make it lots easier for new adapters that doesn't accept '1' for integer
+ columns.
+
* Fixed has_and_belongs_to_many guessing of foreign key so that keys are generated correctly for models like SomeVerySpecialClient
[Florian Weber]
@@ -190,7 +190,7 @@ def has_many(association_id, options = {})
elsif options[:dependent]
module_eval "before_destroy '#{association_name}.each { |o| o.destroy }'"
elsif options[:exclusively_dependent]
- module_eval "before_destroy { |record| #{association_class_name}.delete_all(%(#{association_class_primary_key_name} = '\#{record.id}')) }"
+ module_eval "before_destroy { |record| #{association_class_name}.delete_all(%(#{association_class_primary_key_name} = \#{record.quoted_id})) }"
end
define_method(association_name) do |*params|
@@ -323,7 +323,7 @@ def belongs_to(association_id, options = {})
if options[:remote]
association_finder = <<-"end_eval"
#{association_class_name}.find_first(
- "#{class_primary_key_name} = '\#{id}'#{options[:conditions] ? " AND " + options[:conditions] : ""}",
+ "#{class_primary_key_name} = \#{quoted_id}#{options[:conditions] ? " AND " + options[:conditions] : ""}",
#{options[:order] ? "\"" + options[:order] + "\"" : "nil" }
)
end_eval
@@ -437,7 +437,7 @@ def has_and_belongs_to_many(association_id, options = {})
association
end
- before_destroy_sql = "DELETE FROM #{join_table} WHERE #{association_class_primary_key_name} = '\\\#{self.id}'"
+ before_destroy_sql = "DELETE FROM #{join_table} WHERE #{association_class_primary_key_name} = \\\#{self.quoted_id}"
module_eval(%{before_destroy "self.connection.delete(%{#{before_destroy_sql}})"}) # "
# deprecated api
@@ -81,7 +81,7 @@ def loaded?
end
def quoted_record_ids(records)
- records.map { |record| "'#{@association_class.send(:sanitize, record.id)}'" }.join(',')
+ records.map { |record| record.quoted_id }.join(',')
end
def interpolate_sql_options!(options, *keys)
@@ -13,7 +13,7 @@ def initialize(owner, association_name, association_class_name, association_clas
@finder_sql = options[:finder_sql] ||
"SELECT t.*, j.* FROM #{association_table_name} t, #{@join_table} j " +
"WHERE t.#{@owner.class.primary_key} = j.#{@association_foreign_key} AND " +
- "j.#{association_class_primary_key_name} = '#{@owner.id}' " +
+ "j.#{association_class_primary_key_name} = #{@owner.quoted_id} " +
(options[:conditions] ? " AND " + options[:conditions] : "") + " " +
"ORDER BY #{@order}"
end
@@ -26,11 +26,11 @@ def clear
each { |record| @owner.connection.execute(sql) }
elsif @options[:conditions]
sql =
- "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = '#{@owner.id}' " +
+ "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = #{@owner.quoted_id} " +
"AND #{@association_foreign_key} IN (#{collect { |record| record.id }.join(", ")})"
@owner.connection.execute(sql)
else
- sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = '#{@owner.id}'"
+ sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = #{@owner.quoted_id}"
@owner.connection.execute(sql)
end
@@ -46,7 +46,7 @@ def find(association_id = nil, &block)
if loaded?
find_all { |record| record.id == association_id.to_i }.first
else
- find_all_records(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} = '#{association_id}' ORDER BY")).first
+ find_all_records(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} = #{@owner.send(:quote, association_id)} ORDER BY")).first
end
end
end
@@ -80,7 +80,8 @@ def insert_record(record)
if @options[:insert_sql]
@owner.connection.execute(interpolate_sql(@options[:insert_sql], record))
else
- sql = "INSERT INTO #{@join_table} (#{@association_class_primary_key_name}, #{@association_foreign_key}) VALUES ('#{@owner.id}','#{record.id}')"
+ sql = "INSERT INTO #{@join_table} (#{@association_class_primary_key_name}, #{@association_foreign_key}) " +
+ "VALUES (#{@owner.quoted_id},#{record.quoted_id})"
@owner.connection.execute(sql)
end
end
@@ -98,7 +99,7 @@ def delete_records(records)
records.each { |record| @owner.connection.execute(sql) }
else
ids = quoted_record_ids(records)
- sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = '#{@owner.id}' AND #{@association_foreign_key} IN (#{ids})"
+ sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = #{@owner.quoted_id} AND #{@association_foreign_key} IN (#{ids})"
@owner.connection.execute(sql)
end
end
@@ -8,15 +8,15 @@ def initialize(owner, association_name, association_class_name, association_clas
if options[:finder_sql]
@finder_sql = interpolate_sql(options[:finder_sql])
else
- @finder_sql = "#{@association_class_primary_key_name} = '#{@owner.id}' #{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
+ @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id} #{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
end
if options[:counter_sql]
@counter_sql = interpolate_sql(options[:counter_sql])
elsif options[:finder_sql]
@counter_sql = options[:counter_sql] = @finder_sql.gsub(/SELECT (.*) FROM/i, "SELECT COUNT(*) FROM")
else
- @counter_sql = "#{@association_class_primary_key_name} = '#{@owner.id}'#{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
+ @counter_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}#{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
end
end
@@ -40,8 +40,8 @@ def find_all(runtime_conditions = nil, orderings = nil, limit = nil, joins = nil
@collection.find_all(&block)
else
@association_class.find_all(
- "#{@association_class_primary_key_name} = '#{@owner.id}' " +
- "#{@conditions ? " AND " + @conditions : ""} #{runtime_conditions ? " AND " + @association_class.send(:sanitize_conditions, runtime_conditions) : ""}",
+ "#{@association_class_primary_key_name} = #{@owner.quoted_id}" +
+ "#{@conditions ? " AND " + @conditions : ""}#{runtime_conditions ? " AND " + @association_class.send(:sanitize_conditions, runtime_conditions) : ""}",
orderings,
limit,
joins
@@ -55,15 +55,15 @@ def find(association_id = nil, &block)
@collection.find(&block)
else
@association_class.find_on_conditions(association_id,
- "#{@association_class_primary_key_name} = '#{@owner.id}' #{@conditions ? " AND " + @conditions : ""}"
+ "#{@association_class_primary_key_name} = #{@owner.quoted_id}#{@conditions ? " AND " + @conditions : ""}"
)
end
end
# Removes all records from this association. Returns +self+ so
# method calls may be chained.
def clear
- @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = '#{@owner.id}'")
+ @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = #{@owner.quoted_id}")
@collection = []
self
end
@@ -101,7 +101,10 @@ def insert_record(record)
def delete_records(records)
ids = quoted_record_ids(records)
- @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = '#{@owner.id}' AND #{@association_class.primary_key} IN (#{ids})")
+ @association_class.update_all(
+ "#{@association_class_primary_key_name} = NULL",
+ "#{@association_class_primary_key_name} = #{@owner.quoted_id} AND #{@association_class.primary_key} IN (#{ids})"
+ )
end
end
end
@@ -239,7 +239,7 @@ def find(*ids)
ids = ids.flatten.compact.uniq
if ids.length > 1
- ids_list = ids.map{ |id| "'#{sanitize(id)}'" }.join(", ")
+ ids_list = ids.map{ |id| "#{sanitize(id)}" }.join(", ")
objects = find_all("#{primary_key} IN (#{ids_list})", primary_key)
if objects.length == ids.length
@@ -249,7 +249,7 @@ def find(*ids)
end
elsif ids.length == 1
id = ids.first
- sql = "SELECT * FROM #{table_name} WHERE #{primary_key} = '#{sanitize(id)}'"
+ sql = "SELECT * FROM #{table_name} WHERE #{primary_key} = #{sanitize(id)}"
sql << " AND #{type_condition}" unless descends_from_active_record?
if record = connection.select_one(sql, "#{name} Find")
@@ -267,7 +267,7 @@ def find(*ids)
# Example:
# Person.find_on_conditions 5, "first_name LIKE '%dav%' AND last_name = 'heinemeier'"
def find_on_conditions(id, conditions)
- find_first("#{primary_key} = '#{sanitize(id)}' AND #{sanitize_conditions(conditions)}") ||
+ find_first("#{primary_key} = #{sanitize(id)} AND #{sanitize_conditions(conditions)}") ||
raise(RecordNotFound, "Couldn't find #{name} with #{primary_key} = #{id} on the condition of #{conditions}")
end
@@ -370,12 +370,12 @@ def count_by_sql(sql)
# for looping over a collection where each element require a number of aggregate values. Like the DiscussionBoard
# that needs to list both the number of posts and comments.
def increment_counter(counter_name, id)
- update_all "#{counter_name} = #{counter_name} + 1", "#{primary_key} = #{id}"
+ update_all "#{counter_name} = #{counter_name} + 1", "#{primary_key} = #{quote(id)}"
end
# Works like increment_counter, but decrements instead.
def decrement_counter(counter_name, id)
- update_all "#{counter_name} = #{counter_name} - 1", "#{primary_key} = #{id}"
+ update_all "#{counter_name} = #{counter_name} - 1", "#{primary_key} = #{quote(id)}"
end
# Attributes named in this macro are protected from mass-assignment, such as <tt>new(attributes)</tt> and
@@ -526,10 +526,13 @@ def descends_from_active_record? # :nodoc:
superclass == Base
end
- # Used to sanitize objects before they're used in an SELECT SQL-statement.
+ def quote(object)
+ connection.quote(object)
+ end
+
+ # Used to sanitize objects before they're used in an SELECT SQL-statement. Delegates to <tt>connection.quote</tt>.
def sanitize(object) # :nodoc:
- return object if Fixnum === object
- object.to_s.gsub(/([;:])/, "").gsub('##', '\#\#').gsub(/'/, "''") # ' (for ruby-mode)
+ connection.quote(object)
end
# Used to aggregate logging and benchmark, so you can measure and represent multiple statements in a single block.
@@ -592,7 +595,7 @@ def add_conditions!(sql, conditions)
def type_condition
" (" + subclasses.inject("#{inheritance_column} = '#{Inflector.demodulize(name)}' ") do |condition, subclass|
- condition << "OR #{inheritance_column} = '#{Inflector.demodulize(subclass.name)}'"
+ condition << "OR #{inheritance_column} = '#{Inflector.demodulize(subclass.name)}' "
end + ") "
end
@@ -638,7 +641,7 @@ def sanitize_conditions(conditions)
statement =~ /\?/ ?
replace_bind_variables(statement, values) :
- statement % values.collect { |value| sanitize(value) }
+ statement % values.collect { |value| connection.quote_string(value.to_s) }
end
def replace_bind_variables(statement, values)
@@ -669,6 +672,10 @@ def id
read_attribute(self.class.primary_key)
end
+ def quoted_id
+ quote(id, self.class.columns_hash[self.class.primary_key])
+ end
+
# Sets the primary ID.
def id=(value)
write_attribute(self.class.primary_key, value)
@@ -692,7 +699,7 @@ def destroy
unless new_record?
connection.delete(
"DELETE FROM #{self.class.table_name} " +
- "WHERE #{self.class.primary_key} = '#{id}'",
+ "WHERE #{self.class.primary_key} = #{quote(id)}",
"#{self.class.name} Destroy"
)
end
@@ -814,7 +821,7 @@ def update
connection.update(
"UPDATE #{self.class.table_name} " +
"SET #{quoted_comma_pair_list(connection, attributes_with_quotes(false))} " +
- "WHERE #{self.class.primary_key} = '#{id}'",
+ "WHERE #{self.class.primary_key} = #{quote(id)}",
"#{self.class.name} Update"
)
end
@@ -320,13 +320,14 @@ def rollback_db_transaction() end
def quote(value, column = nil)
case value
- when String then "'#{quote_string(value)}'" # ' (for ruby-mode)
- when NilClass then "NULL"
- when TrueClass then (column && column.type == :boolean ? "'t'" : "1")
- when FalseClass then (column && column.type == :boolean ? "'f'" : "0")
- when Float, Fixnum, Bignum, Date then "'#{value.to_s}'"
- when Time, DateTime then "'#{value.strftime("%Y-%m-%d %H:%M:%S")}'"
- else "'#{quote_string(value.to_yaml)}'"
+ when String then "'#{quote_string(value)}'" # ' (for ruby-mode)
+ when NilClass then "NULL"
+ when TrueClass then (column && column.type == :boolean ? "'t'" : "1")
+ when FalseClass then (column && column.type == :boolean ? "'f'" : "0")
+ when Float, Fixnum, Bignum then value.to_s
+ when Date then "'#{value.to_s}'"
+ when Time, DateTime then "'#{value.strftime("%Y-%m-%d %H:%M:%S")}'"
+ else "'#{quote_string(value.to_yaml)}'"
end
end
@@ -117,6 +117,10 @@ def create_database(name)
execute "CREATE DATABASE #{name}"
end
+ def quote_string(s)
+ Mysql::quote(s)
+ end
+
private
def select(sql, name = nil)
result = nil
@@ -62,18 +62,18 @@ def plural_rules #:doc:
def singular_rules #:doc:
[
- [/(x|ch|ss)es$/, '\1'],
- [/movies$/, 'movie'],
- [/([^aeiouy]|qu)ies$/, '\1y'],
- [/([lr])ves$/, '\1f'],
- [/([^f])ves$/, '\1fe'],
- [/(analy|ba|diagno|parenthe|progno|synop|the)ses$/, '\1sis'],
- [/([ti])a$/, '\1um'],
- [/people$/, 'person'],
- [/men$/, 'man'],
- [/status$/, 'status'],
- [/children$/, 'child'],
- [/s$/, '']
- ]
+ [/(x|ch|ss)es$/, '\1'],
+ [/movies$/, 'movie'],
+ [/([^aeiouy]|qu)ies$/, '\1y'],
+ [/([lr])ves$/, '\1f'],
+ [/([^f])ves$/, '\1fe'],
+ [/(analy|ba|diagno|parenthe|progno|synop|the)ses$/, '\1sis'],
+ [/([ti])a$/, '\1um'],
+ [/people$/, 'person'],
+ [/men$/, 'man'],
+ [/status$/, 'status'],
+ [/children$/, 'child'],
+ [/s$/, '']
+ ]
end
end
@@ -68,7 +68,7 @@ def test_bind_variables
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")
+ assert_not_equal "'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1")
+ assert_equal "'something; select table'", ActiveRecord::Base.sanitize("something; select table")
end
end

0 comments on commit 4940383

Please sign in to comment.