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

Add validation for numerical pagination argument values to be non-negative #8832

Closed
wants to merge 8 commits into from
14 changes: 12 additions & 2 deletions awscli/customizations/paginate.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,21 @@ def _get_cli_name(param_objects, token_name):
return param.cli_name.lstrip('-')


# This function checks that the integral value passed to it is non-negative, and raises a
# ParamValidationError if it not. As such, it can be used as a type for pagination-related
# argparse arguments that must be non-negative integral values.
def nonnegative_int(value):
ival = int(value)
if ival < 0:
raise ValueError("invalid literal for nonnegative int() with base 10: '%s'" % value)
return ival


class PageArgument(BaseCLIArgument):
type_map = {
'string': str,
'integer': int,
'long': int,
'integer': nonnegative_int,
Copy link
Contributor Author

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.

'long': nonnegative_int,
}

def __init__(self, name, documentation, parse_type, serialized_name):
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/customizations/test_paginate.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,3 +321,16 @@ def test_can_handle_missing_page_size(self):
del self.parsed_args.page_size
self.assertIsNone(paginate.ensure_paging_params_not_set(
self.parsed_args, {}))


class TestNonnegativeIntType(TestPaginateBase):

def test_positive_integer_resolves_corectly(self):
self.assertEqual(paginate.nonnegative_int(1), 1)

def test_zero_resolves_corectly(self):
self.assertEqual(paginate.nonnegative_int(0), 0)

def test_negative_integer_raises_error(self):
with self.assertRaises(ValueError):
paginate.nonnegative_int(-1)
Loading