From edf25150bd01c3211d84f3fe3ce4cc493f7f1684 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 20 Jan 2026 19:34:18 +0000 Subject: [PATCH] Fix: various external client issues (#505) --- Dockerfile | 2 +- .../prowlarr/clients/deluge.py | 14 ++- .../prowlarr/clients/sabnzbd.py | 116 ++++++++++++++++-- .../prowlarr/clients/torrent_utils.py | 61 +++++++++ .../release_sources/prowlarr/settings.py | 13 +- shelfmark/release_sources/prowlarr/utils.py | 39 +++++- tests/prowlarr/test_sabnzbd_client.py | 97 ++++++++++----- tests/prowlarr/test_source.py | 25 +++- 8 files changed, 320 insertions(+), 47 deletions(-) diff --git a/Dockerfile b/Dockerfile index 58294c87..063b3de7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -152,7 +152,7 @@ RUN --mount=type=cache,target=/root/.cache/pip \ # Grant read/execute permissions to others RUN chmod -R o+rx /usr/bin/chromium && \ chmod -R o+rx /usr/bin/chromedriver && \ - chmod -R o+w /usr/local/lib/python3.10/site-packages/seleniumbase/drivers/ + chmod -R o+rwx /usr/local/lib/python3.10/site-packages/seleniumbase/drivers/ # Default command to run the application entrypoint script CMD ["/app/entrypoint.sh"] diff --git a/shelfmark/release_sources/prowlarr/clients/deluge.py b/shelfmark/release_sources/prowlarr/clients/deluge.py index a20d5693..ccd68b65 100644 --- a/shelfmark/release_sources/prowlarr/clients/deluge.py +++ b/shelfmark/release_sources/prowlarr/clients/deluge.py @@ -169,6 +169,18 @@ class DelugeClient(DownloadClient): self._connected = True + def _get_daemon_version(self) -> Any: + """Fetch daemon version, preferring daemon.get_version when available.""" + try: + methods = self._rpc_call("system.listMethods") + if isinstance(methods, list) and "daemon.get_version" in methods: + return self._rpc_call("daemon.get_version") + except Exception: + # Fall back to daemon.info to preserve existing behavior. + pass + + return self._rpc_call("daemon.info") + def _try_set_label(self, torrent_id: str, label: str) -> None: """Best-effort label assignment (requires Deluge Label plugin).""" if not label: @@ -195,7 +207,7 @@ class DelugeClient(DownloadClient): def test_connection(self) -> Tuple[bool, str]: try: self._ensure_connected() - version = self._rpc_call("daemon.info") + version = self._get_daemon_version() return True, f"Connected to Deluge {version}" except Exception as e: self._authenticated = False diff --git a/shelfmark/release_sources/prowlarr/clients/sabnzbd.py b/shelfmark/release_sources/prowlarr/clients/sabnzbd.py index 969f44c5..32748545 100644 --- a/shelfmark/release_sources/prowlarr/clients/sabnzbd.py +++ b/shelfmark/release_sources/prowlarr/clients/sabnzbd.py @@ -5,6 +5,7 @@ Uses SABnzbd's REST API directly via requests (no external dependency). """ from typing import Any, Optional, Tuple +from urllib.parse import urlparse import requests @@ -159,6 +160,100 @@ class SABnzbdClient(DownloadClient): return result + def _api_post_file(self, nzb_content: bytes, filename: str, nzb_name: str, category: str) -> Any: + """ + Upload an NZB file to SABnzbd using addfile. + + Returns: + JSON response from SABnzbd. + """ + api_url = f"{self.url}/api" + request_params = { + "apikey": self.api_key, + "mode": "addfile", + "output": "json", + "nzbname": nzb_name, + "cat": category, + } + files = {"name": (filename, nzb_content, "application/x-nzb")} + + response = requests.post(api_url, params=request_params, files=files, timeout=30) + response.raise_for_status() + result = response.json() + + if isinstance(result, dict) and result.get("status") is False: + error = result.get("error", "Unknown error") + raise Exception(f"SABnzbd error: {error}") + + return result + + def _fetch_nzb_content(self, url: str) -> bytes: + """Fetch NZB content, including Prowlarr auth headers when appropriate.""" + headers = self._get_prowlarr_headers(url) + response = requests.get(url, timeout=30, headers=headers) + response.raise_for_status() + return response.content + + def _get_prowlarr_headers(self, url: str) -> dict: + api_key = str(config.get("PROWLARR_API_KEY", "") or "").strip() + if not api_key: + return {} + + prowlarr_url = normalize_http_url(config.get("PROWLARR_URL", "")) + if not prowlarr_url: + return {} + + try: + target = urlparse(url) + base = urlparse(prowlarr_url) + except ValueError: + return {} + + if target.hostname and base.hostname and target.hostname.lower() == base.hostname.lower(): + return {"X-Api-Key": api_key} + + return {} + + @staticmethod + def _build_nzb_filename(name: str, url: str) -> str: + base_name = (name or "download").strip() or "download" + parsed = urlparse(url) + path = parsed.path or "" + lower_path = path.lower() + + if lower_path.endswith(".nzb.gz"): + suffix = ".nzb.gz" + elif lower_path.endswith(".nzb"): + suffix = ".nzb" + else: + suffix = "" + + lower_base = base_name.lower() + if suffix and lower_base.endswith(suffix): + return base_name + if suffix == ".nzb.gz" and lower_base.endswith(".nzb"): + return f"{base_name}.gz" + if suffix: + return f"{base_name}{suffix}" + if lower_base.endswith((".nzb", ".nzb.gz")): + return base_name + return f"{base_name}.nzb" + + @staticmethod + def _extract_nzo_id(result: Any) -> str: + if not isinstance(result, dict): + raise Exception("SABnzbd returned invalid response") + + nzo_ids = result.get("nzo_ids") or result.get("nzo_id") + if isinstance(nzo_ids, list) and nzo_ids: + return str(nzo_ids[0]) + if isinstance(nzo_ids, str) and nzo_ids: + return nzo_ids + if isinstance(nzo_ids, int): + return str(nzo_ids) + + raise Exception("SABnzbd returned no nzo_id") + def test_connection(self) -> Tuple[bool, str]: """Test connection to SABnzbd.""" try: @@ -200,7 +295,16 @@ class SABnzbdClient(DownloadClient): try: logger.debug(f"Adding NZB to SABnzbd: {name}") + nzb_filename = self._build_nzb_filename(name, url) + nzb_content = self._fetch_nzb_content(url) + result = self._api_post_file(nzb_content, nzb_filename, name, category) + nzo_id = self._extract_nzo_id(result) + logger.info(f"Added NZB to SABnzbd: {nzo_id}") + return nzo_id + except Exception as e: + logger.warning(f"SABnzbd addfile failed, falling back to addurl: {e}") + try: result = self._api_call( "addurl", { @@ -209,15 +313,9 @@ class SABnzbdClient(DownloadClient): "cat": category, }, ) - - # SABnzbd returns {"status": True, "nzo_ids": ["SABnzbd_nzo_xxx"]} - nzo_ids = result.get("nzo_ids", []) - if nzo_ids: - nzo_id = nzo_ids[0] - logger.info(f"Added NZB to SABnzbd: {nzo_id}") - return nzo_id - - raise Exception("SABnzbd returned no nzo_id") + nzo_id = self._extract_nzo_id(result) + logger.info(f"Added NZB to SABnzbd via addurl: {nzo_id}") + return nzo_id except Exception as e: logger.error(f"SABnzbd add failed: {e}") raise diff --git a/shelfmark/release_sources/prowlarr/clients/torrent_utils.py b/shelfmark/release_sources/prowlarr/clients/torrent_utils.py index ce2714f9..4a87047c 100644 --- a/shelfmark/release_sources/prowlarr/clients/torrent_utils.py +++ b/shelfmark/release_sources/prowlarr/clients/torrent_utils.py @@ -14,6 +14,50 @@ from shelfmark.core.logger import setup_logger logger = setup_logger(__name__) +_PROWLARR_DOWNLOAD_PATH = re.compile(r"(?:/api/v1/indexer)?/\d+/download$") + + +def _decode_prowlarr_link(link_value: str) -> Optional[str]: + """Decode Prowlarr's link param into a usable URL, if possible.""" + if not link_value: + return None + + value = link_value.strip() + if not value: + return None + + if value.startswith(("http://", "https://", "magnet:")): + return value + + # Try urlsafe + standard base64 decoding with padding. + padded = value + "=" * (-len(value) % 4) + for decoder in (base64.urlsafe_b64decode, base64.b64decode): + try: + decoded = decoder(padded).decode("utf-8", errors="ignore").strip() + except Exception: + continue + if decoded.startswith(("http://", "https://", "magnet:")): + return decoded + + return None + + +def _get_prowlarr_fallback_url(url: str) -> Optional[str]: + """Try to extract the original download URL from a Prowlarr download proxy URL.""" + try: + parsed = urlparse(url) + if not _PROWLARR_DOWNLOAD_PATH.search(parsed.path): + return None + + params = parse_qs(parsed.query) + link_value = (params.get("link") or [None])[0] + if not link_value: + return None + + return _decode_prowlarr_link(link_value) + except Exception: + return None + @dataclass class TorrentInfo: @@ -47,6 +91,7 @@ def extract_torrent_info( url: str, fetch_torrent: bool = True, expected_hash: Optional[str] = None, + allow_prowlarr_fallback: bool = True, ) -> TorrentInfo: """Extract info_hash from magnet link or .torrent URL. @@ -58,6 +103,14 @@ def extract_torrent_info( This mirrors how Sonarr builds an authenticated download request via the indexer when grabbing torrent files. """ + fallback_url: Optional[str] = None + if allow_prowlarr_fallback: + decoded_url = _get_prowlarr_fallback_url(url) + if decoded_url and decoded_url != url: + logger.debug(f"Decoded Prowlarr link, using direct URL: {decoded_url[:80]}...") + fallback_url = url + url = decoded_url + is_magnet = url.startswith("magnet:") # Try to extract hash from magnet URL @@ -134,6 +187,14 @@ def extract_torrent_info( return TorrentInfo(info_hash=info_hash, torrent_data=torrent_data, is_magnet=False) except Exception as e: logger.debug(f"Could not fetch torrent file: {e}") + if allow_prowlarr_fallback and fallback_url: + logger.debug(f"Retrying torrent fetch via Prowlarr proxy: {fallback_url[:80]}...") + return extract_torrent_info( + fallback_url, + fetch_torrent=fetch_torrent, + expected_hash=expected_hash, + allow_prowlarr_fallback=False, + ) return TorrentInfo(info_hash=None, torrent_data=None, is_magnet=False) diff --git a/shelfmark/release_sources/prowlarr/settings.py b/shelfmark/release_sources/prowlarr/settings.py index f1c512c7..89cf696d 100644 --- a/shelfmark/release_sources/prowlarr/settings.py +++ b/shelfmark/release_sources/prowlarr/settings.py @@ -238,6 +238,17 @@ def _test_deluge_connection(current_values: Optional[Dict[str, Any]] = None) -> raise Exception(str(error)) return data.get("result") + def get_daemon_version(session: requests.Session, rpc_id: int) -> Any: + try: + methods = rpc_call(session, rpc_id, "system.listMethods") + if isinstance(methods, list) and "daemon.get_version" in methods: + return rpc_call(session, rpc_id + 1, "daemon.get_version") + except Exception: + # Fall back to daemon.info to preserve existing behavior. + pass + + return rpc_call(session, rpc_id + 1, "daemon.info") + try: session = requests.Session() @@ -266,7 +277,7 @@ def _test_deluge_connection(current_values: Optional[Dict[str, Any]] = None) -> "message": "Deluge Web UI couldn't connect to Deluge core. Check Deluge Web UI → Connection Manager.", } - version = rpc_call(session, 6, "daemon.info") + version = get_daemon_version(session, 6) return {"success": True, "message": f"Connected to Deluge {version}"} except requests.exceptions.ConnectionError: diff --git a/shelfmark/release_sources/prowlarr/utils.py b/shelfmark/release_sources/prowlarr/utils.py index 7098ef29..2eb4f32e 100644 --- a/shelfmark/release_sources/prowlarr/utils.py +++ b/shelfmark/release_sources/prowlarr/utils.py @@ -6,6 +6,7 @@ Provides common helper functions used across the Prowlarr plugin. from pathlib import Path from typing import Optional +from urllib.parse import parse_qsl, urlencode, urlparse, urlunparse def get_protocol(result: dict) -> str: @@ -40,7 +41,7 @@ def get_preferred_download_url(result: dict) -> str: """ protocol = str(result.get("protocol", "")).lower() magnet_url = str(result.get("magnetUrl") or "").strip() - download_url = str(result.get("downloadUrl") or "").strip() + download_url = sanitize_download_url(str(result.get("downloadUrl") or "").strip()) if protocol == "torrent": return magnet_url or download_url @@ -54,6 +55,42 @@ def get_preferred_download_url(result: dict) -> str: return download_url or magnet_url +def sanitize_download_url(download_url: str) -> str: + """Normalize Prowlarr download URLs to avoid malformed query strings.""" + if not download_url: + return download_url + + normalized = download_url.strip() + if not normalized: + return normalized + + lower = normalized.lower() + if not (lower.startswith("http://") or lower.startswith("https://")): + return normalized + + if " " not in normalized: + return normalized + + parsed = urlparse(normalized) + if not parsed.query: + return normalized + + cleaned_pairs = [] + changed = False + for key, value in parse_qsl(parsed.query, keep_blank_values=True): + cleaned_key = key.strip() + cleaned_value = value.strip() + if cleaned_key != key or cleaned_value != value: + changed = True + cleaned_pairs.append((cleaned_key, cleaned_value)) + + if not changed: + return normalized + + cleaned_query = urlencode(cleaned_pairs, doseq=True) + return urlunparse(parsed._replace(query=cleaned_query)) + + def get_protocol_display(result: dict) -> str: """ Get a user-friendly display label for the protocol. diff --git a/tests/prowlarr/test_sabnzbd_client.py b/tests/prowlarr/test_sabnzbd_client.py index 2a6a6966..1ca504a6 100644 --- a/tests/prowlarr/test_sabnzbd_client.py +++ b/tests/prowlarr/test_sabnzbd_client.py @@ -483,27 +483,23 @@ class TestSABnzbdClientAddDownload: lambda key, default="": config_values.get(key, default), ) - mock_response = MagicMock() - mock_response.json.return_value = { - "status": True, - "nzo_ids": ["SABnzbd_nzo_xyz789"], - } + from shelfmark.release_sources.prowlarr.clients.sabnzbd import ( + SABnzbdClient, + ) - with patch( - "shelfmark.release_sources.prowlarr.clients.sabnzbd.requests.get", - return_value=mock_response, - ): - from shelfmark.release_sources.prowlarr.clients.sabnzbd import ( + with patch.object(SABnzbdClient, "_fetch_nzb_content", return_value=b"nzbdata"): + with patch.object( SABnzbdClient, - ) + "_api_post_file", + return_value={"status": True, "nzo_ids": ["SABnzbd_nzo_xyz789"]}, + ): + client = SABnzbdClient() + result = client.add_download( + "https://example.com/download.nzb", + "Test Book", + ) - client = SABnzbdClient() - result = client.add_download( - "https://example.com/download.nzb", - "Test Book", - ) - - assert result == "SABnzbd_nzo_xyz789" + assert result == "SABnzbd_nzo_xyz789" def test_add_download_no_nzo_id(self, monkeypatch): """Test add_download when SABnzbd returns no nzo_id.""" @@ -517,25 +513,60 @@ class TestSABnzbdClientAddDownload: lambda key, default="": config_values.get(key, default), ) - mock_response = MagicMock() - mock_response.json.return_value = { - "status": True, - "nzo_ids": [], - } + from shelfmark.release_sources.prowlarr.clients.sabnzbd import ( + SABnzbdClient, + ) - with patch( - "shelfmark.release_sources.prowlarr.clients.sabnzbd.requests.get", - return_value=mock_response, - ): - from shelfmark.release_sources.prowlarr.clients.sabnzbd import ( + with patch.object(SABnzbdClient, "_fetch_nzb_content", return_value=b"nzbdata"): + with patch.object( SABnzbdClient, - ) + "_api_post_file", + return_value={"status": True, "nzo_ids": []}, + ): + with patch.object( + SABnzbdClient, + "_api_call", + return_value={"status": True, "nzo_ids": []}, + ): + client = SABnzbdClient() + with pytest.raises(Exception) as exc_info: + client.add_download("https://example.com/download.nzb", "Test") - client = SABnzbdClient() - with pytest.raises(Exception) as exc_info: - client.add_download("https://example.com/download.nzb", "Test") + assert "nzo_id" in str(exc_info.value).lower() - assert "nzo_id" in str(exc_info.value).lower() + def test_add_download_fallback_to_addurl(self, monkeypatch): + """Test fallback to addurl when NZB fetch fails.""" + import requests + + config_values = { + "SABNZBD_URL": "http://localhost:8080", + "SABNZBD_API_KEY": "abc123", + "SABNZBD_CATEGORY": "books", + } + monkeypatch.setattr( + "shelfmark.release_sources.prowlarr.clients.sabnzbd.config.get", + lambda key, default="": config_values.get(key, default), + ) + + from shelfmark.release_sources.prowlarr.clients.sabnzbd import ( + SABnzbdClient, + ) + + with patch.object( + SABnzbdClient, + "_fetch_nzb_content", + side_effect=requests.RequestException("Fetch failed"), + ): + with patch.object( + SABnzbdClient, + "_api_call", + return_value={"status": True, "nzo_ids": ["SABnzbd_nzo_fallback"]}, + ) as mock_api_call: + client = SABnzbdClient() + result = client.add_download("https://example.com/download.nzb", "Test Book") + + assert result == "SABnzbd_nzo_fallback" + assert mock_api_call.call_args[0][0] == "addurl" class TestSABnzbdClientRemove: diff --git a/tests/prowlarr/test_source.py b/tests/prowlarr/test_source.py index 722d866e..af6d62a6 100644 --- a/tests/prowlarr/test_source.py +++ b/tests/prowlarr/test_source.py @@ -13,7 +13,7 @@ from shelfmark.release_sources.prowlarr.source import ( _extract_format, _extract_language, ) -from shelfmark.release_sources.prowlarr.utils import get_protocol_display +from shelfmark.release_sources.prowlarr.utils import get_protocol_display, sanitize_download_url from shelfmark.metadata_providers import BookMetadata @@ -165,6 +165,29 @@ class TestGetProtocolDisplay: assert get_protocol_display(result) == "nzb" +class TestSanitizeDownloadUrl: + """Tests for the sanitize_download_url helper.""" + + def test_sanitizes_apikey_whitespace(self): + """Strip whitespace around apikey separators.""" + url = "http://prowlarr:9696/5/download?apikey = 12345" + assert sanitize_download_url(url) == "http://prowlarr:9696/5/download?apikey=12345" + + def test_sanitizes_multiple_query_params(self): + """Sanitize all query pairs while keeping params.""" + url = "http://prowlarr:9696/5/download?apikey = 12345&indexer = 7" + assert sanitize_download_url(url) == "http://prowlarr:9696/5/download?apikey=12345&indexer=7" + + def test_leaves_non_http_urls_untouched(self): + """Do not modify magnet or other non-http URLs.""" + url = "magnet:?xt=urn:btih:abc123" + assert sanitize_download_url(url) == url + + def test_leaves_clean_urls_untouched(self): + """Return clean URLs as-is.""" + url = "https://prowlarr:9696/5/download?apikey=12345" + assert sanitize_download_url(url) == url + class TestExtractLanguage: """Tests for the _extract_language function."""