From c0d940b17b25e9f5b6e1af309fb6da8fb01b3817 Mon Sep 17 00:00:00 2001 From: rlaphoenix Date: Fri, 29 Dec 2023 20:25:57 +0000 Subject: [PATCH] Remove Track.needs_proxy Ok, so there's a few reasons this was done. 1) Design-wise it isn't valid to have --proxy (or via config/otherwise) set a proxy, then unpredictably have it bypassed or disabled. If I specify `--proxy 127.0.0.1:8080`, I would expect it to use that proxy for all communication indefinitely, not switch in and out depending on the track or service. 2) With reason 1, it's also a security problem. The only reason I implemented it in the first place was so I could download faster on my home connection. This means I would authenticate and call APIs under a proxy, then suddenly download manifests and segments e.t.c under my home connection. A competent service could see that as an indicator of bad play and flag you. 3) Maintaining this setup across the codebase is extremely annoying, especially because of how proxies are setup/used by Requests in the Session. There's no way to tell a request session to temporarily disable the proxy and turn it back on later, without having to get the proxy from the session (in an annoying way) store it, then remove it, make the calls, then assuming your still in the same function you can add it back. If you're not in the same function, well, time for some spaghetti code. --- tldr; -1 ux/design/expectations with CLI, -1 security aspect, -1 code maintenance, but only +1 for potentially increased download speeds in certain scenarios. --- devine/commands/dl.py | 16 +++------------- devine/core/manifests/dash.py | 3 --- devine/core/manifests/hls.py | 3 --- devine/core/tracks/track.py | 2 -- 4 files changed, 3 insertions(+), 21 deletions(-) diff --git a/devine/commands/dl.py b/devine/commands/dl.py index 92bd6df..a304884 100644 --- a/devine/commands/dl.py +++ b/devine/commands/dl.py @@ -831,10 +831,7 @@ class dl: progress(downloaded="[yellow]SKIPPED") return - if track.needs_proxy: - proxy = next(iter(service.session.proxies.values()), None) - else: - proxy = None + proxy = next(iter(service.session.proxies.values()), None) save_path = config.directories.temp / f"{track.__class__.__name__}_{track.id}.mp4" if isinstance(track, Subtitle): @@ -925,7 +922,7 @@ class dl: out=save_path, headers=service.session.headers, cookies=service.session.cookies, - proxy=proxy if track.needs_proxy else None, + proxy=proxy, progress=progress ) @@ -964,14 +961,7 @@ class dl: if not self.DL_POOL_SKIP.is_set(): if track.path.stat().st_size <= 3: # Empty UTF-8 BOM == 3 bytes - raise IOError( - "Download failed, the downloaded file is empty. " - f"This {'was' if track.needs_proxy else 'was not'} downloaded with a proxy." + - ( - " Perhaps you need to set `needs_proxy` as True to use the proxy for this track." - if not track.needs_proxy else "" - ) - ) + raise IOError("Download failed, the downloaded file is empty.") if callable(track.OnDownloaded): track.OnDownloaded(track) diff --git a/devine/core/manifests/dash.py b/devine/core/manifests/dash.py index 7390695..dd290c9 100644 --- a/devine/core/manifests/dash.py +++ b/devine/core/manifests/dash.py @@ -237,9 +237,6 @@ class DASH: elif not isinstance(session, Session): raise TypeError(f"Expected session to be a {Session}, not {session!r}") - if not track.needs_proxy and proxy: - proxy = None - if proxy: session.proxies.update({ "all": proxy diff --git a/devine/core/manifests/hls.py b/devine/core/manifests/hls.py index b26c8b5..be5f3bb 100644 --- a/devine/core/manifests/hls.py +++ b/devine/core/manifests/hls.py @@ -202,9 +202,6 @@ class HLS: elif not isinstance(session, Session): raise TypeError(f"Expected session to be a {Session}, not {session!r}") - if not track.needs_proxy and proxy: - proxy = None - if proxy: session.proxies.update({ "all": proxy diff --git a/devine/core/tracks/track.py b/devine/core/tracks/track.py index 62f618e..46a9e60 100644 --- a/devine/core/tracks/track.py +++ b/devine/core/tracks/track.py @@ -32,7 +32,6 @@ class Track: language: Union[Language, str], is_original_lang: bool = False, descriptor: Descriptor = Descriptor.URL, - needs_proxy: bool = False, needs_repack: bool = False, drm: Optional[Iterable[DRM_T]] = None, edition: Optional[str] = None, @@ -45,7 +44,6 @@ class Track: self.is_original_lang = bool(is_original_lang) # optional io metadata self.descriptor = descriptor - self.needs_proxy = bool(needs_proxy) self.needs_repack = bool(needs_repack) # drm self.drm = drm