Merge pull request #36623 from alipman88/exclude_marshal_dump_from_delegate_missing_to

Exclude marshal_dump & _dump methods from being delegated via delegate_missing_to extension, fix #36522

3 files changed, 44 insertions, 2 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index d56d4c22de..1776972dd7 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,5 +1,12 @@
-*   Allow the `on_rotation` proc used when decrypting/verifying a message to be
-    passed at the constructor level.
+*   Do not delegate missing `marshal_dump` and `_dump` methods via the
+    `delegate_missing_to` extension. This avoids unintentionally adding instance
+    variables when calling `Marshal.dump(object)`, should the delegation target of
+    `object` be a method which would otherwise add them. Fixes #36522.
+
+    *Aaron Lipman*
+
+*   Allow the on_rotation proc used when decrypting/verifying a message to be
+    be passed at the constructor level.
 
     Before:
 
diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb
index 54271a3970..14d7f0c484 100644
--- a/activesupport/lib/active_support/core_ext/module/delegation.rb
+++ b/activesupport/lib/active_support/core_ext/module/delegation.rb
@@ -276,6 +276,11 @@ class Module
   # The delegated method must be public on the target, otherwise it will
   # raise +DelegationError+. If you wish to instead return +nil+,
   # use the <tt>:allow_nil</tt> option.
+  #
+  # The <tt>marshal_dump</tt> and <tt>_dump</tt> methods are exempt from
+  # delegation due to possible interference when calling
+  # <tt>Marshal.dump(object)</tt>, should the delegation target method
+  # of <tt>object</tt> add or remove instance variables.
   def delegate_missing_to(target, allow_nil: nil)
     target = target.to_s
     target = "self.#{target}" if DELEGATION_RESERVED_METHOD_NAMES.include?(target)
@@ -285,6 +290,7 @@ class Module
         # It may look like an oversight, but we deliberately do not pass
         # +include_private+, because they do not get delegated.
 
+        return false if name == :marshal_dump || name == :_dump
         #{target}.respond_to?(name) || super
       end
 
diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb
index ec9ecd06ee..dd36a9373a 100644
--- a/activesupport/test/core_ext/module_test.rb
+++ b/activesupport/test/core_ext/module_test.rb
@@ -111,6 +111,24 @@ class DecoratedReserved
   end
 end
 
+class Maze
+  attr_accessor :cavern, :passages
+end
+
+class Cavern
+  delegate_missing_to :target
+
+  attr_reader :maze
+
+  def initialize(maze)
+    @maze = maze
+  end
+
+  def target
+    @maze.passages = :twisty
+  end
+end
+
 class Block
   def hello?
     true
@@ -411,6 +429,17 @@ class ModuleTest < ActiveSupport::TestCase
     assert_respond_to DecoratedTester.new(@david), :extra_missing
   end
 
+  def test_delegate_missing_to_does_not_interfere_with_marshallization
+    maze = Maze.new
+    maze.cavern = Cavern.new(maze)
+
+    array = [maze, nil]
+    serialized_array = Marshal.dump(array)
+    deserialized_array = Marshal.load(serialized_array)
+
+    assert_nil deserialized_array[1]
+  end
+
   def test_delegate_with_case
     event = Event.new(Tester.new)
     assert_equal 1, event.foo