Bug #9675: Marshal.load fails with recursive structures and user defined hash method

• Status: Rejected
• Assignee: -
• Target version: -
• ruby -v: ruby 2.1.0p0 (2013-12-25 revision 44422) [x86_64-darwin13.0]
• Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN

[ccutrer (Cody Cutrer)]

If a user class redefines hash to something that depends on instance variables, and the object is loaded both before a hash, and as a key of a hash of one of its own instance variables (that's loaded before the instance variables needed for the #hash method), it will fail.

It seems like the hash should be constructed during loading without calling #hash, and then after the load has completed, call #rehash on all of the loaded hashes. This should fix any form of nested data structures.

I can repro in 1.9.3p286, 1.9.3p484, and 2.1.0p0 at the least. I discovered when upgrading a far more complicated application from 1.9.3p286 to 1.9.3p484 caused a change in the order of instance variables, thereby triggering the issue. My reduced test case (attached) hits the issue in both versions, though.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Rejected

Cody Cutrer wrote:

It seems like the hash should be constructed during loading without calling #hash, and then after the load has completed, call #rehash on all of the loaded hashes. This should fix any form of nested data structures.

Before the hash get constructed, you can't access @b['id'].

You should use marshal_dump and marshal_load instead.

class A
  attr_accessor :a, :b
  def hash
    @b ? @b['id'].hash : super
  end

  def marshal_dump
    [@a, @b]
  end
  def marshal_load((a, b))
    @a = a
    @b = b
    a.rehash if a
  end
end

a = A.new
a.a = nil
a.b = {'id' => 1}
a.a = {a => 1}

Marshal.load(Marshal.dump(a))

[ccutrer (Cody Cutrer)]

There is no documentation for hash that if you override it, you should not use instance variables. Either this needs to be prevented by ruby, or the documentation needs to be updated. The actual case that triggered this was ActiveRecord::Base, which overrides hash, but not marshal_dump or marshal_load.