-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add validation for numerical pagination argument values to be non-negative #8832
Conversation
class PageArgument(BaseCLIArgument): | ||
type_map = { | ||
'string': str, | ||
'integer': int, | ||
'long': int, | ||
'integer': nonnegative_int, |
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.
One possible concern here is that we are now enforcing ALL numerical page arguments be non-negative. As of today, that's what we want. In the future, if only a subset of pagination arguments should be validated as non-negative, some refactoring would be necessary. Overall, I think this is reasonable, but I'm happy to take feedback.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8832 +/- ##
==========================================
- Coverage 0.08% 0.08% -0.01%
==========================================
Files 210 210
Lines 16908 16940 +32
==========================================
Hits 14 14
- Misses 16894 16926 +32 ☔ View full report in Codecov by Sentry. |
Going with another approach for this |
Description of changes:
I've added validation on numerical pagination arguments to ensure they are non-negative. This fixes a bug where using negative numbers for pagination arguments can cause erroneous results.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.