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 form data validator for validation middleware #1595

Merged
merged 5 commits into from
Nov 4, 2022

Conversation

RobbeSneyders
Copy link
Member

@RobbeSneyders RobbeSneyders commented Sep 24, 2022

This PR moves form data validation to the middleware, which is quite complex due to differences between Swagger 2 and OpenAPI 3 in how form data is represented.

For OpenAPI 3, this is quite simple. The form data is represented as an object in the request body, which means we can just use a request body validator.

For Swagger 2, this is more complicated. The form data is represented as parameters instead. I see two options here:

  • We validate the form data as parameters, which is what Connexion is currently doing. The advantage is that you can easily use the current parameter validation with the spec. The downside is that it's different from OpenAPI 3 and you need to consume the stream in the parameter validation.
  • We transform the FormData parameters in the Swagger2Operation to a body schema spec as in OpenAPI 3. Advantage is that we can then use the same request body validator and only consume the stream there. The downside is additional complexity in the Swagger2Operation.

This PR implements option 2.

Some open points still to address with the request body validator in this PR:

  • I implemented strict_validation, which makes less sense for OpenAPI 3 form validation, as this can now be controlled by additionalProperties in the schema.instead.

@@ -56,7 +67,7 @@ def validate(self, body: dict):
)
raise BadRequestProblem(detail=f"{exception.message}{error_path_msg}")

async def receive(self) -> t.Optional[t.MutableMapping[str, t.Any]]:
async def wrapped_receive(self) -> Receive:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this so validation is not delayed until the stream is consumed by the application. I think it makes sense for connexion to always validate the body (for certain content types), independent on if it's read in the application or not. This also makes it easier to follow / debug validation, and prevents further middlewares / application logic from being executed for an invalid request.

Base automatically changed from feature/response-validation to main October 3, 2022 21:01
@RobbeSneyders RobbeSneyders added this to the Connexion 3.0 milestone Oct 4, 2022
@coveralls
Copy link

coveralls commented Oct 14, 2022

Pull Request Test Coverage Report for Build 3392925032

  • 194 of 205 (94.63%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 94.339%

Changes Missing Coverage Covered Lines Changed/Added Lines %
connexion/datastructures.py 16 18 88.89%
connexion/operations/swagger2.py 45 47 95.74%
connexion/validators.py 99 106 93.4%
Totals Coverage Status
Change from base Build 3202603702: -0.02%
Covered Lines: 2933
Relevant Lines: 3109

💛 - Coveralls

RobbeSneyders and others added 2 commits November 4, 2022 00:42
Add python-multipart dependency to setup.py

Copy validator map so it remains unchanged
@RobbeSneyders RobbeSneyders marked this pull request as ready for review November 3, 2022 23:48
@RobbeSneyders RobbeSneyders changed the title Add form data validator for OpenAPI 3 Add form data validator for validation middleware Nov 3, 2022
* Move Swagger 2 form validation to middleware

* Add unit test for form transformation
* Create MediaTypeDict class for range matching

* Extract parsing instead of using jsonifier

* Add default validator class as parameter defaults

* Clean up form validation
@RobbeSneyders RobbeSneyders merged commit 670bee9 into main Nov 4, 2022
@RobbeSneyders RobbeSneyders deleted the feature/form-validation branch November 4, 2022 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants