Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DaemonManager::startHelper never compared the proxy, so after saving a proxy the running daemon kept connecting to Spotify directly (and a cleared proxy left it on the proxy). Daemon now records the proxy url it was started with (_proxyUrl); startHelper restarts an alive daemon whose url differs from the current setting, without the "stream active" deferral used for cosmetic changes, and stops it when the binary cannot take --proxy (the restart is then refused by Daemon::start). The fail-closed refusal in Daemon::start is now logged at ERROR once per blocked period per player instead of on every watchdog pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HTTPSConnect left io_socket_timeout at the 5 s handshake budget, and LMS Async::write_async arms its "Timed out waiting for data" timer from it, so proxied requests gave up after 5 s instead of the request Timeout. Restore the caller's Timeout (30 s fallback) once the handshake is done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tunnel handshake is blocking (up to 5 s), so a blackholed proxy stalled the LMS main loop 5 s per API request. After a timeout or a failed TCP connect, further tunnels to the same proxy host:port now fail immediately for 30 s with "proxy: unreachable (recent failure, retry in Ns)". CONNECT rejections and TLS errors are not cached (the proxy is reachable); a successful handshake clears the entry. Requests carrying a proxyOverride (the settings Test button) bypass the cache. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
parseProxyUrl accepted any host without whitespace, so e.g. http://a<b:3128 was saved, rejected by the binary's Url::parse (exit 2) and the daemon crash-looped. The host must now be hostname characters (underscore allowed, as in Url::parse) with no empty labels, a dotted-quad IPv4 when the last label is numeric, or a valid IPv6 literal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…omment Guard the insecureHTTPS accessor with can() (older LMS releases may lack it). The module comment claimed the target hostname is never resolved locally; LMS Async::open still resolves dotless names with gethostbyname even with skipDNS (no Spotify host is dotless) -- say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TROUBLESHOOTING: daemons restart when the saved proxy value changes (not on every save); document "proxy: cannot connect to ...", "proxy: TLS setup failed" and the new "proxy: unreachable (recent failure ...)"; note that the network proxy is tested with LMS 9.x. CHANGELOG: mention restart-on-change and the fail-fast negative cache. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
waitpid on a SIGKILLed child set $? = 9 in the END block, making t/26 exit non-zero after all tests passed on Linux (Perl 5.36/5.38). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional Network proxy setting (
http://host:port) that routes all SpotOn traffic to Spotify through an HTTP proxy. This covers the Web API, sign-in/token refresh, Web Player/Pathfinder, audio, and Spotify Connect. The main use case is servers in networks or regions where Spotify is not directly reachable.With the field empty, behaviour is unchanged: the plugin builds exactly the same
Slim::Networking::SimpleAsyncHTTPrequests as before, and the binary is started without--proxy.Why LMS's own "Web proxy" setting is not enough
Slim::Networking::Async::HTTP::use_proxyskips the server-widewebproxyfor everyhttpsURL, and LMS has no HTTP CONNECT support. All Spotify endpoints are https, so the existing setting never applies.Even if it did, it is server-wide: every plugin, radio stream and update check would go through the proxy. Often only Spotify needs a different route, while everything else should stay direct. A per-plugin setting covers that case.
The new setting is independent of
webproxy: the plugin neither reads nor changes it.Design
Perl side:
Plugins::SpotOn::NetPlugins::SpotOn::Net->http(...)replaces every directSlim::Networking::SimpleAsyncHTTP->new(...)in the plugin. A test guards against new direct uses.SimpleAsyncHTTP.Async::HTTPsubclass does the following for https:CONNECT host:443;Net/Socket/HTTPSConnect.pm, modelled on LMS's ownHTTPSSocks);skipDNS, so the proxy resolves Spotify hostnames.localhost,127.0.0.0/8and::1(the daemon, LMS itself) always go direct.proxy:prefix, for exampleproxy: CONNECT rejected: 403 Forbidden.Binary:
librespot-spoton--proxy <url>argument, applied viaSessionConfig.proxyin all session modes (unified daemon, token-login, discover-once, authenticate, get-token). librespot then tunnels the AP connection, dealer websocket and its HTTP client.--checkreports"proxy": true.urlcrate drops the default port, and librespot's proxy socket has no default, so a port-80 proxy cannot work.Fail-closed
proxycapability (for example an older custom binary), the daemon and the credential helpers are not started. A warning is shown in Settings and on the status page.UI
apresolve.spotify.comwithout saving it.Limitations (by design for this PR)
socks5://later.Proxy-Authorization.i.scdn.co) are loaded by browsers, players and the LMS image proxy, not by the plugin, so they go direct. The existing Streaming Mode setting does not change this: it only affects how audio is relayed inside the LAN.Testing
prove t/passes (27 files, 1269 tests) on Perl 5.36 and 5.38 (Linux containers with the same dependencies asperl-tests.yml) and on 5.34 (macOS).t/26_network_proxy.tcovers URL parsing, the bypass rules, the CONNECT handshake against a forked fake proxy (200 on HTTP/1.0 and 1.1, 403/407/502, garbage, closed, silent/timeout), real TLS through the tunnel, redirects staying tunneled, the negative cache, and the guard against direct HTTP calls.t/27_proxy_daemon_restart.tcovers daemon restart on proxy change and the fail-closed path.t/09,t/13,t/16andt/24are extended.cargo test --lockedpasses (15 tests).Notes for the maintainer
Thanks for SpotOn! It's a great)
build-librespot.ymldidn't runcargo test, so the existing PassthroughMixer tests and the new--proxytests never ran in CI. I added a single test step (x86_64 job only, before the build). It only runs on release tags and manual dispatch, not on PRs. Happy to drop it if you'd rather keep CI changes separate.