diff --git a/app/subscriptions.py b/app/subscriptions.py index b845a03..f64e700 100644 --- a/app/subscriptions.py +++ b/app/subscriptions.py @@ -287,6 +287,24 @@ def validate_title_regex(value: Any) -> str: return s +# The name is a display label the user picks; it is persisted and broadcast to +# every connected client, so keep it a bounded single-line string. +SUBSCRIPTION_NAME_MAX_LENGTH = 200 + + +def validate_subscription_name(value: Any) -> str: + """Return a stored subscription name, or raise ValueError if unusable.""" + if not isinstance(value, str): + raise ValueError("name must be a string") + # Collapse newlines/tabs so a pasted title can't break the table layout. + name = " ".join(value.split()) + if not name: + raise ValueError("name must not be empty") + if len(name) > SUBSCRIPTION_NAME_MAX_LENGTH: + raise ValueError(f"name must be at most {SUBSCRIPTION_NAME_MAX_LENGTH} characters") + return name + + def _coerce_bool(value: Any) -> bool: """Accept JSON booleans and common string forms used by API clients.""" if isinstance(value, bool): @@ -674,6 +692,13 @@ class SubscriptionManager: return {"status": "ok"} async def update_subscription(self, sub_id: str, changes: dict) -> dict: + validated_name: Optional[str] = None + if "name" in changes: + try: + validated_name = validate_subscription_name(changes["name"]) + except ValueError as exc: + return {"status": "error", "msg": str(exc)} + validated_tr: Optional[str] = None if "title_regex" in changes: try: @@ -722,8 +747,8 @@ class SubscriptionManager: sub.enabled = validated_enabled if interval_set: sub.check_interval_minutes = validated_interval - if "name" in changes and changes["name"]: - sub.name = str(changes["name"]) + if validated_name is not None: + sub.name = validated_name if validated_tr is not None: sub.title_regex = validated_tr if skip_so_set: diff --git a/app/tests/test_subscriptions.py b/app/tests/test_subscriptions.py index 0f97af1..fbee0db 100644 --- a/app/tests/test_subscriptions.py +++ b/app/tests/test_subscriptions.py @@ -821,6 +821,76 @@ class SubscriptionPersistenceTests(unittest.IsolatedAsyncioTestCase): self.assertEqual(upd["subscription"]["title_regex"], "foo|bar") self.assertEqual(mgr.list_all()[0].title_regex, "foo|bar") + async def _add_one_subscription(self, mgr): + with patch( + "subscriptions.extract_flat_playlist", + return_value=( + {"_type": "channel", "title": "Videos"}, + [{"id": "v1", "title": "One", "webpage_url": "https://example.com/v1"}], + ), + ): + result = await mgr.add_subscription( + "https://example.com/playlist?list=UULFabc", + check_interval_minutes=60, + download_type="video", + codec="auto", + format="any", + quality="best", + folder="", + custom_name_prefix="", + auto_start=True, + playlist_item_limit=0, + split_by_chapters=False, + chapter_template="", + subtitle_language="en", + subtitle_mode="prefer_manual", + ) + return result["subscription"]["id"] + + async def test_update_subscription_renames(self): + """Issue #1044: UULF-style uploads playlists all come back named 'Videos', + so the user needs to be able to relabel them.""" + with tempfile.TemporaryDirectory() as tmp: + mgr = SubscriptionManager(_Config(tmp), _Queue(), _Notifier()) + sub_id = await self._add_one_subscription(mgr) + self.assertEqual(mgr.list_all()[0].name, "Videos") + + upd = await mgr.update_subscription(sub_id, {"name": " Jane's uploads \n"}) + self.assertEqual(upd["status"], "ok") + # Surrounding and interior whitespace is collapsed to keep the name + # a single-line label. + self.assertEqual(upd["subscription"]["name"], "Jane's uploads") + self.assertEqual(mgr.list_all()[0].name, "Jane's uploads") + + async def test_update_subscription_rename_survives_reload(self): + with tempfile.TemporaryDirectory() as tmp: + cfg = _Config(tmp) + mgr = SubscriptionManager(cfg, _Queue(), _Notifier()) + sub_id = await self._add_one_subscription(mgr) + await mgr.update_subscription(sub_id, {"name": "Renamed"}) + + reloaded = SubscriptionManager(cfg, _Queue(), _Notifier()) + self.assertEqual(reloaded.get(sub_id).name, "Renamed") + + async def test_update_subscription_rejects_unusable_name(self): + with tempfile.TemporaryDirectory() as tmp: + mgr = SubscriptionManager(_Config(tmp), _Queue(), _Notifier()) + sub_id = await self._add_one_subscription(mgr) + + for bad in ("", " ", "\n\t", 42, None, ["a"], "x" * 201): + upd = await mgr.update_subscription(sub_id, {"name": bad}) + self.assertEqual(upd["status"], "error", f"expected {bad!r} to be rejected") + self.assertEqual(mgr.list_all()[0].name, "Videos") + + async def test_update_subscription_accepts_name_at_length_limit(self): + with tempfile.TemporaryDirectory() as tmp: + mgr = SubscriptionManager(_Config(tmp), _Queue(), _Notifier()) + sub_id = await self._add_one_subscription(mgr) + + upd = await mgr.update_subscription(sub_id, {"name": "x" * 200}) + self.assertEqual(upd["status"], "ok") + self.assertEqual(mgr.list_all()[0].name, "x" * 200) + async def test_update_subscription_skip_subscriber_only(self): with tempfile.TemporaryDirectory() as tmp: queue = _Queue() diff --git a/ui/src/app/app.html b/ui/src/app/app.html index 5e875f3..c6e436c 100644 --- a/ui/src/app/app.html +++ b/ui/src/app/app.html @@ -958,7 +958,33 @@ [disabled]="downloads.loading" [attr.aria-label]="'Select subscription ' + entry[1].name" /> -