fix(dl): honour --no-proxy-download for attachments

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. |
 

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.
 

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

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"

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

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)

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.