-
Notifications
You must be signed in to change notification settings - Fork 5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parameter error handling #646
Conversation
This change look good for me. |
Thanks for the review. I checked the various getParameter() methods and found a couple I thought needed a try/catch block. The remainder were all used in webapps or in APIs called by applications and I think throwing the exception is OK in those cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments...
1ddbaa8
to
1ce49d5
Compare
1ce49d5
to
264fb18
Compare
This includes removing the FailedRequestFilter
264fb18
to
335348c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No objections anymore
This PR updates the parameter error handling to align with the changes in Jakarta Servlet 6.0
The short version is all getParameterXXX() calls now throw exceptions on invalid parameters.
Note that getParts() still needs to work even if the query string parameters are invalid.
I'm providing this as a PR to give folks a chance to review it before it is merged given that it is a significant change.