Skip to content

Commit

Permalink
Use static message when raising HTTP request parameter parse errors
Browse files Browse the repository at this point in the history
When parsing HTTP request parameters, Rails delegates to a set of parsing
strategies based on the MIME type. If any of these strategies raises an
error Rails rescues it and raises an instance of
`ActionDispatch::Http::Parameters::ParseError` with the same message as
the underlying error.

However, in the presence of malformed JSON, the default parameter parser
for the `application/json` MIME type raises a `JSON:ParserError` with a
message containing the entire malformed JSON string (the request body in
this context). By raising a new error with this same message Rails
inadvertently ends up logging the full HTTP request body at the `fatal`
level. This request body could contain sensitive information or could be
intentionally crafted to be extremely large.

This commit sets the `ActionDispatch::Http::Parameters::ParseError` message
to a static message which mirrors that of the corresponding `debug` log.
  • Loading branch information
aaronlahey committed Aug 28, 2021
1 parent 6bd3925 commit 82fc62c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
8 changes: 8 additions & 0 deletions actionpack/CHANGELOG.md
@@ -1,3 +1,11 @@
* Use a static error message when raising `ActionDispatch::Http::Parameters::ParseError`
to avoid inadvertently logging the HTTP request body at the `fatal` level when it contains
malformed JSON.

Fixes #41145

*Aaron Lahey*

* Add `Middleware#delete!` to delete middleware or raise if not found.

`Middleware#delete!` works just like `Middleware#delete` but will
Expand Down
6 changes: 3 additions & 3 deletions actionpack/lib/action_dispatch/http/parameters.rb
Expand Up @@ -17,8 +17,8 @@ module Parameters
# Raised when raw data from the request cannot be parsed by the parser
# defined for request's content MIME type.
class ParseError < StandardError
def initialize
super($!.message)
def initialize(message = $!.message)
super(message)
end
end

Expand Down Expand Up @@ -93,7 +93,7 @@ def parse_formatted_parameters(parsers)
strategy.call(raw_post)
rescue # JSON or Ruby code block errors.
log_parse_error_once
raise ParseError
raise ParseError, "Error occurred while parsing request parameters"
end
end

Expand Down
Expand Up @@ -80,7 +80,7 @@ def teardown
post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false }
end
assert_equal JSON::ParserError, exception.cause.class
assert_equal exception.cause.message, exception.message
assert_equal "Error occurred while parsing request parameters", exception.message
ensure
$stderr = STDERR
end
Expand Down

0 comments on commit 82fc62c

Please sign in to comment.