aboutsummaryrefslogtreecommitdiffstats
path: root/activemodel
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2019-02-20 20:55:09 +0900
committerRyuta Kamizono <kamipo@gmail.com>2019-02-20 22:00:56 +0900
commit357cd23d3aedabb99fd70b812ffcea2d1cc9893d (patch)
treed4a21b7e618b2d4b71d0534ce180eac9040a4bd5 /activemodel
parentdf2ebf9b59b8ef063923136ba7097328db6c949f (diff)
downloadrails-357cd23d3aedabb99fd70b812ffcea2d1cc9893d.tar.gz
rails-357cd23d3aedabb99fd70b812ffcea2d1cc9893d.tar.bz2
rails-357cd23d3aedabb99fd70b812ffcea2d1cc9893d.zip
Don't allow `where` with non numeric string matches to 0 values
This is a follow-up of #35310. Currently `Topic.find_by(id: "not-a-number")` matches to a `id = 0` record. That is considered as silently leaking information. If non numeric string is given to find by an integer column, it should not be matched to any record. Related #12793.
Diffstat (limited to 'activemodel')
-rw-r--r--activemodel/lib/active_model/type/helpers/numeric.rb7
-rw-r--r--activemodel/lib/active_model/type/integer.rb10
-rw-r--r--activemodel/test/cases/type/integer_test.rb8
3 files changed, 17 insertions, 8 deletions
diff --git a/activemodel/lib/active_model/type/helpers/numeric.rb b/activemodel/lib/active_model/type/helpers/numeric.rb
index 444847a210..1d8171e25b 100644
--- a/activemodel/lib/active_model/type/helpers/numeric.rb
+++ b/activemodel/lib/active_model/type/helpers/numeric.rb
@@ -26,15 +26,18 @@ module ActiveModel
private
def number_to_non_number?(old_value, new_value_before_type_cast)
- old_value != nil && non_numeric_string?(new_value_before_type_cast)
+ old_value != nil && non_numeric_string?(new_value_before_type_cast.to_s)
end
def non_numeric_string?(value)
# 'wibble'.to_i will give zero, we want to make sure
# that we aren't marking int zero to string zero as
# changed.
- !/\A[-+]?\d+/.match?(value.to_s)
+ !NUMERIC_REGEX.match?(value)
end
+
+ NUMERIC_REGEX = /\A\s*[+-]?\d/
+ private_constant :NUMERIC_REGEX
end
end
end
diff --git a/activemodel/lib/active_model/type/integer.rb b/activemodel/lib/active_model/type/integer.rb
index 5878b94171..a7ae57aede 100644
--- a/activemodel/lib/active_model/type/integer.rb
+++ b/activemodel/lib/active_model/type/integer.rb
@@ -24,11 +24,8 @@ module ActiveModel
end
def serialize(value)
- result = super
- if result
- ensure_in_range(result)
- end
- result
+ return if value.is_a?(::String) && non_numeric_string?(value)
+ ensure_in_range(super)
end
private
@@ -39,9 +36,10 @@ module ActiveModel
end
def ensure_in_range(value)
- unless range.cover?(value)
+ if value && !range.cover?(value)
raise ActiveModel::RangeError, "#{value} is out of range for #{self.class} with limit #{_limit} bytes"
end
+ value
end
def max_value
diff --git a/activemodel/test/cases/type/integer_test.rb b/activemodel/test/cases/type/integer_test.rb
index 9bd0110099..304ae33de0 100644
--- a/activemodel/test/cases/type/integer_test.rb
+++ b/activemodel/test/cases/type/integer_test.rb
@@ -50,6 +50,14 @@ module ActiveModel
assert_equal 7200, type.cast(2.hours)
end
+ test "casting string for database" do
+ type = Type::Integer.new
+ assert_nil type.serialize("wibble")
+ assert_equal 5, type.serialize("5wibble")
+ assert_equal 5, type.serialize(" +5")
+ assert_equal(-5, type.serialize(" -5"))
+ end
+
test "casting empty string" do
type = Type::Integer.new
assert_nil type.cast("")