This project is archived and is in readonly mode.

#2066 ✓resolved
Levin Alexander

Time + Duration misbehaves on DST boundary

Reported by Levin Alexander | February 25th, 2009 @ 05:26 AM | in 3.0.2

adding a Duration to a Time misbehaves, when the calculation crosses a DST boundary and the Duration is longer than 24 hours.

Essentially, it seems that 24.hour are treated as a full day, which gives incorrect (or at least unexpected) results if a day is only 23 hours long.


>> ENV["TZ"] = "CET"
=> "CET"
>> Time.local(2009,3,29,0,0,0) # just before the change to DST
=> Sun Mar 29 00:00:00 +0100 2009
>> Time.local(2009,3,29,0,0,0) + 1.day
=> Mon Mar 30 00:00:00 +0200 2009 # midnight the next day, correct and as expected
>> Time.local(2009,3,29,0,0,0) + 24.hours
=> Mon Mar 30 00:00:00 +0200 2009 # => WRONG, only 23 hours have passed because of DST switchover
>> Time.local(2009,3,29,0,0,0) + 24.hours.to_i
=> Mon Mar 30 01:00:00 +0200 2009 # expected result

If the duration added is less than 24 hours the results are correct:


>> Time.local(2009,3,29,0,0,0) + 23.hours
=> Mon Mar 30 00:00:00 +0200 2009 # correct
>> Time.local(2009,3,29,0,0,0) + 22.hours
=> Sun Mar 29 23:00:00 +0200 2009 # also correct

Using minutes instead of hours makes no difference


>> Time.local(2009,3,29,0,0,0) + (23 * 60 + 1).minutes # less than 24 hours
=> Mon Mar 30 00:01:00 +0200 2009 # correct
>> Time.local(2009,3,29,0,0,0) + (24 * 60 + 1).minutes
=> Mon Mar 30 00:01:00 +0200 2009 # WRONG

this shows that the problem occurs on Durations of more than 24 hours:


>> Time.local(2009,3,28,23,0,0) + 23.hours
=> Sun Mar 29 23:00:00 +0200 2009 # correct
>> Time.local(2009,3,28,23,0,0) + 24.hours
=> Sun Mar 29 23:00:00 +0200 2009 # wrong

Comments and changes to this ticket

  • Levin Alexander

    Levin Alexander February 25th, 2009 @ 06:09 AM

    ActiveSupport::TimeWithTimezone behaves as expected:

    
    >> Time.local(2009,3,29,0,0,0).in_time_zone("CET") + 24.hours
    => Mon, 30 Mar 2009 01:00:00 CEST +02:00 # correct
    
  • Levin Alexander

    Levin Alexander February 25th, 2009 @ 06:36 AM

    There are unit tests in time_ext_test.rb that call for the incorrect behavior:

    
      def test_daylight_savings_time_crossings_forward_start
        with_env_tz 'US/Eastern' do
          # st: US: 2005 April 2nd 7:27pm
          assert_equal Time.local(2005,4,3,19,27,0), Time.local(2005,4,2,19,27,0).since(86400), 'st+1.day=>dt'
          assert_equal Time.local(2005,4,4,19,27,0), Time.local(2005,4,3,19,27,0).since(86400), 'dt+1.day=>dt'
        end
    

    Is Time#since really supposed to behave this way?

  • Geoff Buesing

    Geoff Buesing February 25th, 2009 @ 03:27 PM

    • State changed from “new” to “open”
    • Milestone cleared.

    I agree, this is wonky behavior. Reason it exists at all is because, in the days before Rails had a proper Duration class (i.e., 2006), this was a way to fake a duration of 1 day and have it cross a DST boundary correctly.

    Now that we have Duration, it's not needed, it's confusing, and needs to go. Adding 24.hours should always advance the time exactly 24 hours, and adding 1.day should advance to the same time on the next day, which will be 23-25 hours later, depending upon if a DST boundary was crossed.

    A patch for this would be welcome.

  • Michael Curtis

    Michael Curtis March 10th, 2009 @ 02:54 AM

    • Tag changed from active_support, duration, timezone to active_support, duration, patch, timezone

    This should take care of it. I've updated the unit tests in time_ext_test.rb that were enforcing the incorrect behavior, and added a couple tests to duration_test.rb that test this specific case.

    This is my first patch to rails.. if I did anything wrong please let me know!

  • Manfred Stienstra

    Manfred Stienstra March 10th, 2009 @ 04:40 PM

    Michael, can you remove the commented lines? Also, the f variable doesn't seem necessary.

  • Pratik

    Pratik March 10th, 2009 @ 04:55 PM

    • Assigned user set to “Geoff Buesing”
  • Geoff Buesing

    Geoff Buesing March 10th, 2009 @ 05:02 PM

    Also, no longer a need to go through Numeric#since -- i.e. seconds.since(self) -- we should now be able to call Time#+ directly, i.e. self + seconds.

    Time#since effectively becomes "plus with datetime fallback".

  • Michael Curtis

    Michael Curtis March 10th, 2009 @ 05:19 PM

    • Assigned user cleared.

    Here's a new patch, reduced to one commit and no longer going through Numeric#since.

  • Michael Curtis

    Michael Curtis March 10th, 2009 @ 05:19 PM

    • Assigned user set to “Geoff Buesing”
  • Geoff Buesing

    Geoff Buesing March 11th, 2009 @ 02:30 AM

    Revised patch looks good. I'll pull this in after 2.3 final is pushed. Thanks!

  • Repository

    Repository March 29th, 2009 @ 10:47 PM

    • State changed from “open” to “resolved”

    (from [5a8b481f717470b952ac7eb890f260ea98428153]) Time.local instances: Adding 24.hours across the DST boundary adds 24 hours instead of one day [#2066 state:resolved] http://github.com/rails/rails/co...

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
    • Importance changed from “” to “Low”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages