Skip to content
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

Guard against invalid values when resolving pagination and sorting parameters from web requests [DATACMNS-408] #877

Closed
spring-projects-issues opened this issue Dec 4, 2013 · 2 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Dec 4, 2013

Kazuki Shimizu opened DATACMNS-408 and commented

[example of invalid value]

  • page=-1 (negative numeric)
  • size=a (not numeric)
  • sort= (not specified)
  • sort=,DESC (not specified property of sort target)
  • etc ...

Current implementation of the PageableHandlerMethodArgumentResolver is occurred IllegalArgumentException, and http response status is returned 500(Internal Server Error) on the default settings of SpringMVC.
I think a best that http response status is returned 400(Bad Request).


Affects: 1.6.1

Reference URL: #48

Issue Links:

  • DATACMNS-379 SortHanderMethodArgumentResolver fails to resolve comma-only sort parameter

  • DATACMNS-753 SortHandlerMethodArgumentResolver fails to fall back to default if empty parameter is given

  • DATAREST-195 Limiting page size

Backported to: 1.6.3 (Babbage SR2)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 5, 2013

Oliver Drotbohm commented

I fixed this by gracefully falling back to the defaults (in case of invalid page number and size) or ignoring invalid values (for sort properties). I think this makes more sense rather than telling the client that it was wrong and it has to repeat the request, which it would then do using the defaults anyway

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 5, 2013

Kazuki Shimizu commented

Great!!
I agree a that apply the default value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants