fix(downloader): fix qBittorrent SSL connection and rename verification (#923)

- Decouple HTTPS scheme selection from TLS certificate verification:
  `verify=False` always, since self-signed certs are the norm for
  home-server/NAS/Docker qBittorrent setups
- Bump connect timeout from 3.1s to 5.0s for slow TLS handshakes
- Add actionable error messages when HTTPS connection fails
- Fix `continue` → `break` bug in torrents_rename_file verification loop
- Consolidate json imports to top-level
- Add 31 unit tests for QbDownloader constructor, auth, and error handling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Estrella Pan
2026-02-24 19:52:50 +01:00
parent fe1858f0e6
commit ded24b10da
5 changed files with 491 additions and 24 deletions

View File

@@ -13,6 +13,11 @@
- 新增番剧放送日手动设置 API (`PATCH /api/v1/bangumi/{id}/weekday`),支持锁定放送日防止日历刷新覆盖
- 数据库迁移 v9`bangumi` 表新增 `weekday_locked`
### Fixed
- 修复 qBittorrent 下载器 SSL 连接问题:解耦 HTTPS 协议选择与证书验证,自签名证书不再导致连接失败 (#923)
- 修复 `torrents_rename_file` 重命名验证循环中 `continue` 应为 `break` 的逻辑错误
### Changed
- 重构认证模块:提取 `_issue_token` 公共方法,消除 3 处重复的 JWT 签发逻辑

View File

@@ -1,4 +1,5 @@
import asyncio
import json
import logging
import httpx
@@ -25,8 +26,11 @@ class QbDownloader:
async def auth(self, retry=3):
times = 0
timeout = httpx.Timeout(connect=3.1, read=10.0, write=10.0, pool=10.0)
self._client = httpx.AsyncClient(timeout=timeout, verify=self.ssl)
use_https = self.host.startswith("https://")
timeout = httpx.Timeout(connect=5.0, read=10.0, write=10.0, pool=10.0)
# Never verify certificates - self-signed certs are the norm for
# home-server / NAS / Docker qBittorrent setups.
self._client = httpx.AsyncClient(timeout=timeout, verify=False)
while times < retry:
try:
resp = await self._client.post(
@@ -45,13 +49,26 @@ class QbDownloader:
)
await asyncio.sleep(5)
times += 1
except httpx.ConnectError:
logger.error("Cannot connect to qBittorrent Server")
except httpx.ConnectError as e:
if use_https:
logger.error(
"Cannot connect to qBittorrent Server via HTTPS. "
"If your qBittorrent uses plain HTTP, disable SSL in download settings."
)
else:
logger.error("Cannot connect to qBittorrent Server")
logger.info("Please check the IP and port in WebUI settings")
logger.debug("Connection error detail: %s", e)
await asyncio.sleep(10)
times += 1
except Exception as e:
logger.error(f"Unknown error: {e}")
if use_https and "ssl" in str(e).lower():
logger.error(
"TLS/SSL error connecting to qBittorrent. "
"If your qBittorrent uses plain HTTP, disable SSL in download settings."
)
else:
logger.error(f"Unknown error: {e}")
break
return False
@@ -82,7 +99,7 @@ class QbDownloader:
async def prefs_init(self, prefs):
resp = await self._client.post(
self._url("app/setPreferences"),
data={"json": __import__("json").dumps(prefs)},
data={"json": json.dumps(prefs)},
)
return resp
@@ -171,7 +188,6 @@ class QbDownloader:
raise
async def get_torrents_by_tag(self, tag: str) -> list[dict]:
"""Get all torrents with a specific tag."""
resp = await self._client.get(self._url("torrents/info"), params={"tag": tag})
return resp.json()
@@ -221,9 +237,9 @@ class QbDownloader:
if f.get("name") == new_path:
return True
if f.get("name") == old_path:
# File still has old name - try again
# File still has old name - break inner loop and retry
if attempt < 2:
continue
break
# Final attempt failed
logger.debug(
"[Downloader] Rename API returned 200 but file unchanged: %s", old_path
@@ -257,8 +273,6 @@ class QbDownloader:
return resp.json()
async def rss_set_rule(self, rule_name, rule_def):
import json
await self._client.post(
self._url("rss/setRule"),
data={"ruleName": rule_name, "ruleDef": json.dumps(rule_def)},

View File

@@ -306,24 +306,23 @@ class TestIssue990NumberPrefixTitle:
PROBLEM_TITLE = "[ANi] 29 岁单身中坚冒险家的日常 - 07 [1080P][Baha][WEB-DL][AAC AVC][CHT][MP4]"
def test_raw_parser_misparses_leading_number_as_episode(self):
"""raw_parser matches leading '29 ' as episode number, losing the title."""
def test_raw_parser_correctly_parses_leading_number_title(self):
"""raw_parser correctly parses title starting with number and extracts episode."""
result = raw_parser(self.PROBLEM_TITLE)
# The regex matches "29 " as the episode pattern, so episode=29
# and all title fields are None
assert result is not None
assert result.episode == 29
assert result.title_en is None
assert result.title_zh is None
assert result.title_jp is None
assert result.episode == 7
assert result.title_zh == "29 岁单身中坚冒险家的日常"
assert result.resolution == "1080P"
assert result.group == "ANi"
def test_title_parser_returns_none_when_title_raw_empty(self):
"""TitleParser.raw_parser returns None when no title can be extracted."""
def test_title_parser_returns_bangumi_for_number_prefix_title(self):
"""TitleParser.raw_parser returns a valid Bangumi for number-prefixed titles."""
from module.parser.title_parser import TitleParser
result = TitleParser.raw_parser(self.PROBLEM_TITLE)
# Should return None instead of a Bangumi with title_raw=None
assert result is None
assert result is not None
assert result.official_title == "29 岁单身中坚冒险家的日常"
assert result.title_raw == "29 岁单身中坚冒险家的日常"
def test_add_title_alias_rejects_none(self, db_session):
"""add_title_alias should reject None as alias."""

View File

@@ -0,0 +1,449 @@
"""Tests for QbDownloader: constructor, SSL/scheme logic, auth, and error handling.
The implementation keeps _use_https as a local variable computed inside auth()
from self.host, so SSL/HTTPS behaviour is validated by observing auth() side-effects
(log messages) rather than reading an instance attribute directly.
"""
import logging
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
import httpx
from module.downloader.client.qb_downloader import QbDownloader
# ---------------------------------------------------------------------------
# Constructor / URL building
# ---------------------------------------------------------------------------
class TestQbDownloaderConstructor:
"""Verify host URL normalisation at construction time."""
def test_ssl_true_no_scheme_uses_https(self):
"""ssl=True with bare host prepends https://."""
qb = QbDownloader(host="192.168.1.10:8080", username="admin", password="pass", ssl=True)
assert qb.host == "https://192.168.1.10:8080"
def test_ssl_false_no_scheme_uses_http(self):
"""ssl=False with bare host prepends http://."""
qb = QbDownloader(host="192.168.1.10:8080", username="admin", password="pass", ssl=False)
assert qb.host == "http://192.168.1.10:8080"
def test_explicit_http_scheme_preserved_when_ssl_true(self):
"""Explicit http:// scheme is kept even if ssl=True."""
qb = QbDownloader(
host="http://192.168.1.10:8080", username="admin", password="pass", ssl=True
)
assert qb.host == "http://192.168.1.10:8080"
def test_explicit_https_scheme_preserved_when_ssl_false(self):
"""Explicit https:// scheme is kept even if ssl=False."""
qb = QbDownloader(
host="https://192.168.1.10:8080", username="admin", password="pass", ssl=False
)
assert qb.host == "https://192.168.1.10:8080"
def test_explicit_http_scheme_preserved_ssl_false(self):
"""Explicit http:// URL with ssl=False stays as http://."""
qb = QbDownloader(host="http://nas.local:8080", username="u", password="p", ssl=False)
assert qb.host == "http://nas.local:8080"
def test_explicit_https_scheme_preserved_ssl_true(self):
"""Explicit https:// URL with ssl=True stays as https://."""
qb = QbDownloader(host="https://nas.local:8080", username="u", password="p", ssl=True)
assert qb.host == "https://nas.local:8080"
def test_credentials_stored(self):
"""Constructor stores username, password, and ssl flag as-is."""
qb = QbDownloader(
host="localhost:8080", username="admin", password="secret", ssl=False
)
assert qb.username == "admin"
assert qb.password == "secret"
assert qb.ssl is False
def test_client_initially_none(self):
"""_client starts as None before any auth call."""
qb = QbDownloader(host="localhost:8080", username="admin", password="pass", ssl=False)
assert qb._client is None
# ---------------------------------------------------------------------------
# Scheme selection: parametrised matrix
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
"host,ssl,expected_prefix",
[
# Bare host - scheme derived from ssl flag
("192.168.1.1:8080", True, "https://"),
("192.168.1.1:8080", False, "http://"),
("qb.home", True, "https://"),
("qb.home", False, "http://"),
# Explicit http:// always wins regardless of ssl
("http://192.168.1.1:8080", True, "http://"),
("http://192.168.1.1:8080", False, "http://"),
# Explicit https:// always wins regardless of ssl
("https://192.168.1.1:8080", True, "https://"),
("https://192.168.1.1:8080", False, "https://"),
],
)
def test_scheme_selection_matrix(host: str, ssl: bool, expected_prefix: str):
"""Constructor resolves host scheme correctly for all input combinations."""
qb = QbDownloader(host=host, username="u", password="p", ssl=ssl)
assert qb.host.startswith(expected_prefix), (
f"host={host!r} ssl={ssl} -> expected prefix {expected_prefix!r}, got {qb.host!r}"
)
# ---------------------------------------------------------------------------
# auth: AsyncClient is created with verify=False
# ---------------------------------------------------------------------------
class TestAuthClientCreation:
"""auth() must create the httpx.AsyncClient with verify=False unconditionally."""
async def test_auth_creates_client_with_verify_false_when_ssl_true(self):
"""verify=False is used even when ssl=True (self-signed certs are common)."""
qb = QbDownloader(host="192.168.1.10:8080", username="admin", password="pass", ssl=True)
captured: list[dict] = []
class _FakeClient:
def __init__(self, **kwargs):
captured.append(kwargs)
async def post(self, url, data=None):
resp = MagicMock()
resp.status_code = 200
resp.text = "Ok."
return resp
async def aclose(self):
pass
with patch("module.downloader.client.qb_downloader.httpx.AsyncClient", _FakeClient):
result = await qb.auth()
assert result is True
assert len(captured) == 1
assert captured[0].get("verify") is False
async def test_auth_creates_client_with_verify_false_when_ssl_false(self):
"""verify=False is used even when ssl=False."""
qb = QbDownloader(host="192.168.1.10:8080", username="admin", password="pass", ssl=False)
captured: list[dict] = []
class _FakeClient:
def __init__(self, **kwargs):
captured.append(kwargs)
async def post(self, url, data=None):
resp = MagicMock()
resp.status_code = 200
resp.text = "Ok."
return resp
async def aclose(self):
pass
with patch("module.downloader.client.qb_downloader.httpx.AsyncClient", _FakeClient):
result = await qb.auth()
assert result is True
assert captured[0].get("verify") is False
async def test_auth_uses_5_second_connect_timeout(self):
"""auth() sets connect timeout to 5.0 seconds."""
qb = QbDownloader(host="localhost:8080", username="u", password="p", ssl=False)
captured_timeouts: list[httpx.Timeout] = []
class _FakeClient:
def __init__(self, **kwargs):
captured_timeouts.append(kwargs.get("timeout"))
async def post(self, url, data=None):
resp = MagicMock()
resp.status_code = 200
resp.text = "Ok."
return resp
async def aclose(self):
pass
with patch("module.downloader.client.qb_downloader.httpx.AsyncClient", _FakeClient):
await qb.auth()
assert len(captured_timeouts) == 1
assert captured_timeouts[0].connect == pytest.approx(5.0)
# ---------------------------------------------------------------------------
# auth: success / failure paths
# ---------------------------------------------------------------------------
class TestAuthSuccessFailure:
"""auth() return value reflects qBittorrent server responses."""
async def test_auth_returns_true_on_ok_response(self):
"""Returns True when server responds 200 + 'Ok.'."""
qb = QbDownloader(host="localhost:8080", username="admin", password="pass", ssl=False)
mock_client = AsyncMock()
mock_resp = MagicMock()
mock_resp.status_code = 200
mock_resp.text = "Ok."
mock_client.post = AsyncMock(return_value=mock_resp)
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
result = await qb.auth()
assert result is True
async def test_auth_returns_false_on_403(self):
"""Returns False and stops retrying immediately on 403 Forbidden."""
qb = QbDownloader(host="localhost:8080", username="admin", password="pass", ssl=False)
mock_client = AsyncMock()
mock_resp = MagicMock()
mock_resp.status_code = 403
mock_resp.text = "Forbidden"
mock_client.post = AsyncMock(return_value=mock_resp)
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
result = await qb.auth(retry=3)
assert result is False
# Should break immediately on 403, not exhaust all retries
assert mock_client.post.call_count == 1
async def test_auth_retries_up_to_limit_on_server_error(self):
"""Retries up to the retry limit on non-200/non-403 responses."""
qb = QbDownloader(host="localhost:8080", username="admin", password="pass", ssl=False)
mock_client = AsyncMock()
mock_resp = MagicMock()
mock_resp.status_code = 500
mock_resp.text = "Internal Server Error"
mock_client.post = AsyncMock(return_value=mock_resp)
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
with patch(
"module.downloader.client.qb_downloader.asyncio.sleep",
new_callable=AsyncMock,
):
result = await qb.auth(retry=2)
assert result is False
assert mock_client.post.call_count == 2
# ---------------------------------------------------------------------------
# auth: ConnectError logging - HTTPS vs HTTP message branching
# ---------------------------------------------------------------------------
class TestAuthConnectErrorLogging:
"""On ConnectError, the log message depends on whether the URL uses https://."""
async def test_https_url_logs_https_specific_guidance(self, caplog):
"""HTTPS-specific guidance is logged when host uses https:// and ConnectError occurs."""
# Use explicit https:// URL so the local use_https flag is True
qb = QbDownloader(
host="https://192.168.1.10:8080", username="u", password="p", ssl=True
)
mock_client = AsyncMock()
mock_client.post = AsyncMock(side_effect=httpx.ConnectError("Connection refused"))
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
with patch(
"module.downloader.client.qb_downloader.asyncio.sleep",
new_callable=AsyncMock,
):
with caplog.at_level(
logging.ERROR, logger="module.downloader.client.qb_downloader"
):
result = await qb.auth(retry=1)
assert result is False
error_messages = [r.message for r in caplog.records if r.levelno == logging.ERROR]
assert any("HTTPS" in msg for msg in error_messages)
assert any(
"disable SSL" in msg or "plain HTTP" in msg for msg in error_messages
)
async def test_https_url_derived_from_ssl_flag_logs_https_guidance(self, caplog):
"""HTTPS guidance also fires when scheme comes from ssl=True (bare host)."""
# Bare host + ssl=True -> self.host becomes https://... -> use_https=True in auth()
qb = QbDownloader(host="192.168.1.10:8080", username="u", password="p", ssl=True)
assert qb.host.startswith("https://")
mock_client = AsyncMock()
mock_client.post = AsyncMock(side_effect=httpx.ConnectError("refused"))
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
with patch(
"module.downloader.client.qb_downloader.asyncio.sleep",
new_callable=AsyncMock,
):
with caplog.at_level(
logging.ERROR, logger="module.downloader.client.qb_downloader"
):
await qb.auth(retry=1)
error_messages = [r.message for r in caplog.records if r.levelno == logging.ERROR]
assert any("HTTPS" in msg for msg in error_messages)
async def test_http_url_logs_generic_message_without_ssl_hint(self, caplog):
"""Generic connection error is logged when host uses http:// and ConnectError occurs."""
qb = QbDownloader(
host="http://192.168.1.10:8080", username="u", password="p", ssl=False
)
mock_client = AsyncMock()
mock_client.post = AsyncMock(side_effect=httpx.ConnectError("Connection refused"))
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
with patch(
"module.downloader.client.qb_downloader.asyncio.sleep",
new_callable=AsyncMock,
):
with caplog.at_level(
logging.ERROR, logger="module.downloader.client.qb_downloader"
):
result = await qb.auth(retry=1)
assert result is False
error_messages = [r.message for r in caplog.records if r.levelno == logging.ERROR]
assert any("Cannot connect to qBittorrent Server" in msg for msg in error_messages)
# SSL-disable hint must NOT appear for plain HTTP connections
assert not any("disable SSL" in msg for msg in error_messages)
async def test_http_url_derived_from_ssl_flag_false_no_ssl_hint(self, caplog):
"""SSL-disable hint is absent when scheme comes from ssl=False (bare host)."""
qb = QbDownloader(host="192.168.1.10:8080", username="u", password="p", ssl=False)
assert qb.host.startswith("http://")
mock_client = AsyncMock()
mock_client.post = AsyncMock(side_effect=httpx.ConnectError("refused"))
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
with patch(
"module.downloader.client.qb_downloader.asyncio.sleep",
new_callable=AsyncMock,
):
with caplog.at_level(
logging.ERROR, logger="module.downloader.client.qb_downloader"
):
await qb.auth(retry=1)
all_messages = " ".join(r.message for r in caplog.records)
assert "disable SSL" not in all_messages
assert "plain HTTP" not in all_messages
async def test_connect_error_logs_check_ip_port_info(self, caplog):
"""Both HTTPS and HTTP paths log an info message about checking IP/port."""
qb = QbDownloader(
host="https://192.168.1.10:8080", username="u", password="p", ssl=True
)
mock_client = AsyncMock()
mock_client.post = AsyncMock(side_effect=httpx.ConnectError("refused"))
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
with patch(
"module.downloader.client.qb_downloader.asyncio.sleep",
new_callable=AsyncMock,
):
with caplog.at_level(
logging.INFO, logger="module.downloader.client.qb_downloader"
):
await qb.auth(retry=1)
info_messages = [r.message for r in caplog.records if r.levelno == logging.INFO]
assert any("IP" in msg or "port" in msg for msg in info_messages)
async def test_explicit_http_with_ssl_true_still_uses_generic_message(self, caplog):
"""Explicit http:// URL overrides ssl=True: generic error message, no HTTPS hint."""
# ssl=True but explicit http:// -> use_https=False inside auth()
qb = QbDownloader(
host="http://192.168.1.10:8080", username="u", password="p", ssl=True
)
assert qb.host.startswith("http://")
mock_client = AsyncMock()
mock_client.post = AsyncMock(side_effect=httpx.ConnectError("refused"))
with patch(
"module.downloader.client.qb_downloader.httpx.AsyncClient",
return_value=mock_client,
):
with patch(
"module.downloader.client.qb_downloader.asyncio.sleep",
new_callable=AsyncMock,
):
with caplog.at_level(
logging.ERROR, logger="module.downloader.client.qb_downloader"
):
await qb.auth(retry=1)
error_messages = [r.message for r in caplog.records if r.levelno == logging.ERROR]
assert not any("disable SSL" in msg for msg in error_messages)
assert not any("HTTPS" in msg for msg in error_messages)
# ---------------------------------------------------------------------------
# _url helper
# ---------------------------------------------------------------------------
class TestUrlHelper:
"""_url() builds the correct API endpoint path."""
def test_url_format_with_http(self):
"""_url returns host + /api/v2/ + endpoint for http hosts."""
qb = QbDownloader(host="localhost:8080", username="u", password="p", ssl=False)
assert qb._url("auth/login") == "http://localhost:8080/api/v2/auth/login"
def test_url_format_with_https(self):
"""_url includes https:// when SSL is used."""
qb = QbDownloader(host="nas.local:8080", username="u", password="p", ssl=True)
assert qb._url("app/version") == "https://nas.local:8080/api/v2/app/version"
def test_url_with_explicit_http_scheme_overriding_ssl_true(self):
"""_url works correctly when explicit http:// scheme overrides ssl=True."""
qb = QbDownloader(host="http://nas.local:8080", username="u", password="p", ssl=True)
assert qb._url("torrents/info") == "http://nas.local:8080/api/v2/torrents/info"

2
backend/uv.lock generated
View File

@@ -61,7 +61,7 @@ wheels = [
[[package]]
name = "auto-bangumi"
version = "3.2.3"
version = "3.2.4"
source = { virtual = "." }
dependencies = [
{ name = "aiosqlite" },