-
Notifications
You must be signed in to change notification settings - Fork 17
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
Incorrect port when BASE_URL contains protocol's default port #155
Comments
Thanks for catching this and the detailed write up! |
* Move code into testable function * Fix port erasure when url uses standard port but it is explicitly specified * Simplify building default url and fix using undefined mainPort variable * Fix port erasure when https protocol is explicitly specified * Refactor localUrl usage as URL instead of string * Implement pathname joining on URL to make handling trailing/leading slashes take place in nodejs lib * Implements tests for URL building/manipulation
@pedorich-n please try |
Thanks for such a quick fix!
But I am getting a 400 HTTP code from MS when trying to authorize Spotify now. I don't know if that's something on MS's side or my reverse proxy or what... The URL I see is In the logs, I can see
But the page still shows "Auth Interaction Required" and no Edit: |
Oops! Missed a string conversion of the base url when passing to the spotify api. Try the newest |
Yes, it works now! |
Released in v0.8.0 |
Describe the bug
I am running the
multi-scrobbler
behind the reverse proxy on the addresshttp://multi-scrobbler.server.lan
and specify the same address inBASE_URL
(also triedhttp://multi-scrobbler.server.lan:80
).Two issues arise:
multi-scrobbler/src/backend/ioc.ts
Lines 74 to 79 in 0dcd049
items.mainPort
in the code mentioned above doesn't exist, so I ended up withhttp://multi-scrobbler.server.lan:undefined
as a base url for redirects. Looks like this commit caused this issue.Expected behavior
Use the default port when specified.
Logs
Versions (please complete the following information):
Additional context
The port gets lost after normalization because the
normalize-url
also usesURL
.I think the code should either assume the port is 80 when not specified, as it seems to be the behavior of
URL
, or at least respect the explicitly specified port even when it's the default.The current workaround for me is to explicitly specify the redirect URLs for sources.
The text was updated successfully, but these errors were encountered: