Skip to content

Commit

Permalink
Fix Ruby's Time marshaling bug in pre-1.9 versions of Ruby: utc insta…
Browse files Browse the repository at this point in the history
…nces are now correctly unmarshaled with a utc zone instead of the system local zone [#900 state:resolved]
  • Loading branch information
Luca Guidi authored and gbuesing committed Aug 27, 2008
1 parent e710902 commit 4d71e99
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 1 deletion.
2 changes: 2 additions & 0 deletions activesupport/CHANGELOG
@@ -1,5 +1,7 @@
*2.1.1 (next release)*

* Fix Ruby's Time marshaling bug in pre-1.9 versions of Ruby: utc instances are now correctly unmarshaled with a utc zone instead of the system local zone [#900 state:resolved]:activesupport/CHANGELOG

* TimeWithZone: when crossing DST boundary, treat Durations of days, months or years as variable-length, and all other values as absolute length. A time + 24.hours will advance exactly 24 hours, but a time + 1.day will advance 23-25 hours, depending on the day. Ensure consistent behavior across all advancing methods [Geoff Buesing]

* Fix TimeWithZone unmarshaling: coerce unmarshaled Time instances to utc, because Ruby's marshaling of Time instances doesn't respect the zone [Geoff Buesing]
Expand Down
23 changes: 22 additions & 1 deletion activesupport/lib/active_support/core_ext/time.rb
@@ -1,11 +1,32 @@
require 'date'
require 'time'

# Ruby 1.8-cvs and 1.9 define private Time#to_date
class Time
# Ruby 1.8-cvs and 1.9 define private Time#to_date
%w(to_date to_datetime).each do |method|
public method if private_instance_methods.include?(method)
end

# Pre-1.9 versions of Ruby have a bug with marshaling Time instances, where utc instances are
# unmarshaled in the local zone, instead of utc. We're layering behavior on the _dump and _load
# methods so that utc instances can be flagged on dump, and coerced back to utc on load.
if RUBY_VERSION < '1.9'
class << self
alias_method :_original_load, :_load
def _load(marshaled_time)
time = _original_load(marshaled_time)
utc = time.send(:remove_instance_variable, '@marshal_with_utc_coercion')
utc ? time.utc : time
end
end

alias_method :_original_dump, :_dump
def _dump(*args)
obj = self.frozen? ? self.dup : self
obj.instance_variable_set('@marshal_with_utc_coercion', utc?)
obj._original_dump(*args)
end
end
end

require 'active_support/core_ext/time/behavior'
Expand Down
34 changes: 34 additions & 0 deletions activesupport/test/core_ext/time_ext_test.rb
Expand Up @@ -620,3 +620,37 @@ def with_env_tz(new_tz = 'US/Eastern')
old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
end
end

class TimeExtMarshalingTest < Test::Unit::TestCase
def test_marshaling_with_utc_instance
t = Time.utc(2000)
marshaled = Marshal.dump t
unmarshaled = Marshal.load marshaled
assert_equal t, unmarshaled
assert_equal t.zone, unmarshaled.zone
end

def test_marshaling_with_local_instance
t = Time.local(2000)
marshaled = Marshal.dump t
unmarshaled = Marshal.load marshaled
assert_equal t, unmarshaled
assert_equal t.zone, unmarshaled.zone
end

def test_marshaling_with_frozen_utc_instance
t = Time.utc(2000).freeze
marshaled = Marshal.dump t
unmarshaled = Marshal.load marshaled
assert_equal t, unmarshaled
assert_equal t.zone, unmarshaled.zone
end

def test_marshaling_with_frozen_local_instance
t = Time.local(2000).freeze
marshaled = Marshal.dump t
unmarshaled = Marshal.load marshaled
assert_equal t, unmarshaled
assert_equal t.zone, unmarshaled.zone
end
end

3 comments on commit 4d71e99

@han
Copy link
Contributor

@han han commented on 4d71e99 Sep 3, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks in jruby (tested with 1.1.4)

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on 4d71e99 Sep 4, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be a bug in jruby’s unmarshalling code, we’ve reported it upstream and may build our own work around for it.

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on 4d71e99 Sep 4, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve shipped a work around for this. It’s still misbehaving in jruby, but the behaviour should at least not cause errors

Please sign in to comment.