Raise specific exception if the parameters are invalid #713
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This will close rails/rails#11795, rails/rails#3789 and rails/rails#16207 |
| @@ -106,6 +111,8 @@ def parse_nested_query(qs, d = nil) | |||
| end | |||
|
|
|||
| return params.to_params_hash | |||
| rescue ArgumentError => e | |||
| raise InvalidParameterError, e.message | |||
There was a problem hiding this comment.
Can you add the backtrace from e too?
There are some cases where we try to parse the parameters but it fails with ArgumentError. 1. When the parameters come from the query string and contains invalid UTF-8 characters. In this case we raise ArgumentError when trying to match the key portion of parameters with a regexp. 2. When the parameters come from the request body and the query string contains invalid % encoded string. In this case we raise ArgumentError when calling URI.decode_www_form_component. Now both cases raise a InvalidParameterError what inherit from TypeError and we can catch this exception to show a bad request for users instead of an internal server error.
tenderlove
added a commit
that referenced
this pull request
Jul 18, 2014
Raise specific exception if the parameters are invalid
This was referenced Jul 18, 2014
raggi
added a commit
that referenced
this pull request
Jul 18, 2014
|
Thanks for this fix! When can we expect a new version of Rack and Rails that incorporate this fix? |
raggi
added a commit
that referenced
this pull request
Aug 3, 2014
* master: (62 commits) build_nested_query includes integer values Rack::ETag correctly marks etags as Weak Fix yet another body close bug in Rack::Deflater Implement full Logger interface on NullLogger Revert "support empty string multipart filename" support empty string multipart filename multipart/form-data with files with no input name Fix parent type API regression introduced in #713 correct weird case regression from #714 UrlMap: Enable case-insensitive domain matching Raise specific exception if the parameters are invalid Fix media_type_params when Content-Type parameters contains quoted-strings Rack::Multipart::UploadedFile has file extensions multipart content-type match now case insensitive Undo test that falsely exemplifies production env default_middleware_by_environment should always returns empty array for unknown keys Remove rbx from Travis' allow_failures Fix rbx settings for Travis Use latest 2.1 on Travis Enable cleanup of Tempfiles from multipart form data by default ... Conflicts: lib/rack/request.rb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are some cases where we try to parse the parameters but it fails with ArgumentError.
Now both cases raise a InvalidParameterError what inherit from TypeError and we can catch this exception to show a bad request for users instead of an internal server error.
cc @tenderlove