Cyclic references in Set objects raise exception with `YAML#load`

[mcorino]

Environment Information

JRuby version:

jruby 9.4.8.0 (3.1.4) 2024-07-02 4d41e55a67 OpenJDK 64-Bit Server VM 17.0.12+7-suse-150400.3.45.1-x8664 on 17.0.12+7-suse-150400.3.45.1-x8664 +jit [x86_64-linux]

OS:

Linux 5.14.21-150500.55.73-default #1 SMP PREEMPT_DYNAMIC Tue Aug 6 15:51:33 UTC 2024 (a0ede6a) x86_64 x86_64 x86_64 GNU/Linux

Creating a Set object with a cyclic reference and serializing with YAML and subsequently deserializing will result in an exception in YAML#load as can be reproduced by the following code snippit:

require 'set'
require 'yaml'

set = Set.new([:one, :two])
set << set
s = set.to_yaml
YAML.load(s, permitted_classes: [Set, Symbol], aliases: true)

Expected Behavior

As the JRuby YAML implementation correctly serializes the Set object and it's cyclic reference I expected deserialization to work correctly as well as it does in CRuby/MRI.

Actual Behavior

The following exception is raised:

org/jruby/ext/psych/PsychParser.java:308:in `_native_parse': (<unknown>): could not find expected ':' while scanning a simple key at line 6 column 1 (Psych::SyntaxError)
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/lib/ruby/stdlib/psych/parser.rb:62:in `parse'
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/lib/ruby/stdlib/psych.rb:455:in `parse_stream'
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/lib/ruby/stdlib/psych.rb:399:in `parse'
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/lib/ruby/stdlib/psych.rb:323:in `safe_load'
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/lib/ruby/stdlib/psych.rb:369:in `load'
        from (irb):7:in `evaluate'
        from org/jruby/RubyKernel.java:1274:in `eval'
        from org/jruby/RubyKernel.java:1722:in `loop'
        from org/jruby/RubyKernel.java:1423:in `catch'
        from org/jruby/RubyKernel.java:1423:in `catch'
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/lib/ruby/gems/shared/gems/irb-1.4.2/exe/irb:11:in `<main>'
        from org/jruby/RubyKernel.java:1220:in `load'
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/bin/irb:25:in `<main>'
        from org/jruby/RubyKernel.java:1274:in `eval'
        from /home/martin/.rvm/rubies/jruby-9.4.8.0/bin/jruby_executable_hooks:22:in `<main>'
org.snakeyaml.engine.v2.scanner.ScannerImpl.stalePossibleSimpleKeys(ScannerImpl.java:460): while scanning a simple key (Java::OrgSnakeyamlEngineV2Exceptions::ScannerException)
 in reader, line 5, column 3:
      *1: true
      ^
could not find expected ':'
 in reader, line 6, column 1:
    
    ^

        from org.snakeyaml.engine.v2.scanner.ScannerImpl.needMoreTokens(ScannerImpl.java:279)
        from org.snakeyaml.engine.v2.scanner.ScannerImpl.checkToken(ScannerImpl.java:191)
        from org.snakeyaml.engine.v2.parser.ParserImpl$ParseBlockMappingKey.produce(ParserImpl.java:711)
        from org.snakeyaml.engine.v2.parser.ParserImpl.lambda$produce$1(ParserImpl.java:232)
        from java.base/java.util.Optional.ifPresent(Optional.java:178)
        from org.snakeyaml.engine.v2.parser.ParserImpl.produce(ParserImpl.java:232)
        from org.snakeyaml.engine.v2.parser.ParserImpl.hasNext(ParserImpl.java:226)
        from org.jruby.ext.psych.PsychParser.parse(PsychParser.java:248)
        from org.jruby.ext.psych.PsychParser$INVOKER$i$3$0$parse.call(PsychParser$INVOKER$i$3$0$parse.gen)
        from org.jruby.dist/org.jruby.internal.runtime.methods.JavaMethod$JavaMethodThree.call(JavaMethod.java:1371)
        from org.jruby.dist/org.jruby.internal.runtime.methods.JavaMethod$JavaMethodN.call(JavaMethod.java:847)
        from org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:446)
        from org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:92)
        from org.jruby.dist/org.jruby.ir.instructions.CallBase.interpret(CallBase.java:548)
        from org.jruby.dist/org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:363)
        from org.jruby.dist/org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:66)
        ... 197 levels...

[headius]

The resulting YAML does not appear to verify at any of the online YAML verifiers I tried: https://www.google.com/search?q=yaml+verifier

--- &1 !ruby/object:Set
hash:
  :one: true
  :two: true
  *1: true

The errors vary:

• Implicit map keys need to be followed by map values at line 5, column 3
• unidentified alias "1:" at line 5, column 6
• unidentified alias "1:" (5:6)

I notice, however, that the CRuby output is slightly different, with a space between the *1 and the : true:

--- &1 !ruby/object:Set
hash:
  :one: true
  :two: true
  *1 : true

If I feed this into the YAML.load call, I get an internal error in JRuby that appears to be a bug in how our Set implementation interacts with the to_ruby logic in Psych:

$ jruby -X+C blah.rb
Unhandled Java exception: java.lang.NullPointerException: Cannot invoke "org.jruby.RubyHash.hash()" because "this.hash" is null
java.lang.NullPointerException: Cannot invoke "org.jruby.RubyHash.hash()" because "this.hash" is null
                                       hash at org/jruby/ext/set/RubySet.java:927
                                       call at org/jruby/ext/set/RubySet$INVOKER$i$0$0$hash.gen:-1
                              invokedynamic at org/jruby/runtime/Helpers.java:2709
                                   hashCode at org/jruby/RubyObject.java:440
                                  hashValue at org/jruby/RubyHash.java:536
                        internalPutNoResize at org/jruby/RubyHash.java:586
                                internalPut at org/jruby/RubyHash.java:558
                        fastASetCheckString at org/jruby/RubyHash.java:1156
                                    op_aset at org/jruby/RubyHash.java:1191
                                       call at org/jruby/RubyHash$INVOKER$i$2$0$op_aset.gen:-1
                                       call at org/jruby/runtime/callsite/CachingCallSite.java:291
                     invokeOther486:\=\{\}= at Users/headius/work/jruby/lib/ruby/gems/shared/gems/psych_minus_5_dot_1_dot_2_minus_java/lib/psych/visitors//Users/headius/work/jruby/lib/ruby/gems/shared/gems/psych-5.1.2-java/lib/psych/visitors/to_ruby.rb:377
                  RUBY$block$revive_hash$46 at Users/headius/work/jruby/lib/ruby/gems/shared/gems/psych_minus_5_dot_1_dot_2_minus_java/lib/psych/visitors//Users/headius/work/jruby/lib/ruby/gems/shared/gems/psych-5.1.2-java/lib/psych/visitors/to_ruby.rb:377

This error happens because Psych tries to just allocate the Set object and then use it as a hash key while it is being populated, but if initialize does not get called then our internal Hash instance does not get allocated (leading to the NullPointerException here).

So I think there's two bugs here:

• The to_yaml output does not handle the alias properly and misses a space after the alias name
• The Set fails to be recreated because we do not create its internal Hash instance early enough

@kares You wrote most of this native Set impl, what do you think about the NPE here? Should we be creating the Hash in the constructor instead, given this use case? FWIW I don't like that Psych is passing around an object that has been allocateed but not initializeed but that's how it works right now.

The bad output I will look into.

[headius]

@asomov Is this alias output a bug?

SnakeYAML Engine emitter:

--- &1 !ruby/object:Set
hash:
  :one: true
  :two: true
  *1: true

libyaml emitter:

--- &1 !ruby/object:Set
hash:
  :one: true
  :two: true
  *1 : true

The SnakeYAML output does not appear to be valid YAML, missing the space after the *1 alias.

[asomov]

@headius I created a ticket to investigate

[mcorino]

I fed the YAML output from JRuby to MRI's YAML.load without any problems.

[headius]

I fed the YAML output from JRuby to MRI's YAML.load without any problems.

That may simply be because CRuby uses less-strict YAML 1.1. JRuby's YAML engine is YAML 1.2.

[asomov]

The bug in SnakeYAML Engine is reproduced. We will have a look.

[asomov]

It is related to the previous issue:
https://bitbucket.org/snakeyaml/snakeyaml/issues/485/alias-names-are-too-permissive-compared-to

It is not yet solved in the spec: yaml/yaml-spec#75

When anchor/alias may not contain a colon, the space will not be required. SnakeYAML Engine should be fixed.

[asomov]

the explanation for libyaml:
yaml/libyaml#205 (comment)

[asomov]

@headius please check the changes in the tests
The colon is not accepted now in the name of anchor and the extra separating space is not required

[asomov]

@headius I assume the PR in SnakeYAML delivers what is expected.

[headius]

@asomov Thanks for the quick turn around on that fix Andrey! I'm out of town for the weekend but I will confirm this early next week.

[asomov]

@headius I will merge the PR

[headius]

@asomov Sorry for the delay, I have confirmed the fix.

@mcorino I believe this will be fixed with the patch from @asomov and an additional patch I will work out with @kares (requires JRuby release) for the Set allocation issue:

$ jruby blah.rb
"--- &1 !ruby/object:Set\nhash:\n  :one: true\n  :two: true\n  *1: true\n"
#<Set: {:one, :two, #<Set: {...}>}>
$ ruby33 blah.rb
"--- &1 !ruby/object:Set\nhash:\n  :one: true\n  :two: true\n  *1 : true\n"
#<Set: {:one, :two, #<Set: {...}>}>