From e62fff40edde10bd04bbb91ce242f4a7f7ea64a8 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 31 Oct 2014 11:23:24 -0600 Subject: Treat strings greater than int max value as out of range Sufficiently large integers cause `find` and `find_by` to raise `StatementInvalid` instead of `RecordNotFound` or just returning `nil`. Given that we can't cast to `nil` for `Integer` like we would with junk data for other types, we raise a `RangeError` instead, and rescue in places where it would be highly unexpected to get an exception from casting. Fixes #17380 --- activerecord/lib/active_record/core.rb | 4 ++++ .../lib/active_record/relation/finder_methods.rb | 2 ++ activerecord/lib/active_record/type.rb | 1 + activerecord/lib/active_record/type/big_integer.rb | 13 +++++++++++ .../active_record/type/decimal_without_scale.rb | 4 ++-- activerecord/lib/active_record/type/integer.rb | 21 ++++++++++++++++- activerecord/test/cases/base_test.rb | 1 - activerecord/test/cases/finder_test.rb | 27 +++++++++++----------- activerecord/test/cases/migration_test.rb | 1 - 9 files changed, 56 insertions(+), 18 deletions(-) create mode 100644 activerecord/lib/active_record/type/big_integer.rb diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index a0382f99e6..952aeaa703 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -155,6 +155,8 @@ module ActiveRecord raise RecordNotFound, "Couldn't find #{name} with '#{primary_key}'=#{id}" end record + rescue RangeError + raise RecordNotFound, "Couldn't find #{name} with an out of range value for '#{primary_key}'" end def find_by(*args) @@ -185,6 +187,8 @@ module ActiveRecord s.execute(hash.values, self, connection).first rescue TypeError => e raise ActiveRecord::StatementInvalid.new(e.message, e) + rescue RangeError + nil end end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index ddb392a6d4..145b7378cf 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -433,6 +433,8 @@ module ActiveRecord else find_some(ids) end + rescue RangeError + raise RecordNotFound, "Couldn't find #{@klass.name} with an out of range ID" end def find_one(id) diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb index e3d6c5957e..e5acbbb6b3 100644 --- a/activerecord/lib/active_record/type.rb +++ b/activerecord/lib/active_record/type.rb @@ -4,6 +4,7 @@ require 'active_record/type/numeric' require 'active_record/type/time_value' require 'active_record/type/value' +require 'active_record/type/big_integer' require 'active_record/type/binary' require 'active_record/type/boolean' require 'active_record/type/date' diff --git a/activerecord/lib/active_record/type/big_integer.rb b/activerecord/lib/active_record/type/big_integer.rb new file mode 100644 index 0000000000..0c72d8914f --- /dev/null +++ b/activerecord/lib/active_record/type/big_integer.rb @@ -0,0 +1,13 @@ +require 'active_record/type/integer' + +module ActiveRecord + module Type + class BigInteger < Integer # :nodoc: + private + + def max_value + ::Float::INFINITY + end + end + end +end diff --git a/activerecord/lib/active_record/type/decimal_without_scale.rb b/activerecord/lib/active_record/type/decimal_without_scale.rb index cabdcecdd7..ff5559e300 100644 --- a/activerecord/lib/active_record/type/decimal_without_scale.rb +++ b/activerecord/lib/active_record/type/decimal_without_scale.rb @@ -1,8 +1,8 @@ -require 'active_record/type/integer' +require 'active_record/type/big_integer' module ActiveRecord module Type - class DecimalWithoutScale < Integer # :nodoc: + class DecimalWithoutScale < BigInteger # :nodoc: def type :decimal end diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb index 08477d1303..2b0f0b2734 100644 --- a/activerecord/lib/active_record/type/integer.rb +++ b/activerecord/lib/active_record/type/integer.rb @@ -15,9 +15,28 @@ module ActiveRecord case value when true then 1 when false then 0 - else value.to_i rescue nil + else + result = value.to_i rescue nil + ensure_below_max(result) if result + result end end + + def ensure_below_max(value) + if value > max_value + raise RangeError, "#{value} is too large for #{self.class} with limit #{limit || 4}" + end + end + + def max_value + @max_value = determine_max_value unless defined?(@max_value) + @max_value + end + + def determine_max_value + limit = self.limit || 4 + 2 << (limit * 8 - 1) # 8 bits per byte with one bit for sign + end end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 4ad27c0e8d..4e1685f151 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -985,7 +985,6 @@ class BasicsTest < ActiveRecord::TestCase class NumericData < ActiveRecord::Base self.table_name = 'numeric_data' - attribute :world_population, Type::Integer.new attribute :my_house_population, Type::Integer.new attribute :atoms_in_universe, Type::Integer.new end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index ac570ecbe0..d1ff8516e8 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -99,21 +99,10 @@ class FinderTest < ActiveRecord::TestCase end def test_exists_fails_when_parameter_has_invalid_type - if current_adapter?(:PostgreSQLAdapter, :MysqlAdapter) - assert_raises ActiveRecord::StatementInvalid do - Topic.exists?(("9"*53).to_i) # number that's bigger than int - end - else + assert_raises(RangeError) do assert_equal false, Topic.exists?(("9"*53).to_i) # number that's bigger than int end - - if current_adapter?(:PostgreSQLAdapter) - assert_raises ActiveRecord::StatementInvalid do - Topic.exists?("foo") - end - else - assert_equal false, Topic.exists?("foo") - end + assert_equal false, Topic.exists?("foo") end def test_exists_does_not_select_columns_without_alias @@ -209,6 +198,18 @@ class FinderTest < ActiveRecord::TestCase assert_equal 2, last_devs.size end + def test_find_with_large_number + assert_raises(ActiveRecord::RecordNotFound) { Topic.find('9999999999999999999999999999999') } + end + + def test_find_by_with_large_number + assert_nil Topic.find_by(id: '9999999999999999999999999999999') + end + + def test_find_by_id_with_large_number + assert_nil Topic.find_by_id('9999999999999999999999999999999') + end + def test_find_an_empty_array assert_equal [], Topic.find([]) end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 270e446c69..8db01fc847 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -15,7 +15,6 @@ class BigNumber < ActiveRecord::Base unless current_adapter?(:PostgreSQLAdapter, :SQLite3Adapter) attribute :value_of_e, Type::Integer.new end - attribute :world_population, Type::Integer.new attribute :my_house_population, Type::Integer.new end -- cgit v1.2.3