From ab491134a355257eec4730c48977b618c04549a7 Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Sun, 25 Jun 2017 02:13:05 +0100 Subject: Don't wrap parameters if query parameter exists We want to avoid overwriting a query parameter with the wrapped parameters hash. Previously this was implemented by merging the wrapped parameters at the root level if the key already existed, which was effectively a no-op. The query parameter was still overwritten in the filtered parameters hash, however. We can fix that discrepancy with a simpler implementation and less unnecessary work by skipping parameter wrapping entirely if the key was sent as a query parameter. --- actionpack/lib/action_controller/metal/params_wrapper.rb | 9 ++------- actionpack/test/controller/params_wrapper_test.rb | 8 ++++++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 44151c9f71..818af549eb 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -232,12 +232,7 @@ module ActionController # by the metal call stack. def process_action(*args) if _wrapper_enabled? - if request.parameters[_wrapper_key].present? - wrapped_hash = _extract_parameters(request.parameters) - else - wrapped_hash = _wrap_parameters request.request_parameters - end - + wrapped_hash = _wrap_parameters request.request_parameters wrapped_keys = request.request_parameters.keys wrapped_filtered_hash = _wrap_parameters request.filtered_parameters.slice(*wrapped_keys) @@ -282,7 +277,7 @@ module ActionController return false unless request.has_content_type? ref = request.content_mime_type.ref - _wrapper_formats.include?(ref) && _wrapper_key && !request.request_parameters.key?(_wrapper_key) + _wrapper_formats.include?(ref) && _wrapper_key && !request.parameters.key?(_wrapper_key) end end end diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index 4cbb28ef60..c0f01e6df8 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -226,6 +226,14 @@ class ParamsWrapperTest < ActionController::TestCase end end + def test_preserves_query_string_params_in_filtered_params + with_default_wrapper_options do + @request.env["CONTENT_TYPE"] = "application/json" + get :parse, params: { "user" => { "username" => "nixon" } } + assert_equal({ "controller" => "params_wrapper_test/users", "action" => "parse", "user" => { "username" => "nixon" } }, @request.filtered_parameters) + end + end + def test_empty_parameter_set with_default_wrapper_options do @request.env["CONTENT_TYPE"] = "application/json" -- cgit v1.2.3