Skip to content

Commit 8931916

Browse files
committed
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.
1 parent 3c9eb70 commit 8931916

File tree

3 files changed

+54
-2
lines changed

3 files changed

+54
-2
lines changed

activesupport/CHANGELOG.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,23 @@
1+
* Ensure duration parsing is consistent across DST changes
2+
3+
Previously `ActiveSupport::Duration.parse` used `Time.current` and
4+
`Time#advance` to calculate the number of seconds in the duration
5+
from an arbitrary collection of parts. However as `advance` tries to
6+
be consistent across DST boundaries this meant that either the
7+
duration was shorter or longer depending on the time of year.
8+
9+
This was fixed by using an absolute reference point in UTC which
10+
isn't subject to DST transitions. An arbitrary date of Jan 1st, 2000
11+
was chosen for no other reason that it seemed appropriate.
12+
13+
Additionally, duration parsing should now be marginally faster as we
14+
are no longer creating instances of `ActiveSupport::TimeWithZone`
15+
every time we parse a duration string.
16+
17+
Fixes #26941.
18+
19+
*Andrew White*
20+
121
* Use `Hash#compact` and `Hash#compact!` from Ruby 2.4. Old Ruby versions
222
will continue to get these methods from Active Support as before.
323

activesupport/lib/active_support/duration.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ module ActiveSupport
77
#
88
# 1.month.ago # equivalent to Time.now.advance(months: -1)
99
class Duration
10+
EPOCH = ::Time.utc(2000)
11+
1012
attr_accessor :value, :parts
1113

1214
autoload :ISO8601Parser, "active_support/duration/iso8601_parser"
@@ -140,8 +142,7 @@ def respond_to_missing?(method, include_private = false) #:nodoc:
140142
# If invalid string is provided, it will raise +ActiveSupport::Duration::ISO8601Parser::ParsingError+.
141143
def self.parse(iso8601duration)
142144
parts = ISO8601Parser.new(iso8601duration).parse!
143-
time = ::Time.current
144-
new(time.advance(parts) - time, parts)
145+
new(EPOCH.advance(parts) - EPOCH, parts)
145146
end
146147

147148
# Build ISO 8601 Duration string for this duration.

activesupport/test/core_ext/duration_test.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,4 +322,35 @@ def test_iso8601_output_and_reparsing
322322
assert_equal time + duration, time + ActiveSupport::Duration.parse(duration.iso8601), pattern.inspect
323323
end
324324
end
325+
326+
def test_iso8601_parsing_across_spring_dst_boundary
327+
with_env_tz eastern_time_zone do
328+
with_tz_default "Eastern Time (US & Canada)" do
329+
travel_to Time.utc(2016, 3, 11) do
330+
assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i
331+
assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i
332+
end
333+
end
334+
end
335+
end
336+
337+
def test_iso8601_parsing_across_autumn_dst_boundary
338+
with_env_tz eastern_time_zone do
339+
with_tz_default "Eastern Time (US & Canada)" do
340+
travel_to Time.utc(2016, 11, 4) do
341+
assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i
342+
assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i
343+
end
344+
end
345+
end
346+
end
347+
348+
private
349+
def eastern_time_zone
350+
if Gem.win_platform?
351+
"EST5EDT"
352+
else
353+
"America/New_York"
354+
end
355+
end
325356
end

0 commit comments

Comments
 (0)