Skip to content

Commit

Permalink
Fix sanitization of function arguments (#1442)
Browse files Browse the repository at this point in the history
* Add test for incorrectly cased params with pythonic_params

* Fix 1307: only sanitize when checking view func arguments

* Don't sanitize original request form

* Fix OpenAPI3 test fixture for form data

* Fix view function for test
  • Loading branch information
Ruwann committed Feb 1, 2022
1 parent 86af42d commit 1f07bde
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 17 deletions.
2 changes: 1 addition & 1 deletion connexion/decorators/parameter.py
Expand Up @@ -84,7 +84,7 @@ def wrapper(request):
if all_json(consumes):
request_body = request.json
elif consumes[0] in FORM_CONTENT_TYPES:
request_body = {sanitize(k): v for k, v in request.form.items()}
request_body = request.form
else:
request_body = request.body

Expand Down
15 changes: 9 additions & 6 deletions connexion/operations/abstract.py
Expand Up @@ -191,18 +191,21 @@ def _query_args_helper(self, query_defns, query_arguments,
function_arguments, has_kwargs, sanitize):
res = {}
for key, value in query_arguments.items():
key = sanitize(key)
if not has_kwargs and key not in function_arguments:
logger.debug("Query Parameter '%s' not in function arguments", key)
sanitized_key = sanitize(key)
if not has_kwargs and sanitized_key not in function_arguments:
logger.debug("Query Parameter '%s' (sanitized: '%s') not in function arguments",
key, sanitized_key)
else:
logger.debug("Query Parameter '%s' in function arguments", key)
logger.debug("Query Parameter '%s' (sanitized: '%s') in function arguments",
key, sanitized_key)
try:
query_defn = query_defns[key]
except KeyError: # pragma: no cover
logger.error(f"Function argument '{key}' not defined in specification")
logger.error("Function argument '%s' (non-sanitized: %s) not defined in specification",
sanitized_key, key)
else:
logger.debug('%s is a %s', key, query_defn)
res.update({key: self._get_val_from_param(value, query_defn)})
res.update({sanitized_key: self._get_val_from_param(value, query_defn)})
return res

@abc.abstractmethod
Expand Down
2 changes: 1 addition & 1 deletion connexion/operations/openapi.py
Expand Up @@ -362,7 +362,7 @@ def _get_query_defaults(self, query_defns):
return defaults

def _get_query_arguments(self, query, arguments, has_kwargs, sanitize):
query_defns = {sanitize(p["name"]): p
query_defns = {p["name"]: p
for p in self.parameters
if p["in"] == "query"}
default_query_params = self._get_query_defaults(query_defns)
Expand Down
18 changes: 11 additions & 7 deletions connexion/operations/swagger2.py
Expand Up @@ -251,7 +251,7 @@ def body_definition(self):
return body_parameters[0] if body_parameters else {}

def _get_query_arguments(self, query, arguments, has_kwargs, sanitize):
query_defns = {sanitize(p["name"]): p
query_defns = {p["name"]: p
for p in self.parameters
if p["in"] == "query"}
default_query_params = {k: v['default']
Expand All @@ -269,7 +269,7 @@ def _get_body_argument(self, body, arguments, has_kwargs, sanitize):
body = deepcopy(body_parameters[0].get('schema', {}).get('default'))
body_name = sanitize(body_parameters[0].get('name'))

form_defns = {sanitize(p['name']): p
form_defns = {p['name']: p
for p in self.parameters
if p['in'] == 'formData'}

Expand All @@ -290,16 +290,20 @@ def _get_body_argument(self, body, arguments, has_kwargs, sanitize):
if form_defns and body:
form_arguments.update(body)
for key, value in form_arguments.items():
if not has_kwargs and key not in arguments:
logger.debug("FormData parameter '%s' not in function arguments", key)
sanitized_key = sanitize(key)
if not has_kwargs and sanitized_key not in arguments:
logger.debug("FormData parameter '%s' (sanitized: '%s') not in function arguments",
key, sanitized_key)
else:
logger.debug("FormData parameter '%s' in function arguments", key)
logger.debug("FormData parameter '%s' (sanitized: '%s') in function arguments",
key, sanitized_key)
try:
form_defn = form_defns[key]
except KeyError: # pragma: no cover
logger.error(f"Function argument '{key}' not defined in specification")
logger.error("Function argument '%s' (non-sanitized: %s) not defined in specification",
key, sanitized_key)
else:
kwargs[key] = self._get_val_from_param(value, form_defn)
kwargs[sanitized_key] = self._get_val_from_param(value, form_defn)
return kwargs

def _get_val_from_param(self, value, query_defn):
Expand Down
16 changes: 16 additions & 0 deletions tests/api/test_parameters.py
Expand Up @@ -472,6 +472,22 @@ def test_parameters_snake_case(snake_case_app):
assert resp.status_code == 200
resp = app_client.get('/v1.0/test-get-query-shadow?list=123')
assert resp.status_code == 200
# Tests for when CamelCase parameter is supplied, of which the snake_case version
# matches an existing parameter and view func argument, or vice versa
resp = app_client.get('/v1.0/test-get-camel-case-version?truthiness=true&orderBy=asc')
assert resp.status_code == 200
assert resp.get_json() == {'truthiness': True, 'order_by': 'asc'}
resp = app_client.get('/v1.0/test-get-camel-case-version?truthiness=5')
assert resp.status_code == 400
assert resp.get_json()['detail'] == "Wrong type, expected 'boolean' for query parameter 'truthiness'"
# Incorrectly cased params should be ignored
resp = app_client.get('/v1.0/test-get-camel-case-version?Truthiness=true&order_by=asc')
assert resp.status_code == 200
assert resp.get_json() == {'truthiness': False, 'order_by': None} # default values
resp = app_client.get('/v1.0/test-get-camel-case-version?Truthiness=5&order_by=4')
assert resp.status_code == 200
assert resp.get_json() == {'truthiness': False, 'order_by': None} # default values
# TODO: Add tests for body parameters


def test_get_unicode_request(simple_app):
Expand Down
2 changes: 1 addition & 1 deletion tests/fakeapi/hello/__init__.py
Expand Up @@ -474,7 +474,7 @@ def test_param_sanitization3(query=None, body=None):
if query:
result['query'] = query
if body:
result['form'] = body["form"]
result['form'] = body["$form"]
return result


Expand Down
5 changes: 5 additions & 0 deletions tests/fakeapi/snake_case.py
Expand Up @@ -19,6 +19,11 @@ def get_query_shadow(list_):
return data


def get_camelcase(truthiness, order_by=None):
data = {'truthiness': truthiness, 'order_by': order_by}
return data


def post_path_snake(some_id, some_other_id):
data = {'SomeId': some_id, 'SomeOtherId': some_other_id}
return data
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/simple/openapi.yaml
Expand Up @@ -932,7 +932,7 @@ paths:
schema:
type: object
properties:
'form$':
'$form':
description: Just a testing parameter in the form data
type: string
/body-sanitization:
Expand Down
21 changes: 21 additions & 0 deletions tests/fixtures/snake_case/openapi.yaml
Expand Up @@ -79,6 +79,27 @@ paths:
required: true
schema:
type: integer
/test-get-camel-case-version:
get:
summary: Test for CamelCase version of query parameter
description: Test for when a wrongly cased parameter is supplied.
operationId: fakeapi.snake_case.get_camelcase
responses:
'200':
description: OK
parameters:
- name: truthiness
in: query
description: A test parameter, for which 'Truthiness' will be given in the request.
schema:
type: boolean
default: false
- name: orderBy
in: query
description: A CamelCase parameter, for which 'order_by' will be given in the request.
schema:
type: string

'/test-post-path-snake/{SomeId}':
post:
summary: Test converting to snake_case in path
Expand Down
18 changes: 18 additions & 0 deletions tests/fixtures/snake_case/swagger.yaml
Expand Up @@ -169,3 +169,21 @@ paths:
required: true
schema:
type: object
/test-get-camel-case-version:
get:
summary: Test for CamelCase version of query parameter
description: Test for when the CamelCase version of a query parameter is provided.
operationId: fakeapi.snake_case.get_camelcase
responses:
'200':
description: OK
parameters:
- name: truthiness
in: query
description: A test parameter, for which 'Truthiness' will be given in the request.
type: boolean
default: false
- name: orderBy
in: query
description: A CamelCase parameter, for which 'order_by' will be given in the request.
type: string

0 comments on commit 1f07bde

Please sign in to comment.