diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac6f0af..6a54bcd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 签发逻辑 diff --git a/backend/src/module/downloader/client/qb_downloader.py b/backend/src/module/downloader/client/qb_downloader.py index 66f931ff..7f888f6f 100644 --- a/backend/src/module/downloader/client/qb_downloader.py +++ b/backend/src/module/downloader/client/qb_downloader.py @@ -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)}, diff --git a/backend/src/test/test_issue_bugs.py b/backend/src/test/test_issue_bugs.py index a0af3420..f1ed1533 100644 --- a/backend/src/test/test_issue_bugs.py +++ b/backend/src/test/test_issue_bugs.py @@ -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.""" diff --git a/backend/src/test/test_qb_downloader.py b/backend/src/test/test_qb_downloader.py new file mode 100644 index 00000000..1630b4f6 --- /dev/null +++ b/backend/src/test/test_qb_downloader.py @@ -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" diff --git a/backend/uv.lock b/backend/uv.lock index ddea71e3..00473cb5 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -61,7 +61,7 @@ wheels = [ [[package]] name = "auto-bangumi" -version = "3.2.3" +version = "3.2.4" source = { virtual = "." } dependencies = [ { name = "aiosqlite" },