Restructure reading object links in Marshal.load

natalie-lang · Dec 16, 2024 · b741d9e · b741d9e
1 parent 96ff3f6
commit b741d9e
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 67 deletions.
diff --git a/spec/core/marshal/shared/load.rb b/spec/core/marshal/shared/load.rb
@@ -393,11 +393,9 @@
         str = "string"
 
         # this string represents: [<#UserDefinedImmediate A>, <#String "string">, <#String "string">]
-        NATFIXME 'Load object links', exception: IndexError, message: 'index 1 outside of array bounds: -1...1' do
-          marshaled_obj = Marshal.send(@method, "\004\b[\bu:\031UserDefinedImmediate\000\"\vstring@\a")
+        marshaled_obj = Marshal.send(@method, "\004\b[\bu:\031UserDefinedImmediate\000\"\vstring@\a")
 
-          marshaled_obj.should == [nil, str, str]
-        end
+        marshaled_obj.should == [nil, str, str]
       end
 
       it "loads any structure with multiple references to the same object, followed by multiple instances of another object" do
@@ -915,13 +913,11 @@ def io.binmode; raise "binmode"; end
       obj = Exception.new("foo")
       obj.instance_variable_set :@arr, arr
 
-      NATFIXME 'Load object links', exception: IndexError, message: 'index 2 outside of array bounds: -2...2' do
-        loaded = Marshal.send(@method, "\x04\bo:\x0EException\b:\tmesg\"\bfoo:\abt0:\t@arr[\t:\aso;\t\"\ahi@\b")
-        new_arr = loaded.instance_variable_get :@arr
+      loaded = Marshal.send(@method, "\x04\bo:\x0EException\b:\tmesg\"\bfoo:\abt0:\t@arr[\t:\aso;\t\"\ahi@\b")
+      new_arr = loaded.instance_variable_get :@arr
 
-        loaded.message.should == obj.message
-        new_arr.should == arr
-      end
+      loaded.message.should == obj.message
+      new_arr.should == arr
     end
   end
 
@@ -948,12 +944,10 @@ def io.binmode; raise "binmode"; end
       obj = Object.new
       obj.instance_variable_set :@str, arr
 
-      NATFIXME 'Load object links', exception: IndexError, message: 'index 1 outside of array bounds: -1...1' do
-        new_obj = Marshal.send(@method, "\004\bo:\vObject\006:\t@str[\t:\aso;\a\"\ahi@\a")
-        new_str = new_obj.instance_variable_get :@str
+      new_obj = Marshal.send(@method, "\004\bo:\vObject\006:\t@str[\t:\aso;\a\"\ahi@\a")
+      new_str = new_obj.instance_variable_get :@str
 
-        new_str.should == arr
-      end
+      new_str.should == arr
     end
 
     it "loads an Object with a non-US-ASCII instance variable" do

diff --git a/src/marshal.rb b/src/marshal.rb
@@ -438,7 +438,7 @@ def read_version
 
     def read_object_link
       index = read_integer
-      @object_lookup.fetch(index - 1)
+      @object_lookup.fetch(index)
     end
 
     def read_signed_byte
@@ -643,58 +643,61 @@ def read_ivars(object)
 
     def read_value
       char = read_byte.chr
-      value = case char
-              when '0'
-                nil
-              when 'T'
-                true
-              when 'F'
-                false
-              when '@'
-                read_object_link
-              when 'i'
-                read_integer
-              when 'l'
-                read_big_integer
-              when '"'
-                read_string
-              when ':'
-                read_symbol
-              when ';'
-                read_symbol_link
-              when 'f'
-                read_float
-              when '['
-                read_array
-              when '{'
-                read_hash
-              when '}'
-                read_hash_with_default
-              when 'c'
-                read_class
-              when 'm'
-                read_module
-              when '/'
-                read_regexp
-              when 'U'
-                read_user_marshaled_object_with_allocate
-              when 'u'
-                read_user_marshaled_object_without_allocate
-              when 'S'
-                read_struct
-              when 'o'
-                read_object
-              when 'I'
-                result = read_value
-                read_ivars(result) unless result.is_a?(Regexp)
-                result
-              else
-                raise ArgumentError, 'dump format error'
-              end
-      if !value.nil? && !value.is_a?(TrueClass) && !value.is_a?(FalseClass) && !value.is_a?(Integer) && !value.is_a?(Float) && !value.is_a?(Symbol)
-        @object_lookup << value
+      case char
+      when '0'
+        nil
+      when 'T'
+        true
+      when 'F'
+        false
+      when '@'
+        read_object_link
+      when 'i'
+        read_integer
+      when 'l'
+        read_big_integer
+      when ':'
+        read_symbol
+      when ';'
+        read_symbol_link
+      when 'f'
+        read_float
+      when 'I'
+        result = read_value
+        read_ivars(result) unless result.is_a?(Regexp)
+        result
+      else
+        index = @object_lookup.size
+        @object_lookup << nil # placeholder
+        value = case char
+                when '"'
+                  read_string
+                when '['
+                  read_array
+                when '{'
+                  read_hash
+                when '}'
+                  read_hash_with_default
+                when 'c'
+                  read_class
+                when 'm'
+                  read_module
+                when '/'
+                  read_regexp
+                when 'U'
+                  read_user_marshaled_object_with_allocate
+                when 'u'
+                  read_user_marshaled_object_without_allocate
+                when 'S'
+                  read_struct
+                when 'o'
+                  read_object
+                else
+                  raise ArgumentError, 'dump format error'
+                end
+        @object_lookup[index] = value
+        value
       end
-      value
     end
 
     def find_constant(name)