ruby/psych

Psych.load fails to load a Hash subclass with ivars accessed on []=

Closed this issue · 3 comments

Currently, Psych can't load classes like below (real world example Faraday::Utils::Headers)

    class HashWithCustomSetter < Hash
      def initialize
        @keys_copy = []
        super
      end

      def []=(k, v)
        @keys_copy << k
        super
      end
    end

    def test_hash_with_custom_setter
      t1 = HashWithCustomSetter.new
      t1[:foo] = 'bar'
      t2 = Psych.load(Psych.dump(t1))
      assert_equal t1, t2
    end

Problem being

NoMethodError: undefined method `<<' for nil:NilClass
	from (irb):9:in `[]='
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:362:in `block in revive_hash'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:336:in `each'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:336:in `each_slice'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:336:in `revive_hash'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:271:in `block in visit_Psych_Nodes_Mapping'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:268:in `each'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:268:in `each_slice'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/to_ruby.rb:268:in `visit_Psych_Nodes_Mapping'
	from /Users/md/.rubies/ruby-2.3.3/lib/ruby/2.3.0/psych/visitors/visitor.rb:16:in `visit'

I was playing a bit with to_ruby visitor and the naive solution (iterate separately on ivars and later on elements) fixes this, but it's two iterations instead of one.

As I'm not very familiar with the codebase, my questions are:

  • do we want Psych to handle such classes? (Syck does handle them)
  • is my proposed solution acceptable?

@irvingwashington the problem is that this visitor does allocate your object, but didn't call the initialize method. An way to fix that would be to add:

 @keys_copy ||= []

into the []= method.

I've sent up a PR to demonstrate this issue. I think the solution would be to either make sure the parser processes ivars before elements or changing the emitter so that ivars come before elements in the YAML output.

See: #383

Fixed in a26d9c3. Thanks!