`get_schema_view()` incorrectly sets path prefixes when supplying `url` parameter

[grondman]

Unless I'm missing something, DRF is not adhering to the OpenAPI Specification. The OpenAPI specification 3.0.2 (which is the version currently hardcoded in the DRF source), says the following on the path field of a Paths object: "A relative path to an individual endpoint. The field name MUST begin with a slash. The path is appended (no relative URL resolution) to the expanded URL from the Server Object's url field in order to construct the full URL."

When supplying a url argument to get_schema_view(), I was expecting it to end up in a servers section of the schema, but instead, it gets prepended to all paths in the schema: https://github.com/encode/django-rest-framework/blob/3578683a694e5414c363185b751bcb811d12358a/rest_framework/schemas/openapi.py#L95

This is not in keeping with the specification, those paths - quote spec - "MUST begin with a slash". I'm now working around this behaviour with a custom generator_class, but I think the default should simply adhere to the specification.

[n2ygk]

The way you add extra stuff that is not yet (or may never be) supported to your schema is by extending get_schema() and then filling in the extra stuff. I've documented an example of that here: https://columbia-it-django-jsonapi-training.readthedocs.io/en/latest/documenting-api.html#adding-what-s-missing-to-the-generated-schema

I hope this helps.

On Mon, Nov 9, 2020 at 5:08 AM Ivo Grondman [email protected] wrote:

Unless I'm missing something, DRF is not adhering to the OpenAPI Specification. The OpenAPI specification 3.0.2 (which is the version currently hardcoded in the DRF source), says the following on the path field of a Paths object http://spec.openapis.org/oas/v3.0.2#paths-object: "A relative path to an individual endpoint. The field name MUST begin with a slash. The path is appended (no relative URL resolution) to the expanded URL from the Server Object's url field in order to construct the full URL."

When supplying a url argument to get_schema_view(), I was expecting it to end up in a servers section of the schema, but instead, it gets prepended to all paths in the schema:

https://github.com/encode/django-rest-framework/blob/3578683a694e5414c363185b751bcb811d12358a/rest_framework/schemas/openapi.py#L95

This is not in keeping with the specification, those paths - quote spec - "MUST begin with a slash". I'm now working around this behaviour with a custom generator_class, but I think the default should simply adhere to the specification.

In fact, I didn't see a way of getting a servers section in my schema, unless using a CoreAPIOpenAPIRenderer or CoreAPIJSONOpenAPIRenderer, requiring coreapi to be installed, but I don't see why I should have any ties with coreapi if all I want to use is OpenAPI.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/encode/django-rest-framework/issues/7631, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBHS54JON7WFFVWINRVX4LSO65SZANCNFSM4TPD765A .

[grondman]

That's actually what I meant with "working around this behaviour with a custom generator_class :)

I do strongly object to the notion that I'm asking for "extra stuff". Yes, it is indeed true that the servers section is optional in the specification and I'm perfectly capable of adding that myself. However, DRF should not implement stuff that is not in - or even going against - the specification, which is what is happening here. Supplying the url argument will make the paths be prepended with it, whereas the paths must begin with a slash.

[n2ygk]

I don't quite follow the URL part of your issue. Sounds like you should submit a PR.

My point was just that the openapi code is a work in progress, and that while you are waiting on missing features, there's a documented approach to filling in missing information that's pretty easy to use.

On Mon, Nov 9, 2020 at 8:33 AM Ivo Grondman [email protected] wrote:

That's actually what I meant with "working around this behaviour with a custom generator_class :)

I do strongly object to the notion that I'm asking for "extra stuff". Yes, it is indeed true that the servers section is optional in the specification and I'm perfectly capable of adding that myself. However, DRF should not implement stuff that is not in - or even going against - the specification, which is what is happening here. Supplying the url argument will make the paths be prepended with it, whereas the paths must begin with a slash.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/encode/django-rest-framework/issues/7631#issuecomment-724015730, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBHS57C5PJXT6YV4ERVWOLSO7VSRANCNFSM4TPD765A .

[grondman]

I don't quite follow the URL part of your issue.

I'll try to be as concise as possible. Two sitatuations:

1. I do not suppy the url argument to get_schema_view(), i.e. something like:

get_schema_view(title='Some API', version='1.0.0', 
                renderer_classes=[renderers.JSONOpenAPIRenderer])

This results in a schema like

{
    "openapi": "3.0.2",
    "info": {
        "title": "Some API",
        "version": "1.0.0"
    },
    "paths": {
        "/some-endpoint/": {
            "get": {
                "operationId": "listThings",
                ...
            }
            ...
        }
        ...
    },
    "components": {
      ...
    }
}

This is perfectly adhering to the specification as /some-endpoint/ indeed begins with a slash.

2. I do suppy the url argument to get_schema_view(), i.e. something like:

get_schema_view(title='Some API', version='1.0.0', url='http://somefunkyurl.com/',
                renderer_classes=[renderers.JSONOpenAPIRenderer])

This results in a schema like

{
    "openapi": "3.0.2",
    "info": {
        "title": "Some API",
        "version": "1.0.0"
    },
    "paths": {
        "http://somefunkyurl.com/some-endpoint/": {
            "get": {
                "operationId": "listThings",
                ...
            }
            ...
        }
        ...
    },
    "components": {
      ...
    }
}

The endpoint path no longer starts with a slash, but has the full URL in front of it, which should not be there according to the specification (unless I'm misreading it of course).

Sounds like you should submit a PR.

I might if we agree that this is an actual issue.

My point was just that the openapi code is a work in progress, and that while you are waiting on missing features, there's a documented approach to filling in missing information that's pretty easy to use.

My point is that this is not about missing features or missing information. It's about an already existing implementation that goes against the OpenAPI specification.

[n2ygk]

Aha! Yes, that looks like a bug and that the url should go in the servers array where it belongs. I suggest that you change the title of the issue to something more specific, perhaps along the lines of get_schema_view() url parameter incorrectly sets path prefixes

[grondman]

Glad that we're on the same page now :)

If I have time I can try to come up with the PR myself.

[carltongibson]

Hi both. Thanks for the report and discussion. Looks like a bug yes, so a PR with a test case capturing the error would be great.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cite-reader]

I'm interested in fixing this.

I believe the correct way forward is to simply include the string value of the url parameter as the OpenAPI document's sole server. This shouldn't be a semantic change: the meaning of the keys of the Paths object is to naively appended them to the URLs in server objects, as the current implementation does; however, the server objects' URL field is optionally allowed to be absolute.

[cite-reader]

Requesting comment on a couple of lines in the BaseSchemaGenerator initializer:

        if url and not url.endswith('/'):
            url += '/'

If we do place this url in the server object, this isn't a well-founded precondition: keys of the path object are required to begin with a / character, so forcing one to be present here would be redundant, and the spec's example objects don't include the trailing slash. Is this needed for CoreAPI, in which case these lines ought to be moved to coreapi.SchemaGenerator.__init__, or are they entirely redundant?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[auvipy]

Requesting comment on a couple of lines in the BaseSchemaGenerator initializer:

        if url and not url.endswith('/'):
            url += '/'

If we do place this url in the server object, this isn't a well-founded precondition: keys of the path object are required to begin with a / character, so forcing one to be present here would be redundant, and the spec's example objects don't include the trailing slash. Is this needed for CoreAPI, in which case these lines ought to be moved to coreapi.SchemaGenerator.__init__, or are they entirely redundant?

Core API is now deprecated, even the OpenAPI schema is also scheduled for deprecation in favor of Drf-spectacular

[tomchristie]

Are we able to start closing schema generation tickets at this point, @tfranzel?

[tfranzel]

Yes I think so @tomchristie. Pretty much all the tickets and PRs I saw that addressed DRF's schema generation were solved quite some time ago in spectacular. Just like #8429, which got merged a couple of minutes ago.

Imho it would be wasted effort to keep on fixing things there. It might also give the false impression that there is still substantial improvement on the horizon for that implementation.

[auvipy]

OK I will also hunt down on them