Add error handling for requests with invalid params (Fixes #1058) #1070
Conversation
jkowens
force-pushed
the
fix-1058
branch
9 times, most recently
from
a297e70 to
45044e8
Compare
jkowens
changed the title
jkowens
force-pushed
the
fix-1058
branch
3 times, most recently
from
cab3170 to
b51de25
Compare
jkowens
changed the title
jkowens
force-pushed
the
fix-1058
branch
3 times, most recently
from
3b13a0a to
77ada0c
Compare
|
I've iterated on this several times, but I think I've finally got it figured out. If someone could review this and provide comments/suggestions that would be awesome! |
|
Yeesh, I'm afraid this issue is too ambitious to try to make into the next stable release. I'm worried that my eyes are too weary to make this decision alone. Let's wait for more feedback. |
|
/cc @sinatra/team-sinatra |
Requests that raise an error when parameters are parsed will now respond with a 400 status instead of 500.
|
After looking into this I think it's ok to merge for the next major release. @jkowens Thank you! |
| @@ -73,6 +73,12 @@ def unlink? | |||
| request_method == "UNLINK" | |||
| end | |||
|
|
|||
| def params | |||
| super | |||
| rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e | |||
There was a problem hiding this comment.
@zzak If there is any interest in backporting this fix to stable this would have to be changed to rescuing TypeError and ArgumentError.
There was a problem hiding this comment.
No, since I consider this a behavior change and added functionality
Requests that raise errors when parsing query parameters will now respond with a 400 status instead of 500 as suggested by the Rack docs. Fixes #1058. See #1058 for steps to reproduce.
Exceptions caused by parsing bad parameters were raised outside of the
dispatch!method so the default exception handler was not invoked. That is why these exceptions bubbled up to the app server and exception messages were being displayed in the browser.