diff options
10 files changed, 71 insertions, 51 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 80218b9070..eb8a906338 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -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] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b83ce831c7..6f6da10a01 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -190,7 +190,7 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index a60b9ddab5..00758aa66c 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -81,7 +81,7 @@ module ActiveRecord 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) diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 8dec71403c..d53650fbd8 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -13,7 +13,7 @@ module ActiveRecord @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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 0f2d20d240..1d8441e6f8 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -8,7 +8,7 @@ module ActiveRecord 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] @@ -16,7 +16,7 @@ module ActiveRecord 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 @@ module ActiveRecord @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,7 +55,7 @@ module ActiveRecord @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 @@ -63,7 +63,7 @@ module ActiveRecord # 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 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index a45480945e..f52a1524d2 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -239,7 +239,7 @@ module ActiveRecord #:nodoc: 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 @@ module ActiveRecord #:nodoc: 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 @@ module ActiveRecord #:nodoc: # 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 @@ module ActiveRecord #:nodoc: # 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 @@ module ActiveRecord #: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 @@ module ActiveRecord #:nodoc: 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 @@ module ActiveRecord #:nodoc: 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 @@ module ActiveRecord #:nodoc: 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 @@ module ActiveRecord #:nodoc: 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 @@ module ActiveRecord #:nodoc: 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 diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 301ee208a3..6f4dca1b7b 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -320,13 +320,14 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 5dcdded5bc..55c15c6823 100755 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -117,6 +117,10 @@ module ActiveRecord execute "CREATE DATABASE #{name}" end + def quote_string(s) + Mysql::quote(s) + end + private def select(sql, name = nil) result = nil diff --git a/activerecord/lib/active_record/support/inflector.rb b/activerecord/lib/active_record/support/inflector.rb index b41f85188b..bc5724e742 100644 --- a/activerecord/lib/active_record/support/inflector.rb +++ b/activerecord/lib/active_record/support/inflector.rb @@ -62,18 +62,18 @@ module Inflector 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
\ No newline at end of file diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb index b7b4ab589a..cc240c8acc 100755 --- a/activerecord/test/finder_test.rb +++ b/activerecord/test/finder_test.rb @@ -68,7 +68,7 @@ class FinderTest < Test::Unit::TestCase 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
\ No newline at end of file |