aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew White <andrew.white@unboxed.co>2016-10-31 17:21:15 +0000
committerAndrew White <andrew.white@unboxed.co>2016-10-31 17:25:43 +0000
commit8931916f4a1c1d8e70c06063ba63928c5c7eab1e (patch)
tree936a2edb3fcacb16cc7357309b682781d1ce4f9e
parent3c9eb704e8b7fc175728f0340043b888120764cc (diff)
downloadrails-8931916f4a1c1d8e70c06063ba63928c5c7eab1e.tar.gz
rails-8931916f4a1c1d8e70c06063ba63928c5c7eab1e.tar.bz2
rails-8931916f4a1c1d8e70c06063ba63928c5c7eab1e.zip
Ensure duration parsing is consistent across DST changes
Previously `ActiveSupport::Duration.parse` used `Time.current` and `Time#advance` to calculate the number of seconds in the duration from an arbitrary collection of parts. However as `advance` tries to be consistent across DST boundaries this meant that either the duration was shorter or longer depending on the time of year. This was fixed by using an absolute reference point in UTC which isn't subject to DST transitions. An arbitrary date of Jan 1st, 2000 was chosen for no other reason that it seemed appropriate. Additionally, duration parsing should now be marginally faster as we are no longer creating instances of `ActiveSupport::TimeWithZone` every time we parse a duration string. Fixes #26941.
-rw-r--r--activesupport/CHANGELOG.md20
-rw-r--r--activesupport/lib/active_support/duration.rb5
-rw-r--r--activesupport/test/core_ext/duration_test.rb31
3 files changed, 54 insertions, 2 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 83ff80e31a..fac91c31da 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,23 @@
+* Ensure duration parsing is consistent across DST changes
+
+ Previously `ActiveSupport::Duration.parse` used `Time.current` and
+ `Time#advance` to calculate the number of seconds in the duration
+ from an arbitrary collection of parts. However as `advance` tries to
+ be consistent across DST boundaries this meant that either the
+ duration was shorter or longer depending on the time of year.
+
+ This was fixed by using an absolute reference point in UTC which
+ isn't subject to DST transitions. An arbitrary date of Jan 1st, 2000
+ was chosen for no other reason that it seemed appropriate.
+
+ Additionally, duration parsing should now be marginally faster as we
+ are no longer creating instances of `ActiveSupport::TimeWithZone`
+ every time we parse a duration string.
+
+ Fixes #26941.
+
+ *Andrew White*
+
* Use `Hash#compact` and `Hash#compact!` from Ruby 2.4. Old Ruby versions
will continue to get these methods from Active Support as before.
diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb
index 862dabb1e8..82322291d0 100644
--- a/activesupport/lib/active_support/duration.rb
+++ b/activesupport/lib/active_support/duration.rb
@@ -7,6 +7,8 @@ module ActiveSupport
#
# 1.month.ago # equivalent to Time.now.advance(months: -1)
class Duration
+ EPOCH = ::Time.utc(2000)
+
attr_accessor :value, :parts
autoload :ISO8601Parser, "active_support/duration/iso8601_parser"
@@ -140,8 +142,7 @@ module ActiveSupport
# If invalid string is provided, it will raise +ActiveSupport::Duration::ISO8601Parser::ParsingError+.
def self.parse(iso8601duration)
parts = ISO8601Parser.new(iso8601duration).parse!
- time = ::Time.current
- new(time.advance(parts) - time, parts)
+ new(EPOCH.advance(parts) - EPOCH, parts)
end
# Build ISO 8601 Duration string for this duration.
diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb
index 53575272e3..c8e8b8a350 100644
--- a/activesupport/test/core_ext/duration_test.rb
+++ b/activesupport/test/core_ext/duration_test.rb
@@ -322,4 +322,35 @@ class DurationTest < ActiveSupport::TestCase
assert_equal time + duration, time + ActiveSupport::Duration.parse(duration.iso8601), pattern.inspect
end
end
+
+ def test_iso8601_parsing_across_spring_dst_boundary
+ with_env_tz eastern_time_zone do
+ with_tz_default "Eastern Time (US & Canada)" do
+ travel_to Time.utc(2016, 3, 11) do
+ assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i
+ assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i
+ end
+ end
+ end
+ end
+
+ def test_iso8601_parsing_across_autumn_dst_boundary
+ with_env_tz eastern_time_zone do
+ with_tz_default "Eastern Time (US & Canada)" do
+ travel_to Time.utc(2016, 11, 4) do
+ assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i
+ assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i
+ end
+ end
+ end
+ end
+
+ private
+ def eastern_time_zone
+ if Gem.win_platform?
+ "EST5EDT"
+ else
+ "America/New_York"
+ end
+ end
end