Skip to content

Handle qBittorrent 5.2 cleanup auth#2326

Merged
claytono merged 1 commit into
mainfrom
fix-qbittorrent-cleanup-auth
Jun 4, 2026
Merged

Handle qBittorrent 5.2 cleanup auth#2326
claytono merged 1 commit into
mainfrom
fix-qbittorrent-cleanup-auth

Conversation

@claytono

@claytono claytono commented Jun 4, 2026

Copy link
Copy Markdown
Owner

Accept successful qBittorrent WebAPI login responses with empty bodies. Reuse the first Set-Cookie header like cross-seed's own qBittorrent client so cleanup works with renamed session cookies.

Accept successful qBittorrent WebAPI login responses with empty bodies. Reuse the first Set-Cookie header like cross-seed's own qBittorrent client so cleanup works with renamed session cookies.
Copilot AI review requested due to automatic review settings June 4, 2026 19:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the cross-seed cleanup sidecar’s qBittorrent WebAPI authentication handling to work with qBittorrent 5.2 behavior changes (successful login with empty body) and cookie naming changes.

Changes:

  • Accepts successful login responses when the body is empty (in addition to Ok.).
  • Switches from hard-coding SID= cookie selection to reusing the first Set-Cookie header returned by qBittorrent.
  • Adds a compatibility helper to retrieve Set-Cookie values across different fetch/Headers implementations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread kubernetes/cross-seed/cleanup.js
@coderabbitai

This comment has been minimized.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
kubernetes/cross-seed/cleanup.js (2)

182-188: 💤 Low value

Consider extracting only the cookie name=value portion.

The full Set-Cookie header includes attributes like Path=/; HttpOnly which aren't meant to be sent back in Cookie headers. While qBittorrent likely parses this leniently (and this matches cross-seed's behavior per the comment), sending only the name=value portion would be more HTTP-correct.

If you want to keep strict compatibility with cross-seed's approach, this can remain as-is.

♻️ Optional: Extract just the cookie value
   // Match cross-seed's qBittorrent client: keep the first Set-Cookie
   // header and send it back verbatim as the Cookie header.
   const cookie = getSetCookieHeaders(resp.headers)[0];
   if (!cookie) {
     throw new Error('login succeeded but no cookie returned');
   }
-  qbitCookieHeader = cookie;
+  // Extract just the name=value portion before any attributes
+  qbitCookieHeader = cookie.split(';')[0];
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@kubernetes/cross-seed/cleanup.js` around lines 182 - 188, The code currently
assigns the full Set-Cookie header string (from
getSetCookieHeaders(resp.headers)[0]) to qbitCookieHeader, which can include
attributes like Path and HttpOnly that should not be sent in a Cookie header;
change the logic where cookie is derived so it extracts only the name=value
portion (split on the first ';' or parse until the first semicolon) and assign
that trimmed name=value string to qbitCookieHeader (referencing
getSetCookieHeaders, the local variable cookie, and qbitCookieHeader to locate
and update the code).

20-21: 💤 Low value

Documentation mentions "SID cookie" but implementation is now cookie-agnostic.

The comment references "SID cookie" but the refactored code now uses the first Set-Cookie header regardless of name (to handle qBit 5.2's renamed cookies). Consider updating the comment to reflect the new behavior.

📝 Suggested update
-*   cross-seed PVC at /config so it can require() config.js and unlink
-*   markers under /config/cross-seeds/. qBit session opened once at
-*   startup, SID cookie reused across iterations, re-login on 401/403.
+*   cross-seed PVC at /config so it can require() config.js and unlink
+*   markers under /config/cross-seeds/. qBit session opened once at
+*   startup, session cookie reused across iterations, re-login on 401/403.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@kubernetes/cross-seed/cleanup.js` around lines 20 - 21, Update the inline
comment that currently mentions "SID cookie" to reflect the cookie-agnostic
behavior used by the implementation: replace the phrase "SID cookie reused
across iterations" with wording that explains the code now takes the first
Set-Cookie header (regardless of cookie name) to support qBit 5.2's renamed
cookies, e.g. "first Set-Cookie header reused across iterations (cookie-agnostic
to support qBit 5.2)"; ensure this change is made in the comment block
containing the string "SID cookie" in cleanup.js so the documentation matches
the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@kubernetes/cross-seed/cleanup.js`:
- Around line 182-188: The code currently assigns the full Set-Cookie header
string (from getSetCookieHeaders(resp.headers)[0]) to qbitCookieHeader, which
can include attributes like Path and HttpOnly that should not be sent in a
Cookie header; change the logic where cookie is derived so it extracts only the
name=value portion (split on the first ';' or parse until the first semicolon)
and assign that trimmed name=value string to qbitCookieHeader (referencing
getSetCookieHeaders, the local variable cookie, and qbitCookieHeader to locate
and update the code).
- Around line 20-21: Update the inline comment that currently mentions "SID
cookie" to reflect the cookie-agnostic behavior used by the implementation:
replace the phrase "SID cookie reused across iterations" with wording that
explains the code now takes the first Set-Cookie header (regardless of cookie
name) to support qBit 5.2's renamed cookies, e.g. "first Set-Cookie header
reused across iterations (cookie-agnostic to support qBit 5.2)"; ensure this
change is made in the comment block containing the string "SID cookie" in
cleanup.js so the documentation matches the implementation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f34afcb3-4c0c-45b2-9ae8-b1762a17d385

📥 Commits

Reviewing files that changed from the base of the PR and between 4e0d493 and 03e05cb.

📒 Files selected for processing (1)
  • kubernetes/cross-seed/cleanup.js

@claytono claytono merged commit ee00733 into main Jun 4, 2026
21 checks passed
@claytono claytono deleted the fix-qbittorrent-cleanup-auth branch June 4, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants