Skip to content

Commit 679128d

Browse files
Jay Pignatajeremy
authored andcommitted
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 <jeremy@bitsweat.net>
1 parent 911acc1 commit 679128d

File tree

3 files changed

+46
-5
lines changed

3 files changed

+46
-5
lines changed

actionpack/lib/action_dispatch/middleware/params_parser.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ def parse_formatted_parameters(env)
4747
false
4848
end
4949
rescue Exception => e # YAML, XML or Ruby code block errors
50+
logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}"
51+
5052
raise
5153
{ "body" => request.raw_post,
5254
"content_type" => request.content_type,
@@ -67,5 +69,9 @@ def content_type_from_legacy_post_data_format_header(env)
6769

6870
nil
6971
end
72+
73+
def logger
74+
defined?(Rails.logger) ? Rails.logger : Logger.new($stderr)
75+
end
7076
end
7177
end

actionpack/test/dispatch/request/json_params_parsing_test.rb

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,36 @@ def teardown
3030
)
3131
end
3232

33+
test "logs error if parsing unsuccessful" do
34+
with_test_routing do
35+
begin
36+
$stderr = StringIO.new
37+
json = "[\"person]\": {\"name\": \"David\"}}"
38+
post "/parse", json, {'CONTENT_TYPE' => 'application/json'}
39+
assert_response :error
40+
$stderr.rewind && err = $stderr.read
41+
assert err =~ /Error occurred while parsing request parameters/
42+
ensure
43+
$stderr = STDERR
44+
end
45+
end
46+
end
47+
3348
private
3449
def assert_parses(expected, actual, headers = {})
50+
with_test_routing do
51+
post "/parse", actual, headers
52+
assert_response :ok
53+
assert_equal(expected, TestController.last_request_parameters)
54+
end
55+
end
56+
57+
def with_test_routing
3558
with_routing do |set|
3659
set.draw do |map|
3760
map.connect ':action', :controller => "json_params_parsing_test/test"
3861
end
39-
40-
post "/parse", actual, headers
41-
assert_response :ok
42-
assert_equal(expected, TestController.last_request_parameters)
62+
yield
4363
end
4464
end
4565
end

actionpack/test/dispatch/request/xml_params_parsing_test.rb

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,21 @@ def teardown
3838
end
3939
end
4040

41+
test "logs error if parsing unsuccessful" do
42+
with_test_routing do
43+
begin
44+
$stderr = StringIO.new
45+
xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{ActiveSupport::Base64.encode64('ABC')}</avatar></pineapple>"
46+
post "/parse", xml, default_headers
47+
assert_response :error
48+
$stderr.rewind && err = $stderr.read
49+
assert err =~ /Error occurred while parsing request parameters/
50+
ensure
51+
$stderr = STDERR
52+
end
53+
end
54+
end
55+
4156
test "parses multiple files" do
4257
xml = <<-end_body
4358
<person>
@@ -85,4 +100,4 @@ class LegacyXmlParamsParsingTest < XmlParamsParsingTest
85100
def default_headers
86101
{'HTTP_X_POST_DATA_FORMAT' => 'xml'}
87102
end
88-
end
103+
end

0 commit comments

Comments
 (0)