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