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

Reintroduce header param type conversion #1931

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CordulaGuder
Copy link

This PR fixes the type conversion of header params that seems to have gone missing during the refactoring.

In Connexion 2, the ParameterValidator used to do type conversion in its main function validate_parameter, so it did that for all kinds of params it came across. In Connexion 3, this has moved to the URIParser classes (connexion.uri_parsing), so header params are not being type converted anymore. This PR fixes this bug.

Changes proposed in this pull request:

  • add type conversion to validate_header_parameter in ParameterValidator

@RobbeSneyders
Copy link
Member

Thanks @CordulaGuder,

We explicitly deduplicated all coerce_type calls during the refactoring and moved them into the uri_parser so it is done in a single place. I indeed see that this misses the headers.

However by implementing it in the validation, the types in the headers are validated after coercion while they are passed to the application without coercion. If we want to reactivate this, we should centralize it as well. We can either also do it in the URIParser, although I'm not sure if that makes sense, or in the ConnexionRequest class.

See #1934 which was triggered by reviewing this PR.

RobbeSneyders added a commit that referenced this pull request May 28, 2024
During the refactoring for Connexion 3, we deduplicated all
`coerce_type` calls during the refactoring and moved them into the
`uri_parser` so it is done in a single place. When looking into #1931
however, I noticed we are still using the `uri_parser` from more places
than we should.

This PR centralizes the parsing further into the `ConnexionRequest`
class. During validation, we now instantiate a `ConnexionRequest`
instead of a Starlette `Request`, and leverage it instead of calling the
`uri_parser` directly.

I split the parsing and validation in the tests so they can be tested in
isolation.
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.

2 participants