Coerce types only in uri parser (#1627)

This PR moves all type coercing into the URI parsers and makes sure it's
only done once for each code path.
This commit is contained in:
Robbe Sneyders
2023-01-30 08:49:49 +01:00
committed by GitHub
parent 769120e5a0
commit 022bb8f34a
7 changed files with 40 additions and 84 deletions

View File

@@ -9,7 +9,7 @@ import logging
import re
from connexion.exceptions import TypeValidationError
from connexion.utils import all_json, coerce_type, deep_merge, is_null, is_nullable
from connexion.utils import all_json, coerce_type, deep_merge
logger = logging.getLogger("connexion.decorators.uri_parsing")
@@ -119,9 +119,7 @@ class AbstractURIParser(metaclass=abc.ABCMeta):
else:
resolved_param[k] = values[-1]
if not (is_nullable(param_defn) and is_null(resolved_param[k])):
try:
# TODO: coerce types in a single place
resolved_param[k] = coerce_type(
param_defn, resolved_param[k], "parameter", k
)
@@ -166,6 +164,7 @@ class OpenAPIURIParser(AbstractURIParser):
form_data[k] = self._split(form_data[k], encoding, "form")
elif "contentType" in encoding and all_json([encoding.get("contentType")]):
form_data[k] = json.loads(form_data[k])
form_data[k] = coerce_type(defn, form_data[k], "requestBody", k)
return form_data
def _make_deep_object(self, k, v):

View File

@@ -6,14 +6,10 @@ from starlette.datastructures import FormData, Headers, UploadFile
from starlette.formparsers import FormParser, MultiPartParser
from starlette.types import Receive, Scope
from connexion.exceptions import (
BadRequestProblem,
ExtraParameterProblem,
TypeValidationError,
)
from connexion.exceptions import BadRequestProblem, ExtraParameterProblem
from connexion.json_schema import Draft4RequestValidator
from connexion.uri_parsing import AbstractURIParser
from connexion.utils import coerce_type, is_null
from connexion.utils import is_null
logger = logging.getLogger("connexion.validators.form_data")
@@ -76,16 +72,7 @@ class FormDataValidator:
)
raise BadRequestProblem(detail=f"{exception.message}{error_path_msg}")
def validate(self, data: FormData) -> None:
if self.strict_validation:
form_params = data.keys()
spec_params = self.schema.get("properties", {}).keys()
errors = set(form_params).difference(set(spec_params))
if errors:
raise ExtraParameterProblem(errors, [])
props = self.schema.get("properties", {})
errs = []
def _parse(self, data: FormData) -> dict:
if self.uri_parser is not None:
# Don't parse file_data
form_data = {}
@@ -94,7 +81,8 @@ class FormDataValidator:
if isinstance(v, str):
form_data[k] = data.getlist(k)
elif isinstance(v, UploadFile):
file_data[k] = data.getlist(k)
# Replace files with empty strings for validation
file_data[k] = ""
data = self.uri_parser.resolve_form(form_data)
# Add the files again
@@ -102,22 +90,20 @@ class FormDataValidator:
else:
data = {k: data.getlist(k) for k in data}
for k, param_defn in props.items():
if k in data:
if param_defn.get("format", "") == "binary":
# Replace files with empty strings for validation
data[k] = ""
continue
return data
try:
data[k] = coerce_type(param_defn, data[k], "requestBody", k)
except TypeValidationError as e:
logger.exception(e)
errs += [str(e)]
def _validate_strictly(self, data: FormData) -> None:
form_params = data.keys()
spec_params = self.schema.get("properties", {}).keys()
errors = set(form_params).difference(set(spec_params))
if errors:
raise ExtraParameterProblem(errors, [])
if errs:
raise BadRequestProblem(detail=errs)
def validate(self, data: FormData) -> None:
if self.strict_validation:
self._validate_strictly(data)
data = self._parse(data)
self._validate(data)
async def wrapped_receive(self) -> Receive:

View File

@@ -5,12 +5,8 @@ import logging
from jsonschema import Draft4Validator, ValidationError
from starlette.requests import Request
from connexion.exceptions import (
BadRequestProblem,
ExtraParameterProblem,
TypeValidationError,
)
from connexion.utils import boolean, coerce_type, is_null, is_nullable
from connexion.exceptions import BadRequestProblem, ExtraParameterProblem
from connexion.utils import boolean, is_null, is_nullable
logger = logging.getLogger("connexion.validators.parameter")
@@ -38,35 +34,17 @@ class ParameterValidator:
@staticmethod
def validate_parameter(parameter_type, value, param, param_name=None):
if value is not None:
if is_nullable(param) and is_null(value):
return
try:
converted_value = coerce_type(param, value, parameter_type, param_name)
except TypeValidationError as e:
return str(e)
elif value is not None:
param = copy.deepcopy(param)
param = param.get("schema", param)
if "required" in param:
del param["required"]
try:
Draft4Validator(param, format_checker=draft4_format_checker).validate(
converted_value
value
)
except ValidationError as exception:
debug_msg = (
"Error while converting value {converted_value} from param "
"{type_converted_value} of type real type {param_type} to the declared type {param}"
)
fmt_params = dict(
converted_value=str(converted_value),
type_converted_value=type(converted_value),
param_type=param.get("type"),
param=param,
)
logger.info(debug_msg.format(**fmt_params))
return str(exception)
elif param.get("required"):
@@ -102,10 +80,8 @@ class ParameterValidator:
return self.validate_parameter("query", val, param)
def validate_path_parameter(self, param, request):
# TODO: activate
# path_params = self.uri_parser.resolve_path(request.path_params)
# val = path_params.get(param["name"].replace("-", "_"))
val = request.path_params.get(param["name"].replace("-", "_"))
path_params = self.uri_parser.resolve_path(request.path_params)
val = path_params.get(param["name"].replace("-", "_"))
return self.validate_parameter("path", val, param)
def validate_header_parameter(self, param, request):

View File

@@ -561,10 +561,7 @@ def test_parameters_snake_case(snake_case_app):
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'"
)
assert resp.get_json()["detail"].startswith("'5' is not of type 'boolean'")
# Incorrectly cased params should be ignored
resp = app_client.get(
"/v1.0/test-get-camel-case-version?Truthiness=true&order_by=asc"

View File

@@ -8,7 +8,4 @@ def test_app(unordered_definition_app):
) # type: flask.Response
assert response.status_code == 400
response_data = json.loads(response.data.decode("utf-8", "replace"))
assert (
response_data["detail"]
== "Wrong type, expected 'integer' for query parameter 'first'"
)
assert response_data["detail"].startswith("'first' is not of type 'integer'")

View File

@@ -2,6 +2,7 @@ from unittest.mock import MagicMock
import pytest
from connexion.json_schema import Draft4RequestValidator, Draft4ResponseValidator
from connexion.utils import coerce_type
from connexion.validators.parameter import ParameterValidator
from jsonschema import ValidationError
@@ -68,6 +69,7 @@ def test_get_valid_parameter_with_enum_array_header():
},
"name": "test_header_param",
}
value = coerce_type(param, value, "header", "test_header_param")
result = ParameterValidator.validate_parameter("header", value, param)
assert result is None
@@ -86,7 +88,6 @@ Failed validating 'type' in schema:
On instance:
20"""
assert result == expected_result
logger.info.assert_called_once()
def test_invalid_type_value_error(monkeypatch):
@@ -94,7 +95,7 @@ def test_invalid_type_value_error(monkeypatch):
result = ParameterValidator.validate_parameter(
"formdata", value, {"type": "boolean", "name": "foo"}
)
assert result == "Wrong type, expected 'boolean' for formdata parameter 'foo'"
assert result.startswith("{'test': 1, 'second': 2} is not of type 'boolean'")
def test_enum_error(monkeypatch):

View File

@@ -42,17 +42,17 @@ def test_parameter_validator(monkeypatch):
request = MagicMock(path_params={"p1": ""}, **kwargs)
with pytest.raises(BadRequestProblem) as exc:
validator.validate_request(request)
assert exc.value.detail == "Wrong type, expected 'integer' for path parameter 'p1'"
assert exc.value.detail.startswith("'' is not of type 'integer'")
request = MagicMock(path_params={"p1": "foo"}, **kwargs)
with pytest.raises(BadRequestProblem) as exc:
validator.validate_request(request)
assert exc.value.detail == "Wrong type, expected 'integer' for path parameter 'p1'"
assert exc.value.detail.startswith("'foo' is not of type 'integer'")
request = MagicMock(path_params={"p1": "1.2"}, **kwargs)
with pytest.raises(BadRequestProblem) as exc:
validator.validate_request(request)
assert exc.value.detail == "Wrong type, expected 'integer' for path parameter 'p1'"
assert exc.value.detail.startswith("'1.2' is not of type 'integer'")
request = MagicMock(
path_params={"p1": 1}, query_params=QueryParams("q1=4"), headers={}, cookies={}