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

Better handling of HTTP redirects #2512

Closed
mitar opened this issue Mar 29, 2021 · 3 comments
Closed

Better handling of HTTP redirects #2512

mitar opened this issue Mar 29, 2021 · 3 comments
Labels
http Supporting HTTP features and interactions

Comments

@mitar
Copy link

mitar commented Mar 29, 2021

It looks to me like the OpenAPI specification does not have enough of support for HTTP redirects returned from the API. For example, we have /user/me endpoint which returns a HTTP redirect to something like /user/<id> based on the user associated with the access token used for the API call. We allow only GET requests on /user/me while we allow a full set of HTTP methods on /user/<id> (like creation of the user, deletion, updating).

Now the issue is how to describe this /user/me endpoint. If we just specify that it returns a 307 HTTP status code and Location header (which is probably the closest to what exactly the call is returning) then this seems to not be well supported by codegen clients. Some codegen clients have to know how to parse the response, but there is no information about the response schema after redirect, or in other words, there is no information in the spec about where will Location always point to, so client cannot prepare for that in advance: only dynamically after Location header is received it could check to which API endpoint it is pointing to and parse response using that API endpoint's response handling code. But if one wants to have all API responses handled statically, this is not possible.

Moreover, in browsers, you cannot really obtain Location header for 3xx responses using fetch. So you cannot manually traverse the redirect here. Which means that the client in the browser cannot really know where it is going in the redirect and only after it already receives the response it might be able to figure out where it got redirected and how to parse the response.

In summary, I think OpenAPI specification is missing a critical component to specify redirects: where does redirects go to? Is it to some other endpoint and which one, or is it something completely else. By knowing to which endpoint it is going, codegen client could know how to parse API responses after redirect.

@silviokaviedes
Copy link

Though the initial report stands correct, i am wondering - is it even worth to address this issue or to come up with a solution? It appears to me that a Location-Header that references to another operation of the same API has so many drawbacks. Why not just return the id of a resource within the returned payload? The consumer knows by definition of the Open-API-spec how to reach the resource.

@spacether
Copy link

spacether commented Oct 4, 2022

Thinking about this as a client implementor.

  • I would not want clients to follow redirects because I want the type hint of the endpoint response to be correct. That type hint would be no body and the new url location from the header.
  • The user of the client can take the returned response and if the status is in the 300 range then the Location header should be present and usable as the new location.
  • Assuming the location is in the current api, the code using the client can then call the new endpoint

If one wanted to, one could bake that all in to a fn signature of the endpoint with follow_redirect = True BUT that would make the types returned by that signature a lot more complicated. What if the redirection location also redirects? What if errors are thrown at any depth in a redirection chain?

@handrews
Copy link
Member

It's possible to describe the Location header under the Response Object:

responses:
  "307":
    description: Redirect
    headers:
      Location:
        content:
          text/plain:
            schema:
              const: /some/other/resource

There reason for the content: {text/plain: {...}} part is that if you use schema in a Header Object, the string is URI percent-encoded, which would percent-encode the / characters which you don't want (see also PR #3841)

If you are in 3.0, you will need to use a 1-element enum instead of const.

Alternatively, you could probably use a Link Object to do this, although I'm not as familiar with that so I'm not absolutely certain. Plus it sounds like you just want to hard-wire a redirect destination so what I have shown is simpler.

I'm 99.999% sure this answers your question, and since this is a years-old question (my apologies on that.. the project kind of went into hibernation for a bit), I'm going to close this as answered. Please feel free to file a new issue if this is not sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Supporting HTTP features and interactions
Projects
None yet
Development

No branches or pull requests

4 participants