Merge pull request #2387 from herwinw/marshal_restore_ivar

Fixes to Marshal.load with ivars

spec/core/marshal/dump_spec.rb

@@ -438,18 +438,16 @@ def _dump(level)
 
     it "dumps an ascii-compatible Regexp" do
       o = Regexp.new("a".encode("us-ascii"), Regexp::FIXEDENCODING)
-      NATFIXME 'dumps an ascii-compatible Regexp', exception: SpecFailedException do
-        Marshal.dump(o).should == "\x04\bI/\x06a\x10\x06:\x06EF"
+      Marshal.dump(o).should == "\x04\bI/\x06a\x10\x06:\x06EF"
 
-        o = Regexp.new("a".encode("us-ascii"))
-        Marshal.dump(o).should == "\x04\bI/\x06a\x00\x06:\x06EF"
+      o = Regexp.new("a".encode("us-ascii"))
+      Marshal.dump(o).should == "\x04\bI/\x06a\x00\x06:\x06EF"
 
-        o = Regexp.new("a".encode("windows-1251"), Regexp::FIXEDENCODING)
-        Marshal.dump(o).should == "\x04\bI/\x06a\x10\x06:\rencoding\"\x11Windows-1251"
+      o = Regexp.new("a".encode("windows-1251"), Regexp::FIXEDENCODING)
+      Marshal.dump(o).should == "\x04\bI/\x06a\x10\x06:\rencoding\"\x11Windows-1251"
 
-        o = Regexp.new("a".encode("windows-1251"))
-        Marshal.dump(o).should == "\x04\bI/\x06a\x00\x06:\x06EF"
-      end
+      o = Regexp.new("a".encode("windows-1251"))
+      Marshal.dump(o).should == "\x04\bI/\x06a\x00\x06:\x06EF"
     end
 
     it "dumps a UTF-8 Regexp" do
@@ -467,10 +465,10 @@ def _dump(level)
 
     it "dumps a Regexp in another encoding" do
       o = Regexp.new("".dup.force_encoding("utf-16le"), Regexp::FIXEDENCODING)
-      NATFIXME 'dumps a Regexp in another encoding', exception: SpecFailedException do
-        Marshal.dump(o).should == "\x04\bI/\x00\x10\x06:\rencoding\"\rUTF-16LE"
+      Marshal.dump(o).should == "\x04\bI/\x00\x10\x06:\rencoding\"\rUTF-16LE"
 
-        o = Regexp.new("a".encode("utf-16le"), Regexp::FIXEDENCODING)
+      o = Regexp.new("a".encode("utf-16le"), Regexp::FIXEDENCODING)
+      NATFIXME 'encoding issues', exception: Encoding::CompatibilityError do
         Marshal.dump(o).should == "\x04\bI/\aa\x00\x10\x06:\rencoding\"\rUTF-16LE"
       end
     end

spec/core/marshal/shared/load.rb

@@ -554,7 +554,7 @@
       s.instance_variable_set(:@foo, 10)
       obj = ['5', s, 'hi'].extend(Meths, MethsMore)
       obj.instance_variable_set(:@mix, s)
-      NATFIXME 'loads an array having ivar', exception: NameError, message: "`@@foo' is not allowed as an instance variable name" do
+      NATFIXME 'loads an array having ivar', exception: ArgumentError, message: 'dump format error' do
         new_obj = Marshal.send(@method, "\004\bI[\b\"\0065I\"\twell\006:\t@fooi\017\"\ahi\006:\t@mix@\a")
         new_obj.should == obj
         new_obj.instance_variable_get(:@mix).should equal new_obj[1]
@@ -609,8 +609,8 @@
       h = { key: s }
       h.instance_variable_set :@hash_ivar, 'hash ivar'
 
-      NATFIXME 'issue with additional @ in ivar name', exception: NameError do
-        unmarshalled = Marshal.send(@method, Marshal.dump(h))
+      unmarshalled = Marshal.send(@method, Marshal.dump(h))
+      NATFIXME 'it preserves hash ivars when hash contains a string having ivar', exception: SpecFailedException do
         unmarshalled.instance_variable_get(:@hash_ivar).should == 'hash ivar'
         unmarshalled[:key].instance_variable_get(:@string_ivar).should == 'string ivar'
       end
@@ -755,10 +755,8 @@ def io.binmode; raise "binmode"; end
     end
 
     it "loads a string with an ivar" do
-      NATFIXME 'loads a string with an ivar', exception: NameError, message: "`@@foo' is not allowed as an instance variable name" do
-        str = Marshal.send(@method, "\x04\bI\"\x00\x06:\t@fooI\"\bbar\x06:\x06EF")
-        str.instance_variable_get("@foo").should == "bar"
-      end
+      str = Marshal.send(@method, "\x04\bI\"\x00\x06:\t@fooI\"\bbar\x06:\x06EF")
+      str.instance_variable_get("@foo").should == "bar"
     end
 
     it "loads a String subclass with custom constructor" do
@@ -789,9 +787,7 @@ def io.binmode; raise "binmode"; end
       data = "\x04\bI\"\x0Fm\x00\xF6\x00h\x00r\x00e\x00\x06:\rencoding\"\rUTF-16LE"
       result = Marshal.send(@method, data)
       result.should == str
-      NATFIXME 'loads a String in another encoding', exception: SpecFailedException do
-        result.encoding.should equal(Encoding::UTF_16LE)
-      end
+      result.encoding.should equal(Encoding::UTF_16LE)
     end
 
     it "loads a String as BINARY if no encoding is specified at the end" do
@@ -830,11 +826,9 @@ def io.binmode; raise "binmode"; end
     it "loads a struct having ivar" do
       obj = Struct.new("Thick").new
       obj.instance_variable_set(:@foo, 5)
-      NATFIXME 'ivar names', exception: NameError, message: "`@@foo' is not allowed as an instance variable name" do
-        reloaded = Marshal.send(@method, "\004\bIS:\022Struct::Thick\000\006:\t@fooi\n")
-        reloaded.should == obj
-        reloaded.instance_variable_get(:@foo).should == 5
-      end
+      reloaded = Marshal.send(@method, "\004\bIS:\022Struct::Thick\000\006:\t@fooi\n")
+      reloaded.should == obj
+      reloaded.instance_variable_get(:@foo).should == 5
       Struct.send(:remove_const, :Thick)
     end
 
@@ -1081,8 +1075,8 @@ def io.binmode; raise "binmode"; end
         obj = Regexp.new("hello")
         obj.instance_variable_set(:@regexp_ivar, [42])
 
-        NATFIXME 'Correct incorrect ivar name', exception: NameError, message: "`@@regexp_ivar' is not allowed as an instance variable name" do
-          new_obj = Marshal.send(@method, "\x04\bI/\nhello\x00\a:\x06EF:\x11@regexp_ivar[\x06i/")
+        new_obj = Marshal.send(@method, "\x04\bI/\nhello\x00\a:\x06EF:\x11@regexp_ivar[\x06i/")
+        NATFIXME 'restore the regexp instance variables', exception: SpecFailedException do
           new_obj.instance_variables.should == [:@regexp_ivar]
           new_obj.instance_variable_get(:@regexp_ivar).should == [42]
         end

src/marshal.rb

@@ -104,15 +104,24 @@ def write_string_bytes(value)
     def write_encoding_bytes(value)
       ivars = value.instance_variables.map { |ivar_name| [ivar_name, value.instance_variable_get(ivar_name)] }
       case value.encoding
+      when Encoding::ASCII_8BIT
+        nil # no encoding saved
       when Encoding::US_ASCII
         ivars.prepend([:E, false])
       when Encoding::UTF_8
         ivars.prepend([:E, true])
+      else
+        ivars.prepend([:encoding, value.encoding.name])
       end
       write_integer_bytes(ivars.size) unless ivars.empty?
       ivars.each do |ivar_name, ivar_value|
         write(ivar_name)
-        write(ivar_value)
+        if ivar_name == :encoding
+          write_char('"')
+          write_string_bytes(ivar_value)
+        else
+          write(ivar_value)
+        end
       end
     end
 
@@ -570,9 +579,10 @@ def read_ivars(object)
           elsif value == true
             object.force_encoding(Encoding::UTF_8)
           end
+        elsif name == :encoding
+          object.force_encoding(value)
         else
-          ivar_name = '@' + name.to_s
-          object.instance_variable_set(ivar_name, value)
+          object.instance_variable_set(name, value)
         end
       end
     end