From 679128da58636f3ec96e24fcb7daac24666b2dad Mon Sep 17 00:00:00 2001 From: Jay Pignata Date: Tue, 11 Aug 2009 00:56:46 -0400 Subject: [PATCH] Adding a call to logger from params_parser to give detailed debug information when invalid xml or json is posted [#2481 state:committed] Signed-off-by: Jeremy Kemper --- .../middleware/params_parser.rb | 6 ++++ .../request/json_params_parsing_test.rb | 28 ++++++++++++++++--- .../request/xml_params_parsing_test.rb | 17 ++++++++++- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index e83cf9236b9fd..ff2b2fe74b49a 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -47,6 +47,8 @@ def parse_formatted_parameters(env) false end rescue Exception => e # YAML, XML or Ruby code block errors + logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" + raise { "body" => request.raw_post, "content_type" => request.content_type, @@ -67,5 +69,9 @@ def content_type_from_legacy_post_data_format_header(env) nil end + + def logger + defined?(Rails.logger) ? Rails.logger : Logger.new($stderr) + end end end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index a3dde72c4e607..db6cf7b330fcb 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -30,16 +30,36 @@ def teardown ) end + test "logs error if parsing unsuccessful" do + with_test_routing do + begin + $stderr = StringIO.new + json = "[\"person]\": {\"name\": \"David\"}}" + post "/parse", json, {'CONTENT_TYPE' => 'application/json'} + assert_response :error + $stderr.rewind && err = $stderr.read + assert err =~ /Error occurred while parsing request parameters/ + ensure + $stderr = STDERR + end + end + end + private def assert_parses(expected, actual, headers = {}) + with_test_routing do + post "/parse", actual, headers + assert_response :ok + assert_equal(expected, TestController.last_request_parameters) + end + end + + def with_test_routing with_routing do |set| set.draw do |map| map.connect ':action', :controller => "json_params_parsing_test/test" end - - post "/parse", actual, headers - assert_response :ok - assert_equal(expected, TestController.last_request_parameters) + yield end end end diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb index ee764e726efd1..521002b519d0a 100644 --- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/xml_params_parsing_test.rb @@ -38,6 +38,21 @@ def teardown end end + test "logs error if parsing unsuccessful" do + with_test_routing do + begin + $stderr = StringIO.new + xml = "David#{ActiveSupport::Base64.encode64('ABC')}" + post "/parse", xml, default_headers + assert_response :error + $stderr.rewind && err = $stderr.read + assert err =~ /Error occurred while parsing request parameters/ + ensure + $stderr = STDERR + end + end + end + test "parses multiple files" do xml = <<-end_body @@ -85,4 +100,4 @@ class LegacyXmlParamsParsingTest < XmlParamsParsingTest def default_headers {'HTTP_X_POST_DATA_FORMAT' => 'xml'} end -end +end \ No newline at end of file