This project is archived and is in readonly mode.

#3769 ✓resolved
Joe Rafaniello

[Patch] ActiveSupport: Marshaling time objects modify the source object and leave around an instance variable affecting serialized attributes

Reported by Joe Rafaniello | January 21st, 2010 @ 10:20 PM

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
.

Comments and changes to this ticket

  • Joe Rafaniello

    Joe Rafaniello January 25th, 2010 @ 03:31 PM

    • 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

    Joe Rafaniello January 25th, 2010 @ 04:08 PM

    • 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

    Joe Rafaniello March 17th, 2010 @ 04:35 PM

    • 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

    Repository March 27th, 2010 @ 11:34 AM

    • 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...

  • csnk

    csnk May 18th, 2011 @ 08:19 AM

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>

People watching this ticket

Attachments

Tags

Referenced by

Pages