From f172d25fb7fc327240126c095e0ff5e0b1d89601 Mon Sep 17 00:00:00 2001 From: Kyle Hornberg Date: Fri, 5 Oct 2018 09:59:44 -0500 Subject: [PATCH 1/2] Bug Fix: Correctly validate object/dictionary parameters --- src/octokit/__init__.py | 14 ++++++++++---- src/octokit/utils.py | 7 +++++++ tests/test_methods.py | 24 ++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/octokit/__init__.py b/src/octokit/__init__.py index 5c826be..e085811 100644 --- a/src/octokit/__init__.py +++ b/src/octokit/__init__.py @@ -26,14 +26,20 @@ class Base(object): def _validate(self, kwargs, params): cached_kwargs = dict(ChainMap(kwargs, self._attribute_cache['url'])) required_params = [k for k, v in params.items() if v.get('required')] - for p in required_params: - if p not in cached_kwargs: # has all required - message = '{} is a required parameter'.format(p) - raise errors.OctokitParameterError(message) + self._validate_required_params(required_params, cached_kwargs) for kwarg, value in kwargs.items(): param_value = params.get(kwarg) self._validate_params(param_value, kwarg, value, required_params) + def _validate_required_params(self, required_params, cached_kwargs): + for p in required_params: + if '.' in p: + utils.walk_path(cached_kwargs, p.split('.')) + continue + if p not in cached_kwargs: # has all required + message = '{} is a required parameter'.format(p) + raise errors.OctokitParameterError(message) + def _validate_params(self, param_value, kwarg, value, required_params): if not param_value: # is a valid param but not necessarily required message = '{} is not a valid parameter for {}'.format(param_value, kwarg) diff --git a/src/octokit/utils.py b/src/octokit/utils.py index f3742d0..b75044b 100644 --- a/src/octokit/utils.py +++ b/src/octokit/utils.py @@ -15,3 +15,10 @@ def get_json_data(filename): def parameter_transform(params): return {param['name']: param for param in params} + + +def walk_path(obj, path): + if len(path) == 1: + assert obj or obj[path[0]] + else: + walk_path(obj[path[0]], path[1:]) diff --git a/tests/test_methods.py b/tests/test_methods.py index 313adad..06cf808 100644 --- a/tests/test_methods.py +++ b/tests/test_methods.py @@ -237,3 +237,27 @@ class TestClientMethods(object): authorization_id=100, headers={'accept': 'application/vnd.github.ant-man-preview+json'} ) requests.get.assert_called_once_with('https://api.github.com/authorizations/100', params={}, headers=headers) + + def test_dictionary_keys_are_validated(self, mocker): + mocker.patch('requests.put') + headers = {'accept': 'application/vnd.github.v3+json', 'Content-Type': 'application/json'} + data = { + "required_status_checks": { + "strict": True, + "contexts": [], + }, + "required_pull_request_reviews": { + "dismiss_stale_reviews": True + }, + "enforce_admins": True, + "restrictions": { + "users": [], + "teams": [], + } + } + Octokit().repos.update_branch_protection(owner='user', repo='repo', branch='branch', **data) + requests.put.assert_called_with( + 'https://api.github.com/repos/user/repo/branches/branch/protection', + data=json.dumps(data, sort_keys=True), + headers=headers + ) From 4d6bce2d9b299771345fb6c9a82d2dc0279484a1 Mon Sep 17 00:00:00 2001 From: Kyle Hornberg Date: Fri, 5 Oct 2018 10:00:27 -0500 Subject: [PATCH 2/2] Version: 0.9.1 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index cca5253..74063ed 100644 --- a/setup.py +++ b/setup.py @@ -21,7 +21,7 @@ def read(*names, **kwargs): setup( name='octokitpy', - version='0.9.0', + version='0.9.1', license='MIT license', description='Python client for GitHub API', long_description='%s\n%s' % (