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

Redirection with baseUrl issues #2086

Closed
Tolriq opened this issue Dec 1, 2019 · 23 comments
Closed

Redirection with baseUrl issues #2086

Tolriq opened this issue Dec 1, 2019 · 23 comments
Labels
bug Something isn't working stale Stale and will be closed if no activity occurs

Comments

@Tolriq
Copy link

Tolriq commented Dec 1, 2019

It seems some API end points redirection are redirecting to index and not the proper url.

http://10.0.0.4:8096/System/Info redirect to http://10.0.0.4:8096/jellyfin/web/index.html

@Tolriq Tolriq added the bug Something isn't working label Dec 1, 2019
@joshuaboniface
Copy link
Member

That's the expected behavior if a baseurl is set. The API endpoint would be /baseurl/System/Info.

The redirection from root when a baseurl is set is quite naive and redirects any invalid location at the root to the main page.

@Tolriq
Copy link
Author

Tolriq commented Dec 1, 2019

Well got tons of users updated with the new path without warning and ways to tell them ;)
And no way to detect properly the baseurl without parsing the url hoping it will always forward to index.

Does not seems really pretty for API consumers, API endpoint should not redirect with an error indicating proper new endpoint or redirect to proper new url.

@joshuaboniface
Copy link
Member

joshuaboniface commented Dec 1, 2019

These users need to read the release notes for JF 10.4.1. To expedite, they can run sudo sed -i 's|<BaseUrl>/jellyfin</BaseUrl>|<BaseUrl/>|' $( sudo find / -name system.xml ) && sudo service jellyfin restart

This has been an ongoing support problem of people not reading the release notes that make EXTREMELY clear what needs to be done to fix this.

I do agree with you there for proper redirects, but that again kinda defeats the BaseURL purpose, since then you're then aliasing / to /baseurl which is NOT what should be happening if a baseurl is set. That was a workaround to prevent even MORE issues with this. It should, ideally, be returning a 404. The redirect is a hack to force users who have no idea what's going on to be able to access the WebUI. In retrospect that was probably a bad idea since a "can't load" would've forced them to read the notes, but alas.

@Tolriq
Copy link
Author

Tolriq commented Dec 1, 2019

Well on the user side, this baseurl and it's impact should not have been added automatically on 10.4 ;)
Users read 10.4 don't see issue, then update automatically for patch release, now 10.4.2 that does no more talk about this :(

Is that baseurl thing even advertised in the auto detection answer?

How would a remote that auto detect Jellyfin find this and properly auto configure, without user having the deal with errors and manual typing.

@Tolriq
Copy link
Author

Tolriq commented Jan 17, 2020

Bump on this, do you intend to expose that base URL in the discovering packet and provide a proper way for API consumers to get it given an IP.
Since you redirect there's no hiding things reason, so having and endpoint answering and giving the url too would be nice.

@Bond-009
Copy link
Member

This is fixed in master, discovery responds with the baseurl appended to the ip

@Tolriq
Copy link
Author

Tolriq commented Jan 17, 2020

Added to what field? Link to PR?

What about the case of non working discovery? What about redirecting System/Info and providing the value there?

@Bond-009
Copy link
Member

The Address field
#2221

@Tolriq
Copy link
Author

Tolriq commented Jan 17, 2020

Ok thanks, that's a start for the vast majority.

Just to be sure the mater nightly are https://repo.jellyfin.org/releases/server/windows/nightly/jellyfin-nightly_20200117_windows-x64.zip ?

@Bond-009
Copy link
Member

Yep

@Tolriq
Copy link
Author

Tolriq commented Jan 18, 2020

Well can't test facing #2255 and same no error in logs :(

@Tolriq
Copy link
Author

Tolriq commented Jan 30, 2020

Ok so now I can test and it's broken :)

As soon as I fill a baseurl in the networking settings Jellyfin stop answering to the auto discover packet.

Tested with https://repo.jellyfin.org/releases/server/windows/nightly/jellyfin-nightly_20200130_windows-x64.zip

@Tolriq
Copy link
Author

Tolriq commented Feb 14, 2020

So now that it's fixed and working can we talk about the last case: manual setup?

Since there's a redirect there's no security and so nothing to hide.

It would be nice that http://xxx:8096/System/Info/Public properly redirect to http://xxxx:8096/baseurl/System/Info/Public that then already properly expose the baseurl in "LocalAddress":"http://xxx:8096/baseurl"

That would solve the last bit of configuration easiness for users.
@joshuaboniface @Bond-009

@dkanada
Copy link
Member

dkanada commented Feb 15, 2020

@Tolriq so that would be useful when a user incorrectly enters the wrong URL and a redirect would resolve that specific issue? Or were you thinking of a different scenario?

@Tolriq
Copy link
Author

Tolriq commented Feb 15, 2020

User should not have to enter an url if possible just an host and a port, client should detect the baseurl if present and it can and if using http or https.
I deal with users configuring Media Center since nearly 10 years now, having them entering an host in a box and a port in another box and only that is greatly limiting issues and user question during set up.

@Tolriq
Copy link
Author

Tolriq commented Mar 7, 2020

So I suppose this is a no? :)

@dkanada
Copy link
Member

dkanada commented Mar 7, 2020

Didn't see this message, apologies. I'm not keen on the idea for the base URL, because it's not set by default, but the other fields can all be handled on the clients themselves.

@Tolriq
Copy link
Author

Tolriq commented Mar 7, 2020

I'm sorry but I have no idea what you are trying to tell in your answer.

The thing is that you already do a blind redirect so this adds no security to hide the baseurl and a client can detect that parse the url and try to guess the data.
This is not logical that seeing that JEllyfin does answer on the port + know the configuration + will do the redirect that it does not offer a way for clients to get that baseurl, either via redirecting the API that is supposed to be used to get the server information or even adding a new end point.

The end user should not have to know or have to type manually that base url, this brings no security just complexity.

@JustAMan
Copy link
Contributor

@Tolriq I was wondering why cannot you try hitting /System/Info/Public, get redirected to /baseurl/web/index.html, strip away web/index.html and get yourself a value of that baseurl thingy?

What you want us to do is somewhat about working around users, and that code (from my point of view) should live as close to a user as possible, hence in a client...

@Tolriq
Copy link
Author

Tolriq commented Mar 11, 2020

What if it's not an Jellyfin server and a global redirect from a proxy? I need to read the index.html content to be sure what it is? This is all but a clean and proper solution.

Why announce the baseurl in the auto detection? Why do the redirect in the first place, the user should know the full path right? If there was no redirect to bring some kind of security then yes force the user to know the details. If there's a global redirect then having the end user to know the details to configure the client is not nice and bring complexity on his shoulders for 0 benefits.

At some point when I share my Jellyfin to other people they should not have to know the internal configuration details and have a complex setup screen to type on a small screen.

Should a user have to type "http://192.168.1.10:8096/toto" or just 192.168.1.10 with an already filed box for the port? You need to think about users as users and not the tech guy that have configured Jellyfin server.

Having the end user manually enter the base url when you already have a global redirect is a UX non sense.
What I expect from you is end to end logic, if there's a global redirect in place then the tools should be able to properly handle it without some hacks to be sure.

Edit: And just for the context, I've been dealing with a few millions users over the last 10 years, so have a "little" background about users needs and limitations when it comes to connect to Media Centers.

@Bond-009
Copy link
Member

Having the end user manually enter the base url when you already have a global redirect is a UX non sense.

That redirect isn't here to stay, it will get removed

@Tolriq
Copy link
Author

Tolriq commented Mar 11, 2020

In that case it's different, just please return a clear error / message something so clients can know that's it's a Jellyfin server and not something else and help the user filling the proper fields.

@stale
Copy link

stale bot commented Jul 9, 2020

This issue has gone 120 days without comment. To avoid abandoned issues, it will be closed in 21 days if there are no new comments.
If you're the original submitter of this issue, please comment confirming if this issue still affects you in the latest release or nightlies, or close the issue if it has been fixed. If you're another user also affected by this bug, please comment confirming so. Either action will remove the stale label.
This bot exists to prevent issues from becoming stale and forgotten. Jellyfin is always moving forward, and bugs are often fixed as side effects of other changes. We therefore ask that bug report authors remain vigilant about their issues to ensure they are closed if fixed, or re-confirmed - perhaps with fresh logs or reproduction examples - regularly. If you have any questions you can reach us on Matrix or Social Media.

@stale stale bot added the stale Stale and will be closed if no activity occurs label Jul 9, 2020
@stale stale bot closed this as completed Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale Stale and will be closed if no activity occurs
Projects
None yet
Development

No branches or pull requests

5 participants