diff --git a/ChangeLog b/ChangeLog index 3f07725f..01cb919d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,5 @@ +* Fix an infinite recursion issue when registering a Symbol type with a `nil` packer. + 2023-03-29 1.7.0: * Fix a possible double-free issue when GC triggers inside `_msgpack_rmem_alloc2`. diff --git a/ext/java/org/msgpack/jruby/ExtensionRegistry.java b/ext/java/org/msgpack/jruby/ExtensionRegistry.java index c6ce7f20..088e9dd0 100644 --- a/ext/java/org/msgpack/jruby/ExtensionRegistry.java +++ b/ext/java/org/msgpack/jruby/ExtensionRegistry.java @@ -141,11 +141,11 @@ public boolean isRecursive() { } public boolean hasPacker() { - return packerProc != null; + return packerProc != null && !packerProc.isNil(); } public boolean hasUnpacker() { - return unpackerProc != null; + return unpackerProc != null && !unpackerProc.isNil(); } public IRubyObject getPackerProc() { diff --git a/ext/java/org/msgpack/jruby/Factory.java b/ext/java/org/msgpack/jruby/Factory.java index c5a5661c..8eb7b41e 100644 --- a/ext/java/org/msgpack/jruby/Factory.java +++ b/ext/java/org/msgpack/jruby/Factory.java @@ -102,7 +102,13 @@ public IRubyObject registerType(ThreadContext ctx, IRubyObject[] args) { if (args[args.length - 1] instanceof RubyHash) { options = (RubyHash) args[args.length - 1]; packerArg = options.fastARef(runtime.newSymbol("packer")); + if (packerArg != null && packerArg.isNil()) { + packerArg = null; + } unpackerArg = options.fastARef(runtime.newSymbol("unpacker")); + if (unpackerArg != null && unpackerArg.isNil()) { + unpackerArg = null; + } IRubyObject optimizedSymbolsParsingArg = options.fastARef(runtime.newSymbol("optimized_symbols_parsing")); if (optimizedSymbolsParsingArg != null && optimizedSymbolsParsingArg.isTrue()) { throw runtime.newArgumentError("JRuby implementation does not support the optimized_symbols_parsing option"); @@ -149,7 +155,7 @@ public IRubyObject registerType(ThreadContext ctx, IRubyObject[] args) { extensionRegistry.put(extModule, (int) typeId, recursive, packerProc, packerArg, unpackerProc, unpackerArg); - if (extModule == runtime.getSymbol()) { + if (extModule == runtime.getSymbol() && !packerProc.isNil()) { hasSymbolExtType = true; } diff --git a/ext/java/org/msgpack/jruby/Packer.java b/ext/java/org/msgpack/jruby/Packer.java index 453ae67d..50ffce07 100644 --- a/ext/java/org/msgpack/jruby/Packer.java +++ b/ext/java/org/msgpack/jruby/Packer.java @@ -129,7 +129,7 @@ public IRubyObject registerType(ThreadContext ctx, IRubyObject[] args, final Blo registry.put(extModule, (int) typeId, false, proc, arg, null, null); - if (extModule == runtime.getSymbol()) { + if (extModule == runtime.getSymbol() && !proc.isNil()) { encoder.hasSymbolExtType = true; } diff --git a/ext/msgpack/factory_class.c b/ext/msgpack/factory_class.c index 8b2376aa..b9235a5a 100644 --- a/ext/msgpack/factory_class.c +++ b/ext/msgpack/factory_class.c @@ -222,11 +222,12 @@ static VALUE Factory_register_type(int argc, VALUE* argv, VALUE self) unpacker_arg = ID2SYM(rb_intern("from_msgpack_ext")); break; case 3: - /* register_type(0x7f, Time, packer: proc-like, unapcker: proc-like) */ + /* register_type(0x7f, Time, packer: proc-like, unpacker: proc-like) */ options = argv[2]; if(rb_type(options) != T_HASH) { rb_raise(rb_eArgError, "expected Hash but found %s.", rb_obj_classname(options)); } + packer_arg = rb_hash_aref(options, ID2SYM(rb_intern("packer"))); unpacker_arg = rb_hash_aref(options, ID2SYM(rb_intern("unpacker"))); break; @@ -266,7 +267,9 @@ static VALUE Factory_register_type(int argc, VALUE* argv, VALUE self) } if(ext_module == rb_cSymbol) { - fc->has_symbol_ext_type = true; + if(NIL_P(options) || RTEST(rb_hash_aref(options, ID2SYM(rb_intern("packer"))))) { + fc->has_symbol_ext_type = true; + } if(RTEST(options) && RTEST(rb_hash_aref(options, ID2SYM(rb_intern("optimized_symbols_parsing"))))) { fc->optimized_symbol_ext_type = true; } diff --git a/spec/factory_spec.rb b/spec/factory_spec.rb index 7d1f0456..37672f7e 100644 --- a/spec/factory_spec.rb +++ b/spec/factory_spec.rb @@ -249,6 +249,12 @@ class MyType2 < MyType my.b.should == 2 end + it 'handles Symbol type with `packer: nil`' do + factory = MessagePack::Factory.new + factory.register_type(0x00, Symbol, packer: nil) + expect(factory.load(factory.dump(:foo))).to be == "foo" + end + describe "registering multiple ext type for the same class" do let(:payload) do factory = MessagePack::Factory.new