diff options
author | Godfrey Chan <godfreykfc@gmail.com> | 2014-12-15 14:46:32 -0800 |
---|---|---|
committer | Godfrey Chan <godfreykfc@gmail.com> | 2014-12-15 14:46:32 -0800 |
commit | 488aefe7426bbd6f071019dff4c4cd1fbfd56742 (patch) | |
tree | c6d6a90582aa07f37502e34f301a750072f28779 /actionpack | |
parent | 485723e87a969984c054f68d1781cf99f8a68a5f (diff) | |
parent | 8f8ccb9901cab457c6e1d52bdb25acf658fd5777 (diff) | |
download | rails-488aefe7426bbd6f071019dff4c4cd1fbfd56742.tar.gz rails-488aefe7426bbd6f071019dff4c4cd1fbfd56742.tar.bz2 rails-488aefe7426bbd6f071019dff4c4cd1fbfd56742.zip |
Merge pull request #16924 from Sinjo/params-deep-munge-empty-array
Don't convert empty arrays to nils when deep munging params
Diffstat (limited to 'actionpack')
5 files changed, 15 insertions, 17 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3b02994459..115ad54190 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,14 @@ +* Stop converting empty arrays in `params` to `nil` + + This behaviour was introduced in response to CVE-2012-2660, CVE-2012-2694 + and CVE-2013-0155 + + ActiveRecord now issues a safe query when passing an empty array into + a where clause, so there is no longer a need to defend against this type + of input (any nils are still stripped from the array). + + *Chris Sinjakli* + * Fixed usage of optional scopes in URL helpers. *Alex Robbin* diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index d3f93a5352..87609d8aa7 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -53,15 +53,6 @@ module ActionController end end - def deep_munge(event) - debug do - "Value for params[:#{event.payload[:keys].join('][:')}] was set "\ - "to nil, because it was one of [], [null] or [null, null, ...]. "\ - "Go to http://guides.rubyonrails.org/security.html#unsafe-query-generation "\ - "for more information."\ - end - end - %w(write_fragment read_fragment exist_fragment? expire_fragment expire_page write_page).each do |method| class_eval <<-METHOD, __FILE__, __LINE__ + 1 diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index 9d4f1aa3c5..1c9371d89c 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -16,10 +16,6 @@ module ActionDispatch when Array v.grep(Hash) { |x| deep_munge(x, keys) } v.compact! - if v.empty? - hash[k] = nil - ActiveSupport::Notifications.instrument("deep_munge.action_controller", keys: keys) - end when Hash deep_munge(v, keys) end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index c609075e6b..b765a13fa1 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -39,7 +39,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest test "nils are stripped from collections" do assert_parses( - {"person" => nil}, + {"person" => []}, "{\"person\":[null]}", { 'CONTENT_TYPE' => 'application/json' } ) assert_parses( @@ -47,7 +47,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest "{\"person\":[\"foo\",null]}", { 'CONTENT_TYPE' => 'application/json' } ) assert_parses( - {"person" => nil}, + {"person" => []}, "{\"person\":[null, null]}", { 'CONTENT_TYPE' => 'application/json' } ) end diff --git a/actionpack/test/dispatch/request/query_string_parsing_test.rb b/actionpack/test/dispatch/request/query_string_parsing_test.rb index 4e99c26e03..50daafbb54 100644 --- a/actionpack/test/dispatch/request/query_string_parsing_test.rb +++ b/actionpack/test/dispatch/request/query_string_parsing_test.rb @@ -95,8 +95,8 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest assert_parses({"action" => nil}, "action") assert_parses({"action" => {"foo" => nil}}, "action[foo]") assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar]") - assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar][]") - assert_parses({"action" => {"foo" => nil }}, "action[foo][]") + assert_parses({"action" => {"foo" => { "bar" => [] }}}, "action[foo][bar][]") + assert_parses({"action" => {"foo" => [] }}, "action[foo][]") assert_parses({"action"=>{"foo"=>[{"bar"=>nil}]}}, "action[foo][][bar]") end |