Swiftfin icon indicating copy to clipboard operation
Swiftfin copied to clipboard

Use http by default on ConnectToServerView

Open alasclar opened this issue 1 year ago • 3 comments
trafficstars

Change ConnectToServerView to use http by default, without including in it the text field - as specified by @LePips in #967. Retains previous behaviour of the connect button being disabled when text field is empty.

alasclar avatar Feb 11 '24 17:02 alasclar

I just forgot this last time. Additionally, does with actually work with an https 302 redirect? I recall a while ago people asking about it but can't remember if I did work for it. We need to make sure that works beforehand.

I can tell you that it is working with my setup - I am using nginx proxy manager to both direct my jellyfin domain to the correct port on the server and to create a certificate for https. Both before this change and after, the app would successfully connect to my server whether I specify https or not. I have assumed that the final connection would be over https regardless, the same way a browser redirects to https if available anyway, but I don't actually have any proof.

Outputting the serverState upon connection gives an http url, which isn't a good sign. I will have a look at the code further to see if I can find any proof either way.

alasclar avatar Feb 16 '24 13:02 alasclar

@LePips I haven't had much luck with this, I haven't found any evidence that the https redirect is actually happening. If someone with more networking expertise knows that'd be great, but otherwise I don't think it makes sense to assume http in the text entry - as if I don't have to specify the prefix I, as a user, would assume that https is being used if available.

To be honest if a redirect isn't happening I think my previous PR #967 doesn't look like a bad option, but it could also make sense to just add that redirect functionality in. I could achieve that by just calling connectToServer twice and silencing the thrown errors (first thing that came into my head), but that's definitely not the most efficient way of doing that.

alasclar avatar Feb 20 '24 10:02 alasclar

No, we just need to figure out what actually happens with a 301/302 redirect. I remember doing work for it but can't remember if I broke it. Let's not over-complicate things since we should handle that anyways.

I'm not able to personally set anything up for a while but I'll get to it when I can.

LePips avatar Feb 20 '24 16:02 LePips

Sorry it's been a while but I took at this today and sadly a lot more post-processing was required in order to make this possible. We actually are fine on properly handling redirects, however the issue came from how we store the connection URL, which should be the final URL from the response if it's different (I think, idk if this has some weird RFC technicalities).

URL manipulation via strings always feels hacky but this works with a basic http -> https redirect. I wasn't able to test with a redirect to a completely different URL, or from/to a server with the server living at a subpath. However, we can deal with that if it comes up.

If you would like to continue this work, you can see my branch below and merge it into this PR and fix for tvOS since you started this work. If not, I can open a different PR to merge.

  • https://github.com/jellyfin/Swiftfin/compare/main...LePips:SwiftFin:handle-redirect-on-server-connect

LePips avatar Apr 16 '24 23:04 LePips