From c32abe2ebf5a02f953bf9f7d961bbdeed978d336 Mon Sep 17 00:00:00 2001 From: Patrick Toomey Date: Mon, 7 Oct 2013 21:25:50 -0500 Subject: [PATCH] Fix fetch and add unit test. --- lib/action_controller/parameters.rb | 27 ++++++++++++++------------- test/parameters_taint_test.rb | 11 +++++++++++ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/lib/action_controller/parameters.rb b/lib/action_controller/parameters.rb index ba4a83a..3717f06 100644 --- a/lib/action_controller/parameters.rb +++ b/lib/action_controller/parameters.rb @@ -29,7 +29,7 @@ def initialize(params) class Parameters < ActiveSupport::HashWithIndifferentAccess attr_accessor :permitted alias :permitted? :permitted - + cattr_accessor :action_on_unpermitted_parameters, :instance_accessor => false # Never raise an UnpermittedParameters exception because of these params @@ -79,7 +79,8 @@ def [](key) end def fetch(key, *args) - convert_hashes_to_parameters(key, super) + return self[key] if has_key?(key) + super rescue KeyError, IndexError raise ActionController::ParameterMissing.new(key) end @@ -215,23 +216,23 @@ def fields_for_style?(object) object.is_a?(Hash) && object.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) } end - def unpermitted_parameters!(params) + def unpermitted_parameters!(params) return unless self.class.action_on_unpermitted_parameters - + unpermitted_keys = unpermitted_keys(params) - if unpermitted_keys.any? - case self.class.action_on_unpermitted_parameters + if unpermitted_keys.any? + case self.class.action_on_unpermitted_parameters when :log name = "unpermitted_parameters.action_controller" ActiveSupport::Notifications.instrument(name, :keys => unpermitted_keys) - when :raise - raise ActionController::UnpermittedParameters.new(unpermitted_keys) - end - end - end - - def unpermitted_keys(params) + when :raise + raise ActionController::UnpermittedParameters.new(unpermitted_keys) + end + end + end + + def unpermitted_keys(params) self.keys - params.keys - NEVER_UNPERMITTED_PARAMS end end diff --git a/test/parameters_taint_test.rb b/test/parameters_taint_test.rb index 7dc734a..2f7d7ff 100644 --- a/test/parameters_taint_test.rb +++ b/test/parameters_taint_test.rb @@ -22,6 +22,17 @@ class ParametersTaintTest < ActiveSupport::TestCase end end + test "fetch doesnt modify original hash if there is a default" do + assert_nothing_raised do + assert_equal "monkey", @params.fetch(:foo, "monkey") + assert_nil @params[:foo] + assert_equal "monkey", @params.fetch(:foo) { "monkey" } + assert_nil @params[:foo] + assert_equal({ :bar => "monkey" }, @params.fetch(:foo, :bar => "monkey")) + assert_nil @params[:foo] + end + end + test "not permitted is sticky on accessors" do assert !@params.slice(:person).permitted? assert !@params[:person][:name].permitted?