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 index redirect in discovery #471

Open
nielsvanvelzen opened this issue Aug 23, 2022 · 4 comments · May be fixed by #943
Open

Follow index redirect in discovery #471

nielsvanvelzen opened this issue Aug 23, 2022 · 4 comments · May be fixed by #943
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@nielsvanvelzen
Copy link
Member

nielsvanvelzen commented Aug 23, 2022

Sometimes people use a baseurl and only have a redirect on the index page (e.g. jellyfin.local -> jellyfin.local/jellyfin). We should support these redirects to allow easier setup in Jellyfin clients.

@AllegraCodes
Copy link

AllegraCodes commented May 17, 2024

I'd like to take this.

Just to be clear, when you say the path should be added to the candidates, should that replace the existing candidates so we only check candidates with the redirected path, or should candidates with the path be checked in addition to the existing candidates, possibly with an increased score?

@nielsvanvelzen
Copy link
Member Author

The AddressCandidateHelper is the is the "helper" that clients can use to convert user-input to actual server addresses. It's a synchronous API and does not make network calls.

The RecommendedServerDiscovery is used to test a list of servers (commonly those outputted from AddressCandidateHelper) by connecting to them and validating the responses.

I would expect the RecommendedServerDiscovery follows redirects and decreases the score slightly (to prefer non-redirected URL's above redirected URL's). The returned RecommendedServerInfo should have a way to indicate it was redirected from the original input, perhaps by adding both the original and final addresses with a getter "isRedirected" that compares them.

The discover function should have a parameter "followRedirects", similar to the option in HttpClientOptions.

@AllegraCodes
Copy link

Behavior can be tested with our own demo server which redirects to the /stable path.

This doesn't work with the demo site currently as it redirects to stable/web/ instead of just stable/

@nielsvanvelzen
Copy link
Member Author

I've been told we're using a baseurl configuration on demo.jellyfin.org so it will indeed redirect to /stable/web/. So it is not suitable to test this behavior with.

@AllegraCodes AllegraCodes linked a pull request May 29, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: planned
2 participants