This project is archived and is in readonly mode.
[Patch] ActiveSupport: Marshaling time objects modify the source object and leave around an instance variable affecting serialized attributes
Marshaling a time object added an instance variable to the
object which affected the quoting of serialized attributes because
the yaml'ized version of the original object did not match the
marshaled one.
For example, after marshaling a time object such as Time.local(2000), the instance variable: marshal_with_utc_coercion is set but never removed. In addition, the client could Marshal an object and then use the original object and never know that it was modified.
See attached patch and test.
Before patch:
1) Failure:
test_marshaling_does_not_affect_yaml_dump(TimeExtMarshalingTest)
[core_ext/time_ext_test.rb:756:in `test_marshaling_does_not_affect_yaml_dump'
/usr/lib/ruby/gems/1.8/gems/mocha-0.9.8/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:19:in `__send__'
/usr/lib/ruby/gems/1.8/gems/mocha-0.9.8/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:19:in `run']:
<"--- 2000-01-01 00:00:00 -05:00\n"> expected but was
<"--- !timestamp \nat: "2000-01-01 00:00:00
-05:00"\n"@marshal_with_utc_coercion": false\n">.
2) Failure:
test_marshaling_does_not_modify_source_object(TimeExtMarshalingTest)
[core_ext/time_ext_test.rb:748:in `test_marshaling_does_not_modify_source_object'
/usr/lib/ruby/gems/1.8/gems/mocha-0.9.8/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:19:in `__send__'
/usr/lib/ruby/gems/1.8/gems/mocha-0.9.8/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:19:in `run']:
expected but was
.
-
marshaling_time.diff
2.36 KB
-
Joe Rafaniello- Assigned user set to Michael Koziarski
I'm not sure who to assign this ticket to so please forgive me if someone else works on ActiveSupport.
The original commit which fixed the unmarshaling of UTC time objects for pre-ruby 1.9:
http://github.com/rails/rails/commit/ce65a05c5b027175c3c541055081f8...This followup commit changed the marshal load to do a get instead of removing the instance variable if it exists:
http://github.com/rails/rails/commit/2c62baf4bf221aa8aa67f4625fe701...The problem with the followup commit above is that it adds the instance variable but never removes it. The original commit also makes changes to the object itself if it's not frozen. It's not clear to me that this method should ever modify the object in place. The provided patch attempts to fix this.
Any code that relies on the original object remaining "untouched" or that the unmarshaled object is the same as the original object will be wrong.
-
Joe Rafaniello
- Tag changed from activesupport, marshal, time to 2.2-stable, 2.3-stable, activesupport, marshal, time
- Title changed from Patch: ActiveSupport: Marshaling time objects modify the source object and leave around an instance variable affecting serialized attributes to [Patch] ActiveSupport: Marshaling time objects modify the source object and leave around an instance variable affecting serialized attributes
I forgot to mention that this issue was found in rails 2.2-stable but also is in 2.3-stable and current 3.0 code.
-
Joe Rafaniello
- Tag changed from 2.2-stable, 2.3-stable, activesupport, marshal, time to time zone, 2.2-stable, 2.3-stable, 3.0, activesupport, marshal, time
Adding tag for 3.0 code since this issue is still a problem even though the code has been moved around in 3.0. Ie, the bug still exists and is only moved in 3.0
The provided patch for 2.3 allows for us to properly marshal dump/load both local/utc time objects without modifying the object in place and without leaving around an instance variable after load which did not exist prior to dump.
-
Repository- State changed from new to resolved
(from [26e714d26eff2633fd33ba6f21295a376fcad091]) Remove stray instance variable to resolve serialization problem [#3769 state:resolved] (ht: Joe Rafaniello) http://github.com/rails/rails/commit/26e714d26eff2633fd33ba6f21295a...