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

Follow HTTP redirect in remote discovery #929

Conversation

AllegraCodes
Copy link

Optionally add the result of an HTTP redirect to the discovered servers during the remote discovery process, but only if the redirect stays on the same domain.

Resolves #471

Currently done by adding a HEAD request method to the KtorClient, because the generic request method requires an OK status. If this will remain, there is the opportunity to extract common code to reduce the current duplication, but this may also change as a result of bringing this code in line with the existing api patterns.

Allegra added 4 commits May 17, 2024 20:17
…ubpath of the original.

Adds a parameter to RecommendedServerDiscovery.discover() that indicates if we should follow a redirect from the given servers.
If true, send a HEAD request to the given servers, and if it returns a redirect code then add the redirect location to the collection of servers to check, but only if the redirected location is on the same host as the original.
Prefer non-redirected addresses over those that are the result of a redirect.
Pass the original and possible redirected address back through the RecommendedServerInfo object for use.
Use the Headers object instead of a standard map to provide case-insensitive access to the headers.
Correctly override the path on the current URL when given a path-only redirect
@AllegraCodes
Copy link
Author

If I'm understanding the api code generation correctly, Api objects like systemApi that do most/all of the requests currently are generated based off the server api, so we can't add a head request to those, and therefore it's ok to do the head request directly through the KtorClient

@AllegraCodes AllegraCodes marked this pull request as ready for review May 19, 2024 10:27
@AllegraCodes
Copy link
Author

see #943

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.

Follow index redirect in discovery
2 participants