Bug #14314: Marshalling broken in Ruby 2.5.0 for Structs with keyword_init: true

• Status: Closed
• Assignee: k0kubun (Takashi Kokubun)
• Target version: -
• ruby -v: ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-linux]
• Backport: 2.3: DONTNEED, 2.4: DONTNEED, 2.5: DONE

[jurriaan (Jurriaan Pruis)]

Steps to reproduce:

irb(main):001:0> Foo = Struct.new(:foo)
=> Foo
irb(main):002:0> Marshal.load(Marshal.dump(Foo.new('a')))
=> #<struct Foo foo="a">
irb(main):003:0> Bar = Struct.new(:bar, keyword_init: true)
=> Bar(keyword_init: true)
irb(main):004:0> Marshal.load(Marshal.dump(Bar.new(bar: 'a')))
Traceback (most recent call last):
        3: from /home/jurriaan/.rubies/ruby-2.5.0/bin/irb:11:in `<main>'
        2: from (irb):4
        1: from (irb):4:in `load'
ArgumentError (wrong number of arguments (given 1, expected 0))

I expected the keyword_init: true struct to unmarshal correctly.

This issue is caused by marshal.c calling the struct initializer with regular arguments instead of keyword arguments.

[jurriaan (Jurriaan Pruis)]

• Description updated (diff)

[Hanmac (Hans Mackowiak)]

ah yeah i see the problem

comparing the dump of the two cases, the dump doesn't know that the struct might need keyword_init (might not need to know?)

the problem is that its hard coded and can't be overwritten by own dump/load methods
https://github.com/ruby/ruby/blob/trunk/marshal.c#L1792-L1830
i think there need to be a check added if the struct wants keyword_init

[k0kubun (Takashi Kokubun)]

• Status changed from Open to Closed

Applied in changeset trunk|r61616.

---

marshal.c: allow marshalling keyword_init struct

struct.c: define rb_struct_s_keyword_init to shared with marshal.c

internal.h: add the declaration to be used by marshal.c

test/ruby/test_marshal.rb: add test for Bug#14314

[Feature #14314] [ruby-core:84629]

[k0kubun (Takashi Kokubun)]

• Assignee set to k0kubun (Takashi Kokubun)
• Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN to 2.3: DONTNEED, 2.4: DONTNEED, 2.5: REQUIRED

Thank you to catch this. I fixed this in r61616.

[shan (Shannon Skipper)]

k0kubun (Takashi Kokubun) wrote:

Thank you to catch this. I fixed this in r61616.

Will this also fix YAML deserialization or is that a separate issue entirely?

[k0kubun (Takashi Kokubun)]

Will this also fix YAML deserialization or is that a separate issue entirely?

Please file another issue for it.

[shan (Shannon Skipper)]

The YAML issue has also been fixed on trunk.

[naruse (Yui NARUSE)]

• Backport changed from 2.3: DONTNEED, 2.4: DONTNEED, 2.5: REQUIRED to 2.3: DONTNEED, 2.4: DONTNEED, 2.5: DONE

ruby_2_5 r62428 merged revision(s) 61616.

[k0kubun (Takashi Kokubun)]

• Related to Feature #15222: Add a way to distinguish between Struct classes with and without keyword initializer added

[k0kubun (Takashi Kokubun)]

• Related to deleted (Feature #15222: Add a way to distinguish between Struct classes with and without keyword initializer)