From 1a48d50363c024277dbebfef67c48bebe21b538f Mon Sep 17 00:00:00 2001
From: Alexei Emam <alexei@chaptermedia.com>
Date: Thu, 22 Aug 2019 16:11:42 +0100
Subject: [PATCH] Disallow all non-numerics as Duration.build input, Fixes
 #37012

    Prevent `ActiveSupport::Duration.build(value)` from creating instances of
    `ActiveSupport::Duration` unless `value` is of type `Numeric`.

    Addresses the errant set of behaviours described in #37012 where
    `ActiveSupport::Duration` comparisons would fail confusingly
    or return unexpected results when comparing durations built from instances of `String`.

    Before:

        small_duration_from_string = ActiveSupport::Duration.build('9')
        large_duration_from_string = ActiveSupport::Duration.build('100000000000000')
        small_duration_from_int = ActiveSupport::Duration.build(9)

        large_duration_from_string > small_duration_from_string
            => false

        small_duration_from_string == small_duration_from_int
            => false

        small_duration_from_int < large_duration_from_string
            => ArgumentError (comparison of ActiveSupport::Duration::Scalar
                    with ActiveSupport::Duration failed)

        large_duration_from_string > small_duration_from_int
            => ArgumentError (comparison of String with ActiveSupport::Duration failed)

    After:

        small_duration_from_string = ActiveSupport::Duration.build('9')
            => TypeError (can't build an `ActiveSupport::Duration` from a `String`)
---
 activesupport/CHANGELOG.md                   | 33 ++++++++++++++++++++
 activesupport/lib/active_support/duration.rb |  4 +++
 activesupport/test/core_ext/duration_test.rb | 16 ++++++++++
 3 files changed, 53 insertions(+)

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index f83ed8a83f..637cc65d02 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,36 @@
+*   Prevent `ActiveSupport::Duration.build(value)` from creating instances of
+    `ActiveSupport::Duration` unless `value` is of type `Numeric`.
+
+    Addresses the errant set of behaviours described in #37012 where
+    `ActiveSupport::Duration` comparisons would fail confusingly
+    or return unexpected results when comparing durations built from instances of `String`.
+
+    Before:
+
+        small_duration_from_string = ActiveSupport::Duration.build('9')
+        large_duration_from_string = ActiveSupport::Duration.build('100000000000000')
+        small_duration_from_int = ActiveSupport::Duration.build(9)
+
+        large_duration_from_string > small_duration_from_string
+            => false
+
+        small_duration_from_string == small_duration_from_int
+            => false
+
+        small_duration_from_int < large_duration_from_string
+            => ArgumentError (comparison of ActiveSupport::Duration::Scalar
+                    with ActiveSupport::Duration failed)
+
+        large_duration_from_string > small_duration_from_int
+            => ArgumentError (comparison of String with ActiveSupport::Duration failed)
+
+    After:
+
+        small_duration_from_string = ActiveSupport::Duration.build('9')
+            => TypeError (can't build an ActiveSupport::Duration from a String)
+
+    *Alexei Emam*
+
 *   Add `ActiveSupport::Cache::Store#delete_multi` method to delete multiple keys from the cache store.
 
     *Peter Zhu*
diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb
index 2b4f1288f1..70c927259f 100644
--- a/activesupport/lib/active_support/duration.rb
+++ b/activesupport/lib/active_support/duration.rb
@@ -181,6 +181,10 @@ module ActiveSupport
       #   ActiveSupport::Duration.build(2716146).parts  # => {:months=>1, :days=>1}
       #
       def build(value)
+        unless value.is_a?(::Numeric)
+          raise TypeError, "can't build an #{self.name} from a #{value.class.name}"
+        end
+
         parts = {}
         remainder = value.to_f
 
diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb
index 63934e2433..826266c9d7 100644
--- a/activesupport/test/core_ext/duration_test.rb
+++ b/activesupport/test/core_ext/duration_test.rb
@@ -661,6 +661,22 @@ class DurationTest < ActiveSupport::TestCase
     assert_equal 660, (d1 + 60).to_i
   end
 
+  def test_string_build_raises_error
+    error = assert_raises(TypeError) do
+      ActiveSupport::Duration.build("9")
+    end
+
+    assert_equal "can't build an ActiveSupport::Duration from a String", error.message
+  end
+
+  def test_non_numeric_build_raises_error
+    error = assert_raises(TypeError) do
+      ActiveSupport::Duration.build(nil)
+    end
+
+    assert_equal "can't build an ActiveSupport::Duration from a NilClass", error.message
+  end
+
   private
     def eastern_time_zone
       if Gem.win_platform?