From 4b46c5ce83eaff77781d579bdfb6548de7f5a80a Mon Sep 17 00:00:00 2001
From: Kasper Timm Hansen <kaspth@gmail.com>
Date: Thu, 17 Dec 2015 21:21:12 +0100
Subject: [PATCH] Only dup Ruby's Hash and Array.

When calling `to_h` on an `ActionController::Parameters` instance it would
`deep_dup` its internal parameters.

This inadvertently called `dup` on a passed Active Record model which would
create new models. Fix by only dupping Ruby's Arrays and Hashes.
---
 .../metal/strong_parameters.rb                | 20 +++++++++++--
 .../parameters/parameters_permit_test.rb      | 28 +++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index 8bc3c271e2..957aa746c0 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -1,4 +1,5 @@
 require 'active_support/core_ext/hash/indifferent_access'
+require 'active_support/core_ext/hash/transform_values'
 require 'active_support/core_ext/array/wrap'
 require 'active_support/core_ext/string/filters'
 require 'active_support/rescuable'
@@ -175,7 +176,7 @@ module ActionController
     #   safe_params.to_h # => {"name"=>"Senjougahara Hitagi"}
     def to_h
       if permitted?
-        @parameters.deep_dup
+        convert_parameters_to_hashes(@parameters)
       else
         slice(*self.class.always_permitted_parameters).permit!.to_h
       end
@@ -185,7 +186,7 @@ module ActionController
     # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this
     # parameter.
     def to_unsafe_h
-      @parameters.deep_dup
+      convert_parameters_to_hashes(@parameters)
     end
     alias_method :to_unsafe_hash, :to_unsafe_h
 
@@ -594,6 +595,21 @@ module ActionController
         end
       end
 
+      def convert_parameters_to_hashes(value)
+        case value
+        when Array
+          value.map { |v| convert_parameters_to_hashes(v) }
+        when Hash
+          value.transform_values do |v|
+            convert_parameters_to_hashes(v)
+          end.with_indifferent_access
+        when Parameters
+          value.to_h
+        else
+          value
+        end
+      end
+
       def convert_hashes_to_parameters(key, value)
         converted = convert_value_to_parameters(value)
         @parameters[key] = converted unless converted.equal?(value)
diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
index 87816515e7..f23aa599c1 100644
--- a/actionpack/test/controller/parameters/parameters_permit_test.rb
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -297,4 +297,32 @@ class ParametersPermitTest < ActiveSupport::TestCase
     assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess
     assert_not @params.to_h.is_a? ActionController::Parameters
   end
+
+  test "to_h only deep dups Ruby collections" do
+    company = Class.new do
+      attr_reader :dupped
+      def dup; @dupped = true; end
+    end.new
+
+    params = ActionController::Parameters.new(prem: { likes: %i( dancing ) })
+    assert_equal({ 'prem' => { 'likes' => %i( dancing ) } }, params.permit!.to_h)
+
+    params = ActionController::Parameters.new(companies: [ company, :acme ])
+    assert_equal({ 'companies' => [ company, :acme ] }, params.permit!.to_h)
+    assert_not company.dupped
+  end
+
+  test "to_unsafe_h only deep dups Ruby collections" do
+    company = Class.new do
+      attr_reader :dupped
+      def dup; @dupped = true; end
+    end.new
+
+    params = ActionController::Parameters.new(prem: { likes: %i( dancing ) })
+    assert_equal({ 'prem' => { 'likes' => %i( dancing ) } }, params.to_unsafe_h)
+
+    params = ActionController::Parameters.new(companies: [ company, :acme ])
+    assert_equal({ 'companies' => [ company, :acme ] }, params.to_unsafe_h)
+    assert_not company.dupped
+  end
 end