Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Time.zone.parse uses system TZ data for the DST jump even if Time.zone differs #5559

Closed
jarkko opened this Issue · 1 comment

2 participants

Jarkko Laine Aaron Patterson
Jarkko Laine

Current behavior / how to reproduce

If Time.zone is different than the system timezone, and the time when the two zones switch to DST differ, Time.zone.parse will black out the missing hour during the jump to daylight savings time based on both the selected Time.zone and the system timezone.

# system timezone is (GMT+02:00) EET
1.9.3p125 :078 >   Time.zone = "Pacific Time (US & Canada)"
 => "Pacific Time (US & Canada)" 
1.9.3p125 :079 > Time.zone
 => (GMT-08:00) Pacific Time (US & Canada) 
1.9.3p125 :080 > Time.zone.parse("2012-03-11 02:29")
 => Sun, 11 Mar 2012 03:29:00 PDT -07:00 # This is correct
1.9.3p125 :081 > Time.zone.parse("2012-03-25 03:29")
 => Sun, 25 Mar 2012 04:29:00 PDT -07:00 # This is wrong

Expected behavior

Only the jump hour based on Time.zone should be missing.

Root of the problem

Time.parse uses the system timezone if none is specified in the parameter string and as such will automatically black out the missing hour based on the system clock. DateTime.parse and Date._parse do not seem to black out any hours. However, ActiveSupport::TimeWithZone.new will take care of the possible jump, so there is no need to rely on the system clock for this. Thus, the simplest solution would be to solely rely on DateTime.parse in TimeZone#parse. However, it doesn't take a chosen now as an optional argument like Time.parse does.

A solution that fixes the issue but otherwise exactly retains the previous behavior is to check that the hours in the outputs of Time.parse and Date._parse match. If they don't, we're clearly within the jump to DST based on the system clock and should just rely on ActiveSupport::TimeWithZone to do the hour shift if needed. In that the case the now parameter is kind of irrelevant for the hour anyway, because AFAIK you cannot leave the hour away from the string passed to parse.

A patch with tests will follow.

Jarkko Laine jarkko referenced this issue from a commit in jarkko/rails
Jarkko Laine jarkko [#5559] Do not black out the system timezone DST jump hour if Time.zo…
…ne differs from that.

The system timezone DST jump hour should not be blacked out by Time.zone.parse if current Time.zone does not do the jump at that time.

Fixes #5559.
bb4a1d6
Jarkko Laine

A proposed fix: jarkko@bb4a1d6

Jarkko Laine jarkko referenced this issue from a commit in jarkko/rails
Jarkko Laine jarkko [#5559] Do not black out the system timezone DST jump hour if Time.zo…
…ne differs from that.

The system timezone DST jump hour should not be blacked out by Time.zone.parse if current Time.zone does not do the jump at that time.

Fixes #5559.
03becb1
Aaron Patterson tenderlove closed this issue from a commit
Jarkko Laine jarkko [#5559] Do not black out the system timezone DST jump hour if Time.zo…
…ne differs from that.

The system timezone DST jump hour should not be blacked out by Time.zone.parse if current Time.zone does not do the jump at that time.

Fixes #5559.
03becb1
Nicklas Ramhöj ramhoj referenced this issue from a commit in ramhoj/time-zone-article
Nicklas Ramhöj ramhoj Remove chapter on Time.zone.parse bug.
@jarkko’s patch has been accepted and released
in moden versions of Rails.

See: rails/rails#5559
722c251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.