From 97ed810cfc15725a0856227fa9f9eb26930f16c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Emin=20=C4=B0NA=C3=87?= Date: Sat, 20 Feb 2016 11:33:05 +0200 Subject: Use symbol of mime type instead of object to get correct parser After registering new `:json` mime type `parsers.fetch` can't find the mime type because new mime type is not equal to old one. Using symbol of the mime type as key on parsers hash solves the problem. Closes #23766 --- actionpack/lib/action_controller/test_case.rb | 4 ++-- actionpack/lib/action_dispatch/http/parameters.rb | 4 ++-- actionpack/test/controller/webservice_test.rb | 4 ++-- .../test/dispatch/request/json_params_parsing_test.rb | 16 ++++++++++++++++ 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 0c4b661214..700317614f 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -52,7 +52,7 @@ module ActionController self.session = session self.session_options = TestSession::DEFAULT_OPTIONS @custom_param_parsers = { - Mime[:xml] => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } + xml: lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } } end @@ -105,7 +105,7 @@ module ActionController when :url_encoded_form data = non_path_parameters.to_query else - @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters } + @custom_param_parsers[content_mime_type.symbol] = ->(_) { non_path_parameters } data = non_path_parameters.to_query end end diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index cca7376ffa..bcb818d38c 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -4,7 +4,7 @@ module ActionDispatch PARAMETERS_KEY = 'action_dispatch.request.path_parameters' DEFAULT_PARSERS = { - Mime[:json] => lambda { |raw_post| + Mime[:json].symbol => -> (raw_post) { data = ActiveSupport::JSON.decode(raw_post) data.is_a?(Hash) ? data : {:_json => data} } @@ -51,7 +51,7 @@ module ActionDispatch def parse_formatted_parameters(parsers) return yield if content_length.zero? - strategy = parsers.fetch(content_mime_type) { return yield } + strategy = parsers.fetch(content_mime_type.symbol) { return yield } begin strategy.call(raw_post) diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 6d377c4691..f02898e10c 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -65,7 +65,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_register_and_use_json_simple with_test_route_set do - with_params_parsers Mime[:json] => Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do + with_params_parsers json: Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do post "/", params: '{"request":{"summary":"content...","title":"JSON"}}', headers: { 'CONTENT_TYPE' => 'application/json' } @@ -99,7 +99,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_parsing_json_doesnot_rescue_exception req = Class.new(ActionDispatch::Request) do def params_parsers - { Mime[:json] => Proc.new { |data| raise Interrupt } } + { json: Proc.new { |data| raise Interrupt } } end def content_length; get_header('rack.input').length; end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index 64801bff39..0c3c6d5f93 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -150,6 +150,22 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end + test "parses json params after custom json mime type registered" do + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/json' } + ) + end + + test "parses json params after custom json mime type registered with synonym" do + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + ) + end + private def assert_parses(expected, actual, headers = {}) with_test_routing(UsersController) do -- cgit v1.2.3 From a087cf4312c1ec01f3bb021a6791ac3a6ef1cec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 22 Feb 2016 17:20:37 -0300 Subject: Transform the mime object to symbol when registering the parsers This will keep our current API working without having the users to change their codebases. --- actionpack/lib/action_dispatch/http/parameters.rb | 17 +++++++++++++---- .../lib/action_dispatch/middleware/params_parser.rb | 1 + actionpack/test/controller/webservice_test.rb | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index bcb818d38c..ff5031d7d5 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -1,6 +1,8 @@ module ActionDispatch module Http module Parameters + extend ActiveSupport::Concern + PARAMETERS_KEY = 'action_dispatch.request.path_parameters' DEFAULT_PARSERS = { @@ -10,13 +12,20 @@ module ActionDispatch } } - def self.included(klass) - class << klass - attr_accessor :parameter_parsers + included do + class << self + attr_reader :parameter_parsers end - klass.parameter_parsers = DEFAULT_PARSERS + self.parameter_parsers = DEFAULT_PARSERS end + + module ClassMethods + def parameter_parsers=(parsers) # :nodoc: + @parameter_parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } + end + end + # Returns both GET and POST \parameters in a single hash. def parameters params = get_header("action_dispatch.request.parameters") diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index c2a4f46e67..5841c978af 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,7 @@ module ActionDispatch # The +parsers+ argument can take Hash of parsers where key is identifying # content mime type, and value is a lambda that is going to process data. def self.new(app, parsers = {}) + parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } ActionDispatch::Request.parameter_parsers = ActionDispatch::Request::DEFAULT_PARSERS.merge(parsers) app end diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index f02898e10c..daf17558aa 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -65,7 +65,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_register_and_use_json_simple with_test_route_set do - with_params_parsers json: Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do + with_params_parsers Mime[:json] => Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do post "/", params: '{"request":{"summary":"content...","title":"JSON"}}', headers: { 'CONTENT_TYPE' => 'application/json' } -- cgit v1.2.3 From 4f30df4b524b5da22491090e22f5c2de789dd016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 22 Feb 2016 17:37:53 -0300 Subject: Make sure we unregister the mime type before registering Also make sure we don't change the global state of our test suite. --- .../dispatch/request/json_params_parsing_test.rb | 32 +++++++++++++++------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index 0c3c6d5f93..3655c7f570 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -151,19 +151,31 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest end test "parses json params after custom json mime type registered" do - Mime::Type.register "application/json", :json, %w(application/vnd.api+json) - assert_parses( - {"user" => {"username" => "meinac"}, "username" => "meinac"}, - "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/json' } - ) + begin + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end end test "parses json params after custom json mime type registered with synonym" do - Mime::Type.register "application/json", :json, %w(application/vnd.api+json) - assert_parses( - {"user" => {"username" => "meinac"}, "username" => "meinac"}, - "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } - ) + begin + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end end private -- cgit v1.2.3