spec-first / connexion

Connexion is a modern Python web framework that makes spec-first and api-first development easy.
https://connexion.readthedocs.io/en/latest/
Apache License 2.0
4.47k stars 758 forks source link

When basePath isn't specified, it defaults to "" when rendered to a file, which is invalid. #1315

Open AndTheDaysGoBy opened 3 years ago

AndTheDaysGoBy commented 3 years ago

Description

Suppose you have a yaml spec. in which "basePath" is left undefined. Until connexion 2.5.0, when connexion renders /swagger.json and /swagger.yaml, basePath would not be added, the desired functionality.

However, as of connexion 2.5.1, the basePath would always be set to basePath: '' when not specified, which is invalid according to the OpenAPI specification. (https://swagger.io/specification/v2/)

Expected behaviour

The expected behavior i is that if basePath is not specified, it is not added and, if it is added, it is to a valid value, i.e. basePath: '/'. Similarly, if basePath is set to /, i.e. basePath: '/' it'll be served with that value from /swagger.json and /swagger.yaml.

Actual behaviour

The basePath is set to basePath: '', an invalid value.

Steps to reproduce

pip install connexion==2.5.0 have a spec. without the basePath key. Proceed to start the connexion app. and query /swagger.json or /swagger.yaml. Perform openapi-spec-validator --schema 2.0 result.yaml on the data returned. It will succeed. Now, perform pip install -U connexion==2.5.1 and try this again, now basePath: '' is set, hence it'll fail as invalid.

Root Cause -- Always Setting the Base Path + /something Resolves to ["", "something"]

I dug into the source of the issue myself. In order to generate /swagger.json or /swagger.yaml, https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/apis/flask_api.py#L304 or https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/apis/flask_api.py#L307 must be called.

Let us consider the swagger.yaml case for clarity. Previously, what would be returned is yamldumper(self.specification.raw) however, presently, yamldumper(self._spec_for_prefix()) is returned. This is the key distinction. Changing the code to self.specification.raw keeps the invalid basePath: '' from being added.

Now, the reason https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/apis/flask_api.py#L310 results in issues for generating the spec is because the flask url will resolve to /swagger.yaml which, upon performing rsplit('/', 1), yields ['', 'swagger.json'], thus, selecting the 0th position, the base path is set to ''. In particular, https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/apis/flask_api.py#L316 is called (with_base_path) which results in setting https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/spec.py#L203 the basePath. Previously, it wasn't being set at all so, when rendered, basePath wasn't included. Now, we're always setting it. Furthermore, since https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/spec.py#L204 is called whenever we set the base path, it becomes impossible to set basePath to / without it being stripped due to the rstrip https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/spec.py#L30

This wasn't being caught despite the fact that https://github.com/zalando/connexion/blob/1ed14fbde5be3d144760fec4982e90064f151e88/connexion/spec.py#L38 occurs because that validates the initial spec., not what's served.

wrouesnel commented 1 year ago

I can confirm this issue is still present in 2.14.0 as well as of 2022.