From 3ffc7a97353a85f637e6922ca9949275fd238973 Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Tue, 20 Oct 2026 19:59:38 -0600 Subject: [PATCH] fix(api): validate quality and bitrate params like dl and pass cdm_only to services Reject bad quality, bitrate/range conflicts and malformed ranges with a 400, convert quality in the worker, and pass cdm_only into ctx.params. --- tests/remote/unit/test_api_quality_param.py | 50 ++++++++++++++++++++ tests/remote/unit/test_handlers_serialize.py | 16 +++++-- unshackle/core/api/download_manager.py | 5 +- unshackle/core/api/handlers.py | 21 +++++--- 4 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 tests/remote/unit/test_api_quality_param.py diff --git a/tests/remote/unit/test_api_quality_param.py b/tests/remote/unit/test_api_quality_param.py new file mode 100644 index 00000000..83736751 --- /dev/null +++ b/tests/remote/unit/test_api_quality_param.py @@ -0,0 +1,50 @@ +"""The REST download path converts ``quality`` and passes ``cdm_only`` like the CLI.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import click +import pytest + +import unshackle.commands.dl as dl_module +from unshackle.core.api import download_manager, handlers +from unshackle.core.api.handlers import validate_download_parameters +from unshackle.core.services import Services + +pytestmark = pytest.mark.unit + + +class _Captured(Exception): + pass + + +@pytest.mark.parametrize("quality", ["1080p", ["1080"], [1080, 720], "1080,720"]) +def test_the_api_accepts_the_quality_forms_the_cli_accepts(quality: Any) -> None: + assert validate_download_parameters({"quality": quality}) is None + + +def test_a_rest_job_converts_quality_and_passes_cdm_only(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + class FakeDl: + cli = click.Command("dl") + + def __init__(self, ctx: click.Context, **kwargs: object) -> None: + self.ctx = ctx + + def result(self, **kwargs: object) -> None: + raise _Captured(self.ctx.params["quality"], kwargs["quality"], self.ctx.params["cdm_only"]) + + class FakeService: + def __init__(self, ctx: click.Context) -> None: + pass + + monkeypatch.setattr(dl_module, "dl", FakeDl) + monkeypatch.setattr(Services, "get_path", staticmethod(lambda s: tmp_path)) + monkeypatch.setattr(Services, "load", staticmethod(lambda s: FakeService)) + monkeypatch.setattr(handlers, "load_full_cdm", lambda *a: None) + + with pytest.raises(_Captured) as exc: + download_manager.perform_download("job-1", "EXAMPLE1", "t1", {"quality": "1080p", "cdm_only": True}) + + assert exc.value.args == ([1080], [1080], True) diff --git a/tests/remote/unit/test_handlers_serialize.py b/tests/remote/unit/test_handlers_serialize.py index 52c986be..9b650e17 100644 --- a/tests/remote/unit/test_handlers_serialize.py +++ b/tests/remote/unit/test_handlers_serialize.py @@ -279,8 +279,12 @@ def test_validate_download_params_accepts_defaults() -> None: ({"sub_format": "doc"}, "Invalid sub_format"), ({"vbitrate": -1}, "vbitrate"), ({"abitrate": "no"}, "abitrate"), - ({"vbitrate_range": "no-dash-but-letters"}, None), - ({"vbitrate_range": "nope"}, "MIN-MAX"), + ({"vbitrate_range": "no-dash-but-letters"}, "MIN-MAX"), + ({"vbitrate": 5000, "vbitrate_range": "1-2"}, "vbitrate and vbitrate_range"), + ({"abitrate": 128, "abitrate_range": "96-256"}, "abitrate and abitrate_range"), + ({"abitrate_range": "1-2-3"}, "MIN-MAX"), + ({"quality": "4k"}, "Invalid quality"), + ({"quality": [0]}, "above 0"), ({"channels": -3}, "channels"), ({"workers": 0}, "workers"), ({"downloads": 0}, "downloads"), @@ -295,7 +299,6 @@ def test_validate_download_params_accepts_defaults() -> None: def test_validate_download_params_errors(data: dict, fragment) -> None: result = validate_download_parameters(data) if fragment is None: - # A dash-containing string is valid syntactically per current rule assert result is None else: assert result is not None @@ -311,8 +314,6 @@ def test_validate_download_params_accepts_valid_values() -> None: "sub_format": "VTT", "vbitrate": 6000, "abitrate": 128, - "vbitrate_range": "6000-7000", - "abitrate_range": "96-192", "channels": 5.1, "workers": 8, "downloads": 2, @@ -323,6 +324,11 @@ def test_validate_download_params_accepts_valid_values() -> None: ) +@pytest.mark.parametrize("vrange", ["6000-7000", "7000-6000", " 6000 - 7000 "]) +def test_validate_download_params_accepts_valid_bitrate_ranges(vrange: str) -> None: + assert validate_download_parameters({"vbitrate_range": vrange, "abitrate_range": "96-192"}) is None + + async def test_search_missing_service_raises_invalid_input() -> None: # MISSING_SERVICE was never an APIErrorCode member, so this raised AttributeError # and routes.py surfaced a 500 instead of the intended 400. diff --git a/unshackle/core/api/download_manager.py b/unshackle/core/api/download_manager.py index bf1a159b..875d3f36 100644 --- a/unshackle/core/api/download_manager.py +++ b/unshackle/core/api/download_manager.py @@ -18,7 +18,7 @@ from typing import Any, Callable, Dict, List, Optional from unshackle.core.api.events import bus, publish_service_event from unshackle.core.api.sanitize import sanitize_log -from unshackle.core.utils.click_types import AUDIO_CODEC_LIST, VIDEO_CODEC_LIST +from unshackle.core.utils.click_types import AUDIO_CODEC_LIST, QUALITY_LIST, VIDEO_CODEC_LIST from unshackle.core.utils.redact import REDACTED, URL_USERINFO_RE, redact_path, redact_text log = logging.getLogger("download_manager") @@ -409,6 +409,8 @@ def perform_download( if params.get("acodec"): params["acodec"] = AUDIO_CODEC_LIST.convert(params["acodec"]) + params["quality"] = QUALITY_LIST.convert(params["quality"]) if params.get("quality") else [] + range_raw = params.get("range") if range_raw: if isinstance(range_raw, str): @@ -484,6 +486,7 @@ def perform_download( "proxy_download": params.get("proxy_download"), "profile": params.get("profile"), "cdm_name": params.get("cdm"), + "cdm_only": params.get("cdm_only"), "all_drm": params.get("all_drm", False), "repack": params.get("repack", False), "tag": params.get("tag"), diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index 84fea94f..0dc36e3e 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -39,7 +39,7 @@ from unshackle.core.titles import Episode, Movie, Song, Title_T from unshackle.core.titles.music import SONG_FIELDS from unshackle.core.tracks import Audio, Subtitle, Tracks, Video from unshackle.core.utilities import declared_kwargs, kind_only_conflict, original_tag -from unshackle.core.utils.click_types import AUDIO_CODEC_LIST, SUBTITLE_CODEC, VIDEO_CODEC_LIST +from unshackle.core.utils.click_types import AUDIO_CODEC_LIST, QUALITY_LIST, SUBTITLE_CODEC, VIDEO_CODEC_LIST from unshackle.core.utils.collections import ci_get from unshackle.core.utils.hdr10plus import probed_tracks from unshackle.core.utils.redact import REDACTED, URL_USERINFO_RE, redact_all, redact_secrets, redact_text @@ -2218,13 +2218,20 @@ def validate_download_parameters(data: Dict[str, Any]) -> Optional[str]: if not isinstance(data["abitrate"], int) or data["abitrate"] <= 0: return "abitrate must be a positive integer" - if "vbitrate_range" in data and data["vbitrate_range"] is not None: - if not isinstance(data["vbitrate_range"], str) or "-" not in data["vbitrate_range"]: - return "vbitrate_range must be a string in 'MIN-MAX' format (e.g., '6000-7000')" + for prefix, example in (("v", "6000-7000"), ("a", "128-256")): + value = data.get(f"{prefix}bitrate_range") + if value and data.get(f"{prefix}bitrate") is not None: + return f"Cannot use both {prefix}bitrate and {prefix}bitrate_range" + if value is not None and (not isinstance(value, str) or not re.fullmatch(r"\s*\d+\s*-\s*\d+\s*", value)): + return f"{prefix}bitrate_range must be a string in 'MIN-MAX' format (e.g., '{example}')" - if "abitrate_range" in data and data["abitrate_range"] is not None: - if not isinstance(data["abitrate_range"], str) or "-" not in data["abitrate_range"]: - return "abitrate_range must be a string in 'MIN-MAX' format (e.g., '128-256')" + if data.get("quality") is not None: + try: + heights = QUALITY_LIST.convert(data["quality"]) + except click.BadParameter as e: + return f"Invalid quality: {e.message}" + if any(height <= 0 for height in heights): + return "quality heights must be above 0" if "channels" in data and data["channels"] is not None: if not isinstance(data["channels"], (int, float)) or data["channels"] <= 0: