Skip to content
This repository
Browse code

active_record: Quote numeric values compared to string columns.

  • Loading branch information...
commit 04c2d2e2e4197a1564c33f09d3e52253d845925f 1 parent b4f189a
Dylan Thacker-Smith dylanahsmith authored
10 activerecord/CHANGELOG.md
Source Rendered
... ... @@ -1,5 +1,15 @@
1 1 ## Rails 3.2.12 (unreleased) ##
2 2
  3 +* Quote numeric values being compared to non-numeric columns. Otherwise,
  4 + in some database, the string column values will be coerced to a numeric
  5 + allowing 0, 0.0 or false to match any string starting with a non-digit.
  6 +
  7 + Example:
  8 +
  9 + App.where(apikey: 0) # => SELECT * FROM users WHERE apikey = '0'
  10 +
  11 + *Dylan Smith*
  12 +
3 13 * Don't update `column_defaults` when calling destructive methods on column with default value.
4 14 Backport c517602.
5 15 Fix #6115.
10 activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
@@ -25,13 +25,19 @@ def quote(value, column = nil)
25 25 when true, false
26 26 if column && column.type == :integer
27 27 value ? '1' : '0'
  28 + elsif column && [:text, :string, :binary].include?(column.type)
  29 + value ? "'1'" : "'0'"
28 30 else
29 31 value ? quoted_true : quoted_false
30 32 end
31 33 # BigDecimals need to be put in a non-normalized form and quoted.
32 34 when nil then "NULL"
33   - when BigDecimal then value.to_s('F')
34   - when Numeric then value.to_s
  35 + when Numeric, ActiveSupport::Duration
  36 + value = BigDecimal === value ? value.to_s('F') : value.to_s
  37 + if column && ![:integer, :float, :decimal].include?(column.type)
  38 + value = "'#{value}'"
  39 + end
  40 + value
35 41 when Date, Time then "'#{quoted_date(value)}'"
36 42 when Symbol then "'#{quote_string(value.to_s)}'"
37 43 else
2  activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -199,8 +199,6 @@ def quote(value, column = nil)
199 199 if value.kind_of?(String) && column && column.type == :binary && column.class.respond_to?(:string_to_binary)
200 200 s = column.class.string_to_binary(value).unpack("H*")[0]
201 201 "x'#{s}'"
202   - elsif value.kind_of?(BigDecimal)
203   - value.to_s("F")
204 202 else
205 203 super
206 204 end
4 activerecord/lib/active_record/relation/predicate_builder.rb
@@ -51,6 +51,10 @@ def self.build_from_hash(engine, attributes, default_table, allow_table_name = t
51 51 when Class
52 52 # FIXME: I think we need to deprecate this behavior
53 53 attribute.eq(value.name)
  54 + when Integer, ActiveSupport::Duration
  55 + # Arel treats integers as literals, but they should be quoted when compared with strings
  56 + column = engine.connection.schema_cache.columns_hash(table.name)[attribute.name.to_s]
  57 + attribute.eq(Arel::Nodes::SqlLiteral.new(engine.connection.quote(value, column)))
54 58 else
55 59 attribute.eq(value)
56 60 end
14 activerecord/test/cases/quoting_test.rb
@@ -122,35 +122,35 @@ def test_quote_false
122 122 def test_quote_float
123 123 float = 1.2
124 124 assert_equal float.to_s, @quoter.quote(float, nil)
125   - assert_equal float.to_s, @quoter.quote(float, Object.new)
  125 + assert_equal float.to_s, @quoter.quote(float, FakeColumn.new(:float))
126 126 end
127 127
128 128 def test_quote_fixnum
129 129 fixnum = 1
130 130 assert_equal fixnum.to_s, @quoter.quote(fixnum, nil)
131   - assert_equal fixnum.to_s, @quoter.quote(fixnum, Object.new)
  131 + assert_equal fixnum.to_s, @quoter.quote(fixnum, FakeColumn.new(:integer))
132 132 end
133 133
134 134 def test_quote_bignum
135 135 bignum = 1 << 100
136 136 assert_equal bignum.to_s, @quoter.quote(bignum, nil)
137   - assert_equal bignum.to_s, @quoter.quote(bignum, Object.new)
  137 + assert_equal bignum.to_s, @quoter.quote(bignum, FakeColumn.new(:integer))
138 138 end
139 139
140 140 def test_quote_bigdecimal
141 141 bigdec = BigDecimal.new((1 << 100).to_s)
142 142 assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, nil)
143   - assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, Object.new)
  143 + assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, FakeColumn.new(:decimal))
144 144 end
145 145
146 146 def test_dates_and_times
147 147 @quoter.extend(Module.new { def quoted_date(value) 'lol' end })
148 148 assert_equal "'lol'", @quoter.quote(Date.today, nil)
149   - assert_equal "'lol'", @quoter.quote(Date.today, Object.new)
  149 + assert_equal "'lol'", @quoter.quote(Date.today, FakeColumn.new(:date))
150 150 assert_equal "'lol'", @quoter.quote(Time.now, nil)
151   - assert_equal "'lol'", @quoter.quote(Time.now, Object.new)
  151 + assert_equal "'lol'", @quoter.quote(Time.now, FakeColumn.new(:time))
152 152 assert_equal "'lol'", @quoter.quote(DateTime.now, nil)
153   - assert_equal "'lol'", @quoter.quote(DateTime.now, Object.new)
  153 + assert_equal "'lol'", @quoter.quote(DateTime.now, FakeColumn.new(:datetime))
154 154 end
155 155
156 156 def test_crazy_object
25 activerecord/test/cases/relation/where_test.rb
@@ -35,5 +35,30 @@ def test_where_with_table_name_and_empty_array
35 35 def test_where_with_empty_hash_and_no_foreign_key
36 36 assert_equal 0, Edge.where(:sink => {}).count
37 37 end
  38 +
  39 + def test_where_with_integer_for_string_column
  40 + count = Post.where(:title => 0).count
  41 + assert_equal 0, count
  42 + end
  43 +
  44 + def test_where_with_float_for_string_column
  45 + count = Post.where(:title => 0.0).count
  46 + assert_equal 0, count
  47 + end
  48 +
  49 + def test_where_with_boolean_for_string_column
  50 + count = Post.where(:title => false).count
  51 + assert_equal 0, count
  52 + end
  53 +
  54 + def test_where_with_decimal_for_string_column
  55 + count = Post.where(:title => BigDecimal.new(0)).count
  56 + assert_equal 0, count
  57 + end
  58 +
  59 + def test_where_with_duration_for_string_column
  60 + count = Post.where(:title => 0.seconds).count
  61 + assert_equal 0, count
  62 + end
38 63 end
39 64 end
6 activerecord/test/cases/relation_scoping_test.rb
@@ -380,19 +380,19 @@ def test_default_scoping_with_threads
380 380 def test_default_scope_with_inheritance
381 381 wheres = InheritedPoorDeveloperCalledJamis.scoped.where_values_hash
382 382 assert_equal "Jamis", wheres[:name]
383   - assert_equal 50000, wheres[:salary]
  383 + assert_equal Arel.sql("50000"), wheres[:salary]
384 384 end
385 385
386 386 def test_default_scope_with_module_includes
387 387 wheres = ModuleIncludedPoorDeveloperCalledJamis.scoped.where_values_hash
388 388 assert_equal "Jamis", wheres[:name]
389   - assert_equal 50000, wheres[:salary]
  389 + assert_equal Arel.sql("50000"), wheres[:salary]
390 390 end
391 391
392 392 def test_default_scope_with_multiple_calls
393 393 wheres = MultiplePoorDeveloperCalledJamis.scoped.where_values_hash
394 394 assert_equal "Jamis", wheres[:name]
395   - assert_equal 50000, wheres[:salary]
  395 + assert_equal Arel.sql("50000"), wheres[:salary]
396 396 end
397 397
398 398 def test_method_scope
2  activerecord/test/schema/schema.rb
@@ -530,6 +530,8 @@ def create_table(*args, &block)
530 530 create_table :price_estimates, :force => true do |t|
531 531 t.string :estimate_of_type
532 532 t.integer :estimate_of_id
  533 + t.string :thing_type
  534 + t.integer :thing_id
533 535 t.integer :price
534 536 end
535 537

0 comments on commit 04c2d2e

Please sign in to comment.
Something went wrong with that request. Please try again.