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

If you manually set per_page = 0 in the querystring, you get a divide by zero exception #907

Closed
peetucket opened this issue May 14, 2014 · 3 comments · Fixed by #1002
Closed
Milestone

Comments

@peetucket
Copy link
Contributor

Just noticed that if someone messes with the querystring and adjusts the per_page parameter to set it to zero, they can trigger a 500 exception, presumably when the per_page is used as a divisor when computing paging parameters and links. This occurs in Blacklight 4 and occurred in production on the Revs Digital Library site (not sure if we have a bad link or the user messed with the parameter).

@MrDys
Copy link
Contributor

MrDys commented May 14, 2014

This sounds like expected behavior to me.

Sent from my iPhone

On May 14, 2014, at 5:46 PM, Peter Mangiafico notifications@github.com wrote:

Just noticed that if someone messes with the querystring and adjusts the per_page parameter to set it to zero, they can trigger a 500 exception, presumably when the per_page is used a divisor when computing paging parameters and links. This occurs in Blacklight 4.


Reply to this email directly or view it on GitHub.

@cbeer
Copy link
Member

cbeer commented May 14, 2014

If we did some input validation, I'm not sure what what we'd do differently. Throw a 400 Bad Request instead, I suppose.

@peetucket
Copy link
Contributor Author

As long as there isn't some security issue (like a DoS attack) that is possible by a random user knowing how to easily trigger 500s, its not really an issue. If I were to recommend changing something, it would be to simply set it back to the default value (e.g. 10) if the user passes a 0. This results in no errors being generated (with the downside that if your code mistakenly set it to 0, you would not necessarily be able to figure that out).

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

Successfully merging a pull request may close this issue.

3 participants