From bf85c948b4755bbf919944057d38546df9027747 Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Fri, 31 Jul 2026 13:22:18 -0600 Subject: [PATCH] fix(dl): honour --no-proxy-download for attachments --- docs/guide/cli-reference.md | 2 +- docs/guide/downloading.md | 2 +- docs/guide/proxies-and-vpn.md | 4 +- tests/tracks/test_attachment_download.py | 59 +++++++++++++++++++++ unshackle/commands/dl.py | 8 ++- unshackle/core/api/routes.py | 2 +- unshackle/core/tracks/attachment.py | 66 ++++++++++++++---------- 7 files changed, 109 insertions(+), 34 deletions(-) create mode 100644 tests/tracks/test_attachment_download.py diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index ce4d53dd..c5c995af 100644 --- a/docs/guide/cli-reference.md +++ b/docs/guide/cli-reference.md @@ -162,7 +162,7 @@ Keep only certain track types, or skip certain track types. Attachments are alwa |---|---| | `--proxy` | Proxy URI, a 2-letter country code resolved from configured providers, or `provider:region` (e.g. `nordvpn:ca`, `gluetun:us`, `protonvpn:de:berlin`). | | `--no-proxy` | Force-disable all proxy use. | -| `--no-proxy-download` | Bypass the proxy for **segment downloads only** (manifest, licence, and auth stay proxied). | +| `--no-proxy-download` | Bypass the proxy for **all downloads** (manifest, licence, and auth stay proxied). | | `--remote` | Use a remote unshackle server. | | `--server` | Name a remote server from the `remote_services` config. | diff --git a/docs/guide/downloading.md b/docs/guide/downloading.md index 1086e706..6606a9a3 100644 --- a/docs/guide/downloading.md +++ b/docs/guide/downloading.md @@ -457,7 +457,7 @@ unshackle dl --proxy 'http://user:pass@host:8080' EXAMPLE 81234567 Two related flags: - `--no-proxy`: force-disable all proxy use for this run. -- `--no-proxy-download`: bypass the proxy for **segment downloads only**. The manifest, +- `--no-proxy-download`: bypass the proxy for **all downloads**. The manifest, license, and authentication requests still go through the proxy. This is useful when the proxy is only needed to satisfy geo-checks, not to move the bulk of the data. diff --git a/docs/guide/proxies-and-vpn.md b/docs/guide/proxies-and-vpn.md index 443e29e4..d2020503 100644 --- a/docs/guide/proxies-and-vpn.md +++ b/docs/guide/proxies-and-vpn.md @@ -28,7 +28,7 @@ Proxying is controlled at download time by three options on the `dl` command: |---|---| | `--proxy` | The proxy to use. Either an explicit URI, or a query that unshackle resolves against your configured providers. | | `--no-proxy` | Force **all** proxy use off. No providers are initialised and no proxy query is resolved. | -| `--no-proxy-download` | Use the proxy for the manifest, licence, and authentication, but bypass it for the **segment downloads** themselves. | +| `--no-proxy-download` | Use the proxy for the manifest, licence, and authentication, but bypass it for the **downloads** themselves. | ```shell title="Explicit proxy URI" unshackle dl --proxy http://user:pass@1.2.3.4:8080 EXAMPLE 81234567 @@ -52,7 +52,7 @@ unshackle dl --proxy nordvpn:us EXAMPLE 81234567 Segment downloads are usually the largest share of traffic, and a residential-grade VPN proxy is often the slowest link in the chain. `--no-proxy-download` lets you keep the proxy where it matters (passing the geo-check for the manifest and the licence) while -pulling the actual audio and video segments over your normal connection for full speed. +pulling the files themselves over your normal connection at full speed. ```shell title="Proxy the manifest and licence, download segments direct" unshackle dl --proxy gb --no-proxy-download EXAMPLE 10a1234 diff --git a/tests/tracks/test_attachment_download.py b/tests/tracks/test_attachment_download.py new file mode 100644 index 00000000..caab4ea7 --- /dev/null +++ b/tests/tracks/test_attachment_download.py @@ -0,0 +1,59 @@ +"""URL-backed attachments defer their fetch to download(), which honors no_proxy_download.""" + +from __future__ import annotations + +import requests + +import unshackle.core.tracks.track as track_module +from unshackle.core.config import config +from unshackle.core.tracks.attachment import Attachment + +URL = "https://cdn.example.com/images/thumb.jpg" + + +class FakeResponse: + def raise_for_status(self) -> None: + pass + + def iter_content(self, chunk_size: int): + yield b"jpegdata" + + +class FakeSession(requests.Session): + def __init__(self, proxies: dict | None = None) -> None: + super().__init__() + self.proxies = proxies or {} + self.got: list[str] = [] + + def get(self, url, **kwargs): # type: ignore[override] + self.got.append(url) + return FakeResponse() + + +def test_url_attachment_defers_download() -> None: + session = FakeSession() + attachment = Attachment.from_url(URL, name="thumbnail", session=session) + assert attachment.path is None + assert not session.got + assert attachment.mime_type == "image/jpeg" + + +def test_download_without_flag_uses_proxied_session(tmp_path, monkeypatch) -> None: + monkeypatch.setattr(config.directories, "temp", tmp_path) + session = FakeSession(proxies={"https": "http://proxy:8080"}) + attachment = Attachment.from_url(URL, name="thumbnail", session=session) + attachment.download(session) + assert session.got == [URL] + assert attachment.path is not None and attachment.path.read_bytes() == b"jpegdata" + + +def test_download_with_flag_bypasses_proxy(tmp_path, monkeypatch) -> None: + monkeypatch.setattr(config.directories, "temp", tmp_path) + direct = FakeSession() + monkeypatch.setattr(track_module, "direct_session", lambda session: direct) + proxied = FakeSession(proxies={"https": "http://proxy:8080"}) + attachment = Attachment.from_url(URL, name="thumbnail", session=proxied) + attachment.download(proxied, no_proxy_download=True) + assert not proxied.got + assert direct.got == [URL] + assert attachment.path is not None and attachment.path.read_bytes() == b"jpegdata" diff --git a/unshackle/commands/dl.py b/unshackle/commands/dl.py index ed25dd1b..8ef9832e 100644 --- a/unshackle/commands/dl.py +++ b/unshackle/commands/dl.py @@ -678,7 +678,7 @@ class dl: "--no-proxy-download", is_flag=True, default=False, - help="Bypass proxy for segment downloads only. Manifest, license, and auth still use proxy.", + help="Bypass proxy for all downloads. Manifest, license, and auth still use proxy.", ) @click.option("--no-folder", is_flag=True, default=False, help="Disable folder creation for TV Shows.") @click.option( @@ -3028,6 +3028,12 @@ class dl: on_subtitle_skipped=on_subtitle_skipped, ) + for attachment in title.tracks.attachments: + attachment.download( + attachment.session or service.session, + no_proxy_download=no_proxy_download, + ) + if ( len(self.skipped_subtitles) > skipped_before and not title.tracks.videos diff --git a/unshackle/core/api/routes.py b/unshackle/core/api/routes.py index 7605339b..8f49811d 100644 --- a/unshackle/core/api/routes.py +++ b/unshackle/core/api/routes.py @@ -747,7 +747,7 @@ async def download(request: web.Request) -> web.Response: description: Force disable all proxy use (default - false) no_proxy_download: type: boolean - description: Bypass proxy for segment downloads only. Manifest, license, and auth still use proxy (default - false) + description: Bypass proxy for all downloads. Manifest, license, and auth still use proxy (default - false) tag: type: string description: Set the group tag to be used (default - None) diff --git a/unshackle/core/tracks/attachment.py b/unshackle/core/tracks/attachment.py index d9e427dd..b970768c 100644 --- a/unshackle/core/tracks/attachment.py +++ b/unshackle/core/tracks/attachment.py @@ -12,6 +12,9 @@ import requests from unshackle.core.config import config from unshackle.core.constants import DOWNLOAD_LICENCE_ONLY +from unshackle.core.session import RnetSession + +AnySession = Union[requests.Session, RnetSession] class Attachment: @@ -22,13 +25,13 @@ class Attachment: name: Optional[str] = None, mime_type: Optional[str] = None, description: Optional[str] = None, - session: Optional[requests.Session] = None, + session: Optional[AnySession] = None, ): """ Create a new Attachment. If providing a path, the file must already exist. - If providing a URL, the file will be downloaded to the temp directory. + If providing a URL, download() fetches the file during the download phase. Either path or url must be provided. If name is not provided it will use the file name (without extension). @@ -46,12 +49,13 @@ class Attachment: raise ValueError("Either path or url must be provided.") self.url = url + self.session = session + self.file_name: Optional[str] = None if url: if not isinstance(url, str): raise ValueError("The attachment URL must be a string.") - # If a URL is provided, download the file to the temp directory parsed_url = urlparse(url) file_name = os.path.basename(parsed_url.path) or "attachment" @@ -60,30 +64,7 @@ class Attachment: safe_name = re.sub(r'[<>:"/\\|?*]', "", name).replace(" ", "_") file_name = f"{safe_name}{os.path.splitext(file_name)[1]}" - download_path = config.directories.temp / file_name - - # Download the file unless we're in license-only mode - if DOWNLOAD_LICENCE_ONLY.is_set(): - path = None - else: - try: - if session is None: - with requests.Session() as session: - response = session.get(url, stream=True) - response.raise_for_status() - else: - response = session.get(url, stream=True) - response.raise_for_status() - config.directories.temp.mkdir(parents=True, exist_ok=True) - download_path.parent.mkdir(parents=True, exist_ok=True) - - with open(download_path, "wb") as f: - for chunk in response.iter_content(chunk_size=8192): - f.write(chunk) - - path = download_path - except Exception as e: - raise ValueError(f"Failed to download attachment from URL: {e}") + self.file_name = file_name if path is not None and not isinstance(path, (str, Path)): raise ValueError(f"Invalid attachment path type: expected str or Path, got {type(path).__name__}.") @@ -117,6 +98,35 @@ class Attachment: self.mime_type = mime_type self.description = description + def download( + self, + session: Optional[AnySession] = None, + *, + no_proxy_download: bool = False, + ) -> None: + """Download a URL-backed attachment to the temp directory.""" + if self.path is not None or not self.url or DOWNLOAD_LICENCE_ONLY.is_set(): + return + + from unshackle.core.tracks.track import direct_session + + session = session or self.session or requests.Session() + if no_proxy_download and any(session.proxies.values()): + session = direct_session(session) + + download_path = config.directories.temp / (self.file_name or "attachment") + try: + response = session.get(self.url, stream=True) + response.raise_for_status() + download_path.parent.mkdir(parents=True, exist_ok=True) + with open(download_path, "wb") as f: + for chunk in response.iter_content(chunk_size=8192): + f.write(chunk) + except Exception as e: + raise ValueError(f"Failed to download attachment from URL: {e}") + + self.path = download_path + def __repr__(self) -> str: return "{name}({items})".format( name=self.__class__.__name__, items=", ".join([f"{k}={repr(v)}" for k, v in self.__dict__.items()]) @@ -152,7 +162,7 @@ class Attachment: name: Optional[str] = None, mime_type: Optional[str] = None, description: Optional[str] = None, - session: Optional[requests.Session] = None, + session: Optional[AnySession] = None, ) -> "Attachment": """ Create an attachment from a URL.