Project

General

Profile

Actions

Bug #14314

closed

Marshalling broken in Ruby 2.5.0 for Structs with keyword_init: true

Added by jurriaan (Jurriaan Pruis) about 6 years ago. Updated about 6 years ago.

Status:
Closed
Target version:
-
ruby -v:
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-linux]
[ruby-core:84629]

Description

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.

Actions #1

Updated by jurriaan (Jurriaan Pruis) about 6 years ago

  • Description updated (diff)

Updated by Hanmac (Hans Mackowiak) about 6 years ago

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

Actions #3

Updated by k0kubun (Takashi Kokubun) about 6 years ago

  • 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]

Updated by k0kubun (Takashi Kokubun) about 6 years ago

  • 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.

Updated by shan (Shannon Skipper) about 6 years ago

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?

Updated by k0kubun (Takashi Kokubun) about 6 years ago

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

Please file another issue for it.

Updated by shan (Shannon Skipper) about 6 years ago

The YAML issue has also been fixed on trunk.

Updated by naruse (Yui NARUSE) about 6 years ago

  • 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.

Actions #9

Updated by k0kubun (Takashi Kokubun) over 5 years ago

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

Updated by k0kubun (Takashi Kokubun) over 5 years ago

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

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0