fix(qbittorrent): add URL Base support for reverse-proxied instances

[schmitzkr]

Summary

Adds a "URL Base" field to the qBittorrent download client (closes #690), so Listenarr can reach qBittorrent instances served behind a path-prefixed reverse proxy (e.g. example.com/qbittorrent) — matching the field Transmission's client already has.

I deliberately didn't just copy Transmission's mechanism. Transmission's urlBase replaces its whole RPC path, because that mirrors Transmission's own daemon setting (--rpc-url-base) — Listenarr just matches whatever arbitrary path the user configured server-side. qBittorrent has no equivalent: I checked upstream, and a native WebUI base-path setting has been proposed twice (qbittorrent/qBittorrent#21471, closed unmerged; #23467, still open/unmerged) — neither has shipped. So every real qBittorrent-behind-a-proxy deployment today works by the reverse proxy stripping a path prefix before it reaches qBittorrent, which always serves at its own root regardless. That means the correct semantics here are a prefix prepended before the fixed /api/v2/... routes, not a full-path override.

Changes

Added

• QBittorrentHelpers.BuildBaseUrl() — composes authority + normalized urlBase prefix from client settings; no-op when unset, so existing configs are unaffected
• "URL Base" field now renders for qBittorrent in the download client form, with qBittorrent-specific placeholder/help text (distinct from Transmission's field, since the semantics differ)
• Backend test asserting the actual HTTP request hits /qbittorrent/api/v2/app/version when configured
• Frontend test asserting the field renders for qBittorrent and flows into the save/test payload

Changed

• The 8 qBittorrent workflow files that each independently called DownloadClientUriBuilder.BuildAuthority(client) now go through the new helper instead (mechanical, one line each — no other behavior change)

Testing

• dotnet build / dotnet format --verify-no-changes clean
• Targeted backend tests (qBittorrent + Transmission): 71/71 pass
• Full backend suite: 1185/1190 pass (5 pre-existing failures unrelated to this change — diacritics-normalization tests that only fail in an ICU-less sandbox, not a normal dev setup)
• Frontend: Prettier / vue-tsc clean, full suite 393/393 pass
• Manually tested against a real reverse-proxied qBittorrent instance and confirmed the connection works with the new URL Base field set

Notes

AI assistance disclosure: this PR was developed with Claude Code's assistance - the implementation (the helper, the 8-file sweep, the frontend field, and the tests) was written by Claude Code. My role was directing the approach and validating the result: I pushed back on an initial draft that assumed we should just copy Transmission's urlBase mechanism, which prompted the investigation that found qBittorrent needs different (prefix, not replace) semantics - that upstream research and the resulting design decision are reflected above. I also manually tested this against my own qBittorrent instance behind a real reverse proxy and confirmed it resolves the actual problem reported in #690, which is real, live validation beyond the automated test suite. I have not done a line-by-line manual read of the full diff myself - flagging that plainly since this is my first PR on a project I don't maintain, and I'd rather be upfront about the split between direction/testing and code-writing than imply a level of manual review that didn't happen.

One unrelated thing noticed while auditing every place qBittorrent builds a URL: there are 5 separate, independently-written login implementations across these files (pre-existing, not introduced by this change), and only one of them sets a Referrer header that qBittorrent's CSRF check can care about. It shouldn't interact with urlBase (qBittorrent's CSRF/Referer check is origin-based, not path-based), so I left it alone here, but it seemed worth mentioning in case it's the cause of a future report like "Test Connection works but adding a torrent fails."

[schmitzkr]

I have actually stood up a Bindery instance to be able to have my downloads handled. But if this ever gets merged I think I might come back to Listenarr.

[m4bard]

I came to #690 meaning to implement it, found this, and read it instead. It is the better design and I would rather say why than duplicate it.

Everything below is from checking out the branch and running it, not from reading the diff.

The prefix-versus-replace decision is right, and it is checkable

You reasoned that qBittorrent has no server-side base path, so the prefix must be prepended before the fixed /api/v2/... routes rather than replacing them. qBittorrent's own source agrees, and the specific place is src/webui/webapplication.cpp: isSameOrigin compares port and host only, with the scheme comparison commented out and the path not considered at all. So a prefixed base cannot upset anything qBittorrent checks. That seems worth having in the record, since it is the one part of this that is hard to convince yourself of from the Listenarr side alone.

What I verified

The 8-file sweep is genuinely mechanical. Exactly eight one-line substitutions, and diffing the changed hunks shows no other line altered in those files. Afterwards, the only remaining DownloadClientUriBuilder.BuildAuthority call anywhere in the qBittorrent tree is the one inside BuildBaseUrl itself, which is what you want. Nothing was missed.

TestConnection_WithUrlBase_PrefixesApiPath is a real control, not just a passing test. I made BuildBaseUrl return the bare authority and it fails; restored, it passes. Worth stating because a test that cannot fail looks exactly like one that passes.

Full unfiltered backend suite on your branch: 1190 passed, 0 failed. Your five diacritics failures do not reproduce here, which supports your reading that they are an ICU-less sandbox artefact rather than anything you touched.

It still merges cleanly into current canary, and it does not conflict with #906, which touches QbittorrentAddWorkflow.cs as well.

I think your Referrer note is the right observation pointing the wrong way

You flagged that only one of the login implementations sets a Referrer, and guessed the future report would be "Test Connection works but adding a torrent fails". Reading webapplication.cpp, I think it is the other way round, and the reason is a bit reassuring.

When neither Origin nor Referer is present, qBittorrent falls back to Sec-Fetch-Site, and when that is absent too it returns early with the comment "owasp.org recommends to block this request, but doing so will inevitably lead Web API users to spoof headers so let's be permissive here (Web API clients don't send 'Sec-Fetch-Site' at all)". So the logins that send no Referrer are permitted deliberately rather than by luck.

QbittorrentConnectionTester.cs:95 is the only one that sets Referrer, and setting it opts into the strict comparison the others never reach. If an origin mismatch ever bites, Test Connection is the call that can fail while the torrent add keeps working. I counted six login call sites rather than five, since the connection tester has two.

None of that is a problem with this PR. Your change moves that header from the bare authority to the prefixed one, and since only host and port are compared, it still passes.

One thing I would fix, and one I probably would not

A full URL in the field is accepted and silently mangled. I ran the normalization over a few inputs:

/qbittorrent                              -> http://host:8080/qbittorrent
qbittorrent                               -> http://host:8080/qbittorrent
/qbittorrent/                             -> http://host:8080/qbittorrent
  /qbittorrent                            -> http://host:8080/qbittorrent
https://seedbox.example.com/qbittorrent   -> http://host:8080/https://seedbox.example.com/qbittorrent

The first four are exactly right. The last one worries me because it is the likely paste: #690 describes the setup as https://{seedbox_host}/qbittorrent and your own comment describes yours as https://example.com/qbittorrent/, so someone reading either is being shown a full URL and asked for a path. They get a config that fails with no indication why.

Sonarr, Radarr and Readarr reject anything beginning with a scheme in their ValidUrlBase rule. Would you want the same here? A guard in ResolveUrlBasePrefix, or a warning on the field, either would do. I am asking rather than suggesting a patch because it is your PR and you have the real proxied instance to try it against.

The other one I noticed is that //qbittorrent produces a double slash. That seems genuinely not worth code.

On the disclosure

For what it is worth, the line-by-line read you said you had not done is the thing I have now done, and the code holds up. Flagging the split between direction and code-writing was the right call and it made this easy to review well.

Checked against canary a630572e, and against qBittorrent master for the WebUI behaviour.

[schmitzkr]

Thanks for this — genuinely one of the more thorough reviews I've had on this repo. The isSameOrigin citation, the mutation-test on TestConnection_WithUrlBase_PrefixesApiPath, and catching that the Referrer risk points the opposite direction from what I guessed are all things I hadn't verified myself, and I checked each of them against the branch afterward — all correct as written.

I've pushed a fix for the one open item: a value like https://seedbox.example.com/qbittorrent pasted into URL Base is now rejected instead of being silently concatenated into a broken URL. ResolveUrlBasePrefix now rejects anything that parses as an absolute URI (Uri.TryCreate(trimmed, UriKind.Absolute, ...)) and throws a QbittorrentException telling the user to strip the scheme/host and enter just the path. QbittorrentConnectionTester now catches that specifically so the message actually reaches the UI via Test Connection, instead of getting swallowed into a generic "Connection failed." Added TestConnection_WithFullUrlAsUrlBase_ReturnsClearError alongside the existing urlBase test to cover it.

In the same spirit of disclosure you raised: this fix, and the verification of your review's claims against the branch beforehand, was implemented by Claude Sonnet 5 via Claude Code, at my direction — I reviewed the diff and pushed it myself.

[m4bard]

Thanks for checking those against the branch rather than taking them on trust. That is the part that makes a review worth writing.

The rejection does what you intended for a pasted full URL. It also rejects the value the field is documented to take, but only on Linux, so I would not expect it to have shown up for you.

What I ran

Checked out 82ced00 and ran the suite. TestConnection_WithUrlBase_PrefixesApiPath fails at QbittorrentAdapterTests.cs:119, on Assert.True(success), with urlBase set to /qbittorrent. Full unfiltered suite: 1190 passed, 1 failed. Your new TestConnection_WithFullUrlAsUrlBase_ReturnsClearError passes.

Why

Uri.TryCreate(trimmed, UriKind.Absolute, out _) treats a leading-slash path as an absolute file: URI on Unix. I ran it directly rather than reasoning about it:

Uri.TryCreate("/qbittorrent", Absolute)                       = True   scheme=file
Uri.TryCreate("qbittorrent",  Absolute)                       = False
Uri.TryCreate("https://seedbox.example.com/qbittorrent", ...) = True   scheme=https

So /qbittorrent, which is the placeholder in your own help text, now throws with the message asking the user to remove a scheme and host that are not there. qbittorrent without the leading slash still works, which will make it look intermittent to whoever hits it.

On Windows /qbittorrent does not parse as absolute, which I assume is why the suite looked green when you pushed. Listenarr ships as a container, so the deployed path is the one that fails.

Options, and I do not think it matters much which

Checking the scheme after parsing keeps the shape you have:

if (Uri.TryCreate(trimmed, UriKind.Absolute, out var parsed)
    && (parsed.Scheme == Uri.UriSchemeHttp || parsed.Scheme == Uri.UriSchemeHttps))

A plain trimmed.Contains("://") would also do it and reads more obviously, at the cost of accepting something like httpx://host that nobody is going to type.

Either way it seems worth a case in the test for /qbittorrent surviving, since that is the one the current guard eats.

On the disclosure

Appreciated, and mine is the same: the review, the runs above and this comment were done with Claude Code at my direction. I read the diff and the failure myself before writing this.

[schmitzkr]

Confirmed — you're right, and thanks for actually running it instead of trusting the reasoning (mine or otherwise). Uri.TryCreate(trimmed, UriKind.Absolute, out _) matching a leading-slash path as file: on Unix is not something I'd have caught by inspection.

Pushed a fix: swapped that check for DownloadClientUriBuilder.TryParseHttpOrHttpsAbsoluteUri, which is already used elsewhere in this codebase for exactly this "reject anything that isn't a genuine http/https absolute URI" case, so /qbittorrent no longer gets misread as an absolute URI at all while https://seedbox.example.com/qbittorrent still gets rejected correctly. I went with reusing that helper over either of your two suggested shapes since it keeps the scheme-checking logic in one place rather than duplicating it a third time in this file.

This time I actually ran the suite before pushing rather than reasoning about it from static analysis (which is exactly the gap that let this ship in the first place) — full QbittorrentAdapterTests run, 35/35 passed, including TestConnection_WithUrlBase_PrefixesApiPath and TestConnection_WithFullUrlAsUrlBase_ReturnsClearError both green. Didn't add a further isolated case for /qbittorrent surviving specifically, since TestConnection_WithUrlBase_PrefixesApiPath already asserts exactly that (asserts success with urlBase = "/qbittorrent") — happy to add a narrower one if you think the coverage is too indirect.

One more disclosure note, since we're both doing this openly: my commits now carry a Co-Authored-By: Claude Sonnet 5 trailer going forward, for the same reason as the rest of this — starting with the one above. The earlier one (82ced00) predates the convention; I'm leaving it as-is rather than rewriting history that's already referenced by SHA in this thread.