rails/rails — pull request #7444 by szimek (2012-08-24 19:26:11+00:00)
Title: Raise generic ParseError exception when ParamsParser fails parsing request params

@tenderlove this PR is based on our discussion in #7424

When `ActionDispatch::ParamsParser#parse_formatted_parameters` raises an exception, it's re-raised as `ActionDispatch::ParamsParser::ParseError` instead of the original exception - we're assuming that the only thing that can raise an exception there is a parser. This way it's much easier to catch parsing error that originated in `ParamsParser` (skipping parsing errors raised later down the chain e.g. in actions) and return a custom response e.g. 400.

BTW. There's a funky thing going on with request body in `ParamsParser#parse_formatted_parameters` ([link](https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/params_parser.rb#L37-58)). Proc and Yaml strategies use `request.raw_post` (which reads request body and then rewinds it before being parsed), but JSON and XML ones read and rewind request body on their own e.g.:

```ruby
when :json
data = ActiveSupport::JSON.decode(request.body)
request.body.rewind if request.body.respond_to?(:rewind)
...
```

The issue is that if JSON.decode raises an exception, request body is not rewinded and thus calling `request.raw_post` later inside the error handler returns nil. Is there any reason why not all strategies use `request.raw_post`? This should most likely fix this issue.