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.

connexion/uri_parsing.py

@@ -9,7 +9,7 @@
 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,14 +119,12 @@ def resolve_params(self, params, _in):
             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
-                    )
-                except TypeValidationError:
-                    pass
+            try:
+                resolved_param[k] = coerce_type(
+                    param_defn, resolved_param[k], "parameter", k
+                )
+            except TypeValidationError:
+                pass
 
         return resolved_param
 
@@ -166,6 +164,7 @@ def resolve_form(self, form_data):
                 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):

connexion/validators/form_data.py

@@ -6,14 +6,10 @@
 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 @@ def _validate(self, data: dict) -> None:
             )
             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,30 +81,29 @@ def validate(self, data: FormData) -> None:
                 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
             data.update(file_data)
         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:

connexion/validators/parameter.py

@@ -5,12 +5,8 @@
 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 @@ def __init__(self, parameters, uri_parser, strict_validation=False):
 
     @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)
+        if is_nullable(param) and is_null(value):
+            return
 
+        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 @@ def validate_query_parameter(self, param, request):
         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):

tests/api/test_parameters.py

@@ -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"

tests/api/test_unordered_definition.py

@@ -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'")

tests/decorators/test_validation.py

@@ -2,6 +2,7 @@
 
 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,15 +88,14 @@ def test_invalid_type(monkeypatch):
 On instance:
     20"""
     assert result == expected_result
-    logger.info.assert_called_once()
 
 
 def test_invalid_type_value_error(monkeypatch):
     value = {"test": 1, "second": 2}
     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):

tests/test_validation.py

@@ -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={}