Project

General

Profile

Bug #14314

Marshalling broken in Ruby 2.5.0 for Structs with keyword_init: true

Added by jurriaan (Jurriaan Pruis) almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
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.

Associated revisions

Revision 26df9588
Added by k0kubun (Takashi Kokubun) almost 2 years ago

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]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@61616 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 61616
Added by k0kubun (Takashi Kokubun) almost 2 years ago

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]

Revision 61616
Added by k0kubun (Takashi Kokubun) almost 2 years ago

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]

Revision 15f641a5
Added by naruse (Yui NARUSE) over 1 year ago

merge revision(s) 61616: [Backport #14314]

    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]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@62428 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 62428
Added by naruse (Yui NARUSE) over 1 year ago

merge revision(s) 61616: [Backport #14314]

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]

History

#1

Updated by jurriaan (Jurriaan Pruis) almost 2 years ago

  • Description updated (diff)

Updated by Hanmac (Hans Mackowiak) almost 2 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

#3

Updated by k0kubun (Takashi Kokubun) almost 2 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) almost 2 years ago

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

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

Updated by shan (Shannon Skipper) almost 2 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) almost 2 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) almost 2 years ago

The YAML issue has also been fixed on trunk.

Updated by naruse (Yui NARUSE) over 1 year 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.

#9

Updated by k0kubun (Takashi Kokubun) about 1 year ago

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

Updated by k0kubun (Takashi Kokubun) about 1 year ago

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

Also available in: Atom PDF