From 4dd0a1d4ad79df6bb3429e51c81c67ba7506c4ec Mon Sep 17 00:00:00 2001 From: Brendon Murphy Date: Tue, 19 Feb 2013 00:21:42 -0800 Subject: [PATCH] Stop fetch from mutating when default is a hash See gh-56 for an issue on this --- lib/action_controller/parameters.rb | 9 ++++++++- test/parameters_taint_test.rb | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/action_controller/parameters.rb b/lib/action_controller/parameters.rb index bf80e42..699d13b 100644 --- a/lib/action_controller/parameters.rb +++ b/lib/action_controller/parameters.rb @@ -79,7 +79,14 @@ def [](key) end def fetch(key, *args) - convert_hashes_to_parameters(key, super) + value = super + # Don't rely on +convert_hashes_to_parameters+ + # so as to not mutate via a +fetch+ + if value.is_a?(Hash) + value = self.class.new(value) + value.permit! if permitted? + end + value rescue KeyError, IndexError raise ActionController::ParameterMissing.new(key) end diff --git a/test/parameters_taint_test.rb b/test/parameters_taint_test.rb index 7dc734a..b1e0e6e 100644 --- a/test/parameters_taint_test.rb +++ b/test/parameters_taint_test.rb @@ -22,6 +22,13 @@ class ParametersTaintTest < ActiveSupport::TestCase end end + test "fetch where the default is a hash will not mutate the instance" do + @params.fetch :foo, {} + assert !@params.has_key?(:foo) + @params.fetch :foo, {:fizz => :buzz} + assert !@params.has_key?(:foo) + end + test "not permitted is sticky on accessors" do assert !@params.slice(:person).permitted? assert !@params[:person][:name].permitted?