From ea3c456d3b34992240f6225c93a39bb285d9cf4a Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Sat, 24 Oct 2020 15:06:30 +0100 Subject: [PATCH 1/3] Patch Rails :deep_munge issue so empty array parameters don't get converted to nil Before people were unable to remove coordinator fees from an order cycle because Rails was converting the empty :coordinator_fee_ids array paramter into nil. This issue was introduced to Rails in v4.0.0.beta1 and isn't fixed until v5.0.0.beta1 Another way to fix this could be to do something like 'params[:coordinator_fee_ids] ||= []' but it seems like this issue could problems in other parts of the app so a more general fix might be better. Fixes #6224 --- ...ction_dispatch_request_deep_munge_patch.rb | 1 + lib/action_dispatch/request.rb | 47 +++++++++++++++++++ spec/lib/action_dispatch/request_spec.rb | 18 +++++++ 3 files changed, 66 insertions(+) create mode 100644 config/initializers/action_dispatch_request_deep_munge_patch.rb create mode 100644 lib/action_dispatch/request.rb create mode 100644 spec/lib/action_dispatch/request_spec.rb diff --git a/config/initializers/action_dispatch_request_deep_munge_patch.rb b/config/initializers/action_dispatch_request_deep_munge_patch.rb new file mode 100644 index 0000000000..9967e08a0b --- /dev/null +++ b/config/initializers/action_dispatch_request_deep_munge_patch.rb @@ -0,0 +1 @@ +require "action_dispatch/request" diff --git a/lib/action_dispatch/request.rb b/lib/action_dispatch/request.rb new file mode 100644 index 0000000000..681d0890bc --- /dev/null +++ b/lib/action_dispatch/request.rb @@ -0,0 +1,47 @@ +# This patch fixes the Rails issue where ActionDispatch::Request#deep_munge was converting empty +# array paramters into nils, see https://github.com/rails/rails/issues/13420 +# +# Before this patch: +# +# | JSON | Hash | +# |----------------------------------|-------------------------| +# | { "person": [] } | { 'person' => nil } | +# +# After patch: +# +# | JSON | Hash | +# |----------------------------------|-------------------------| +# | { "person": [] } | { 'person' => [] } | +# +# The issue started in Rails v4.0.0.beta1: +# +# https://github.com/rails/rails/commit/8e577fe560d5756fcc67840ba304d79ada6804e4 +# +# This patch can be removed on or after Rails v5.0.0.beta1 when the issue was fixed: +# +# https://github.com/rails/rails/commit/8f8ccb9901cab457c6e1d52bdb25acf658fd5777 +# +# Credit: +# +# https://gist.github.com/victorblasco/f675b4cbaf9c0bc19f81 + +module ActionDispatch + class Request + def deep_munge(hash) + hash.each do |k, v| + case v + when Array + v.grep(Hash) { |x| deep_munge(x) } + v.compact! + + # This patch removes the next line + # hash[k] = nil if v.empty? + when Hash + deep_munge(v) + end + end + + hash + end + end +end diff --git a/spec/lib/action_dispatch/request_spec.rb b/spec/lib/action_dispatch/request_spec.rb new file mode 100644 index 0000000000..c0ab839f08 --- /dev/null +++ b/spec/lib/action_dispatch/request_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe ActionDispatch::Request do + it "strips nils from arrays" do + assert_parses({ "key" => ["value"] }, 'key[]=value&key[]') + assert_parses({ "key1" => { "key2" => ["value"] } }, 'key1[key2][]=value&key1[key2][]') + end + + it "doesn't convert an empty array to nil" do + assert_parses({ "key" => [] }, 'key[]') + end + + private + + def assert_parses(expected, actual) + assert_equal expected, ActionDispatch::Request.new('QUERY_STRING' => actual).query_parameters + end +end From 5a66c855bc2e56954f1250360f6ffb91a76cf539 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 30 Oct 2020 10:09:53 +0000 Subject: [PATCH 2/3] Add :frozen_string_literal magic comment and underscore prefix to unused block argument for Rubocop --- lib/action_dispatch/request.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/action_dispatch/request.rb b/lib/action_dispatch/request.rb index 681d0890bc..a0f26ecd01 100644 --- a/lib/action_dispatch/request.rb +++ b/lib/action_dispatch/request.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # This patch fixes the Rails issue where ActionDispatch::Request#deep_munge was converting empty # array paramters into nils, see https://github.com/rails/rails/issues/13420 # @@ -28,7 +30,7 @@ module ActionDispatch class Request def deep_munge(hash) - hash.each do |k, v| + hash.each do |_k, v| case v when Array v.grep(Hash) { |x| deep_munge(x) } From 5aa8c783b110cd91aaabdbdae057a17f6b59180d Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 30 Oct 2020 11:10:27 +0000 Subject: [PATCH 3/3] Use Rspec syntax instead of Test::Unit in ActionDispatch::Request spec. --- spec/lib/action_dispatch/request_spec.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/spec/lib/action_dispatch/request_spec.rb b/spec/lib/action_dispatch/request_spec.rb index c0ab839f08..59caf701cb 100644 --- a/spec/lib/action_dispatch/request_spec.rb +++ b/spec/lib/action_dispatch/request_spec.rb @@ -1,18 +1,25 @@ +# frozen_string_literal: true + require 'spec_helper' describe ActionDispatch::Request do it "strips nils from arrays" do - assert_parses({ "key" => ["value"] }, 'key[]=value&key[]') - assert_parses({ "key1" => { "key2" => ["value"] } }, 'key1[key2][]=value&key1[key2][]') + expect(parse_query_parameters('key[]=value&key[]')).to eq({ "key" => ["value"] }) + end + + it "strips nils from nested arrays" do + expect( + parse_query_parameters('key1[key2][]=value&key1[key2][]') + ).to eq({ "key1" => { "key2" => ["value"] } }) end it "doesn't convert an empty array to nil" do - assert_parses({ "key" => [] }, 'key[]') + expect(parse_query_parameters('key[]')).to eq({ "key" => [] }) end private - def assert_parses(expected, actual) - assert_equal expected, ActionDispatch::Request.new('QUERY_STRING' => actual).query_parameters + def parse_query_parameters(query_parameters) + ActionDispatch::Request.new("QUERY_STRING" => query_parameters).query_parameters end end