spotDL / spotify-downloader

Download your Spotify playlists and songs along with album art and metadata (from YouTube if a match is found).
https://spotdl.readthedocs.io/en/latest/
MIT License
15.79k stars 1.5k forks source link

Web-ui Not Using Designated Port #2067

Closed Alchemichai closed 1 week ago

Alchemichai commented 3 months ago

System OS

Docker

Python Version

3.7 (CPython)

Install Source

pip / PyPi

Install version / commit hash

4.2.5

Expected Behavior vs Actual Behavior

The spotdl web port option causes the web client to stop being able to communicate with the server. This presents as "Error: Network Error" when searching for music. This seems to be because the pre-compiled webpage is just downloaded directly from the web-ui repo here: https://github.com/spotDL/spotify-downloader/blob/bd388458e3d83b107a041da517d63a7f6932f81a/spotdl/console/web.py#L68-L71 The web page has been compiled with the port hard-coded to "8800", search the below line for "8800": https://github.com/spotDL/web-ui/blob/292bdeb351a0c1d90d7ac509293dfcf63d69ebf0/dist/assets/index-CGxZV3ZT.js#L22

const Ge = {
    PROTOCOL: Hn.PROTOCOL || window.location.protocol,
    WS_PROTOCOL: Hn.WS_PROTOCOL || window.location.protocol === "https:" ? "wss:" : "ws:",
    BACKEND: Hn.BACKEND || window.location.hostname,
    PORT: "8800",
    WS_PORT: "8800",
    BASEURL: Hn.BASEURL || ""
};

The source code that generated the suspect compiled code is below, I'm not sure why it compiled to a static value here, I'm a bit of a novice. https://github.com/spotDL/web-ui/blob/292bdeb351a0c1d90d7ac509293dfcf63d69ebf0/src/config.js#L1-L10

const config = {
  PROTOCOL: process.env.PROTOCOL || window.location.protocol,
  WS_PROTOCOL: process.env.WS_PROTOCOL || window.location.protocol === 'https:' ? 'wss:' : 'ws:',
  BACKEND: process.env.BACKEND || window.location.hostname,
  PORT: process.env.PORT || window.location.port,
  WS_PORT: process.env.WS_PORT || window.location.port,
  BASEURL: process.env.BASEURL || '',
}

Steps to reproduce - Ensure to include actual links!

  1. spotdl web --port 6969
  2. ctrl-shit-i (may have to reload the page)
  3. Note that some GET requests have failed as they attempted to use port 8800 instead of 6969, not nice. Also the websocket failed to connect for the same reason.

Traceback

Updating web app

Files are stored in temporary directory and will be deleted after the program exits to save them to current directory permanently enable the
`web_use_output_dir` option
Starting web server

INFO:     Started server process [6684]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     Uvicorn running on http://localhost:6969 (Press CTRL+C to quit)
INFO:     ::1:54677 - "GET / HTTP/1.1" 200 OK
INFO:     ::1:54677 - "GET /assets/index-CGxZV3ZT.js HTTP/1.1" 200 OK
INFO:     ::1:54678 - "GET /assets/index-BDDgGyM9.css HTTP/1.1" 200 OK
INFO:     ::1:54677 - "GET /favicon.ico HTTP/1.1" 200 OK

Other details

No response

Alchemichai commented 3 months ago

I had almost forgotten as I fixed the offending js with an override; This also prevents Reverse-Proxies from working as it will always append the port 8800 to the host name. my.domain.com is not considered the same as my.domain.com:8800 so SSL will fail and I believe this counts as cross-domain so the server will refuse connection.

In any case, the code that worked for me was the code you'd expect the source to have compiled to:

const Ge = {
    PROTOCOL: Hn.PROTOCOL || window.location.protocol,
    WS_PROTOCOL: Hn.WS_PROTOCOL || window.location.protocol === "https:" ? "wss:" : "ws:",
    BACKEND: Hn.BACKEND || window.location.hostname,
    PORT: Hn.PORT || window.location.port,
    WS_PORT: Hn.WS_PORT || window.location.port,
    BASEURL: Hn.BASEURL || ""
};
m88youngling commented 1 month ago

Do you think there's any way to make this work on the reverse proxy side? I've been trying this advanced configuration for nginx proxy manager (AI generated because I'm an idiot) but it's not working. My goal was to replace all port numbers in connection requests with the regular URL:

# Rewrite rule to remove the port number from URLs
proxy_redirect ~^(https://spotdl.yourdomain.com):8800(/.*)$ $1$2;

# Set the Host header to the proxy's $host variable without the port number
proxy_set_header Host $host;

# Prevent Nginx from appending the port number in redirects
port_in_redirect off;
jvpernis commented 1 month ago

I have this issue as well, having this app hosted behind a reverse proxy. Weirdly enough, support for dynamic values was added back in 2022 in commit 0d00cc498

 const config = {
-  PROTOCOL: process.env.PROTOCOL || 'http',
-  WS_PROTOCOL: process.env.WS_PROTOCOL || 'ws',
-  BACKEND: process.env.BACKEND || 'localhost',
-  PORT: process.env.PORT || '8800',
-  WS_PORT: process.env.WS_PORT || '8800',
+  PROTOCOL: process.env.PROTOCOL || window.location.protocol,
+  WS_PROTOCOL: process.env.WS_PROTOCOL || 'ws:',
+  BACKEND: process.env.BACKEND || window.location.hostname,
+  PORT: process.env.PORT || window.location.port,
+  WS_PORT: process.env.WS_PORT || window.location.port,
   BASEURL: process.env.BASEURL || '',
 }

When prettifying the current build in the dist directory we can see that it is actually different from either version of this change:

const Ge = {
  PROTOCOL: Hn.PROTOCOL ||
  window.location.protocol,
  WS_PROTOCOL: Hn.WS_PROTOCOL ||
  window.location.protocol === 'https:' ? 'wss:' : 'ws:',
  BACKEND: Hn.BACKEND ||
  window.location.hostname,
  PORT: '8800',
  WS_PORT: '8800',
  BASEURL: Hn.BASEURL ||
  ''
};

I am unable to trace where this hard-coded port is coming from. When I build the project myself I get the output you would expect:

const Fe = {
    PROTOCOL: Bt.PROTOCOL || window.location.protocol,
    WS_PROTOCOL: Bt.WS_PROTOCOL || window.location.protocol === "https:" ? "wss:" : "ws:",
    BACKEND: Bt.BACKEND || window.location.hostname,
    PORT: Bt.PORT || window.location.port,
    WS_PORT: Bt.WS_PORT || window.location.port,
    BASEURL: Bt.BASEURL || ""
};

This defect seems to be only recently introduced with the latest build in 292bdeb3 a few months ago. Perhaps this was not a clean build @phcreery?

As spotdl web always fetches the latest version from GitHub, we cannot actually downgrade to a previous version. My workaround for now is to replace 8800 with 443 in the container in the downloaded file in the following location:

However this manual operation is needed every time the container is recreated.

xnetcat commented 1 week ago

Do you think there's any way to make this work on the reverse proxy side? I've been trying this advanced configuration for nginx proxy manager (AI generated because I'm an idiot) but it's not working. My goal was to replace all port numbers in connection requests with the regular URL:

# Rewrite rule to remove the port number from URLs
proxy_redirect ~^(https://spotdl.yourdomain.com):8800(/.*)$ $1$2;

# Set the Host header to the proxy's $host variable without the port number
proxy_set_header Host $host;

# Prevent Nginx from appending the port number in redirects
port_in_redirect off;

You would have to build a custom version with changed config

xnetcat commented 1 week ago

pushed update to master branch, let me know if it works for you guys :)