ruby-changes:67989
From: Jean <ko1@a...>
Date: Wed, 15 Sep 2021 08:00:31 +0900 (JST)
Subject: [ruby-changes:67989] 89242279e6 (master): Marshal.load: do not call the proc until strings have their encoding
https://git.ruby-lang.org/ruby.git/commit/?id=89242279e6 From 89242279e61b023a81c58065c62a82de8829d0b3 Mon Sep 17 00:00:00 2001 From: Jean Boussier <jean.boussier@g...> Date: Wed, 1 Sep 2021 09:43:39 +0100 Subject: Marshal.load: do not call the proc until strings have their encoding Ref: https://bugs.ruby-lang.org/issues/18141 --- marshal.c | 7 +++- spec/ruby/core/marshal/shared/load.rb | 62 +++++++++++++++++++++++------------ test/ruby/test_marshal.rb | 17 ++++++++++ 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/marshal.c b/marshal.c index ea43c62..a817cf9 100644 --- a/marshal.c +++ b/marshal.c @@ -1690,6 +1690,9 @@ r_object0(struct load_arg *arg, int *ivp, VALUE extmod) https://github.com/ruby/ruby/blob/trunk/marshal.c#L1690 v = r_object0(arg, &ivar, extmod); if (ivar) r_ivar(v, NULL, arg); + if (RB_TYPE_P(v, T_STRING)) { + v = r_leave(v, arg); + } } break; @@ -1819,7 +1822,9 @@ r_object0(struct load_arg *arg, int *ivp, VALUE extmod) https://github.com/ruby/ruby/blob/trunk/marshal.c#L1822 case TYPE_STRING: v = r_entry(r_string(arg), arg); - v = r_leave(v, arg); + if (!ivp) { + v = r_leave(v, arg); + } break; case TYPE_REGEXP: diff --git a/spec/ruby/core/marshal/shared/load.rb b/spec/ruby/core/marshal/shared/load.rb index a79fb51..18f11d7 100644 --- a/spec/ruby/core/marshal/shared/load.rb +++ b/spec/ruby/core/marshal/shared/load.rb @@ -20,6 +20,24 @@ describe :marshal_load, shared: true do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/marshal/shared/load.rb#L20 end describe "when called with a proc" do + ruby_bug "#18141", ""..."3.1" do + it "call the proc with fully initialized strings" do + utf8_string = "foo".encode(Encoding::UTF_8) + Marshal.send(@method, Marshal.dump(utf8_string), proc { |arg| + if arg.is_a?(String) + arg.should == utf8_string + arg.encoding.should == Encoding::UTF_8 + end + arg + }) + end + + it "no longer mutate the object after it was passed to the proc" do + string = Marshal.load(Marshal.dump("foo"), :freeze.to_proc) + string.should.frozen? + end + end + it "returns the value of the proc" do Marshal.send(@method, Marshal.dump([1,2]), proc { [3,4] }).should == [3,4] end @@ -35,27 +53,29 @@ describe :marshal_load, shared: true do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/marshal/shared/load.rb#L53 ret.size.should == 3 end - it "loads an Array with proc" do - arr = [] - s = 'hi' - s.instance_variable_set(:@foo, 5) - st = Struct.new("Brittle", :a).new - st.instance_variable_set(:@clue, 'none') - st.a = 0.0 - h = Hash.new('def') - h['nine'] = 9 - a = [:a, :b, :c] - a.instance_variable_set(:@two, 2) - obj = [s, 10, s, s, st, a] - obj.instance_variable_set(:@zoo, 'ant') - proc = Proc.new { |o| arr << o; o} - - Marshal.send(@method, "\x04\bI[\vI\"\ahi\a:\x06EF:\t@fooi\ni\x0F@\x06@\x06IS:\x14Struct::Brittle\x06:\x06af\x060\x06:\n@clueI\"\tnone\x06;\x00FI[\b;\b:\x06b:\x06c\x06:\t@twoi\a\x06:\t@zooI\"\bant\x06;\x00F", proc) - - arr.should == ["hi", false, 5, 10, "hi", "hi", 0.0, st, "none", false, - :b, :c, a, 2, ["hi", 10, "hi", "hi", st, [:a, :b, :c]], "ant", false] - - Struct.send(:remove_const, :Brittle) + ruby_bug "#18141", ""..."3.1" do + it "loads an Array with proc" do + arr = [] + s = 'hi' + s.instance_variable_set(:@foo, 5) + st = Struct.new("Brittle", :a).new + st.instance_variable_set(:@clue, 'none') + st.a = 0.0 + h = Hash.new('def') + h['nine'] = 9 + a = [:a, :b, :c] + a.instance_variable_set(:@two, 2) + obj = [s, 10, s, s, st, a] + obj.instance_variable_set(:@zoo, 'ant') + proc = Proc.new { |o| arr << o; o} + + Marshal.send(@method, "\x04\bI[\vI\"\ahi\a:\x06EF:\t@fooi\ni\x0F@\x06@\x06IS:\x14Struct::Brittle\x06:\x06af\x060\x06:\n@clueI\"\tnone\x06;\x00FI[\b;\b:\x06b:\x06c\x06:\t@twoi\a\x06:\t@zooI\"\bant\x06;\x00F", proc) + + arr.should == [false, 5, "hi", 10, "hi", "hi", 0.0, st, false, "none", + :b, :c, a, 2, ["hi", 10, "hi", "hi", st, [:a, :b, :c]], false, "ant"] + + Struct.send(:remove_const, :Brittle) + end end end diff --git a/test/ruby/test_marshal.rb b/test/ruby/test_marshal.rb index 5e6891a..a0d9d96 100644 --- a/test/ruby/test_marshal.rb +++ b/test/ruby/test_marshal.rb @@ -676,6 +676,23 @@ class TestMarshal < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_marshal.rb#L676 assert_equal(['X', 'X'], Marshal.load(Marshal.dump(obj), ->(v) { v == str ? v.upcase : v })) end + def test_marshal_proc_string_encoding + string = "foo" + payload = Marshal.dump(string) + Marshal.load(payload, ->(v) { + if v.is_a?(String) + assert_equal(string, v) + assert_equal(string.encoding, v.encoding) + end + v + }) + end + + def test_marshal_proc_freeze + object = { foo: [42, "bar"] } + assert_equal object, Marshal.load(Marshal.dump(object), :freeze.to_proc) + end + def test_marshal_load_extended_class_crash assert_separately([], "#{<<-"begin;"}\n#{<<-"end;"}") begin; -- cgit v1.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/