fix(downloader): harden ranged reads, teardown and multiprocess sessions

Validates the 206 range end, bounds the DASH collapse, and preserves cookie domains and pool size in spawned children.

tests/core/test_adaptive_workers.py

@@ -1,7 +1,6 @@
-"""Unit tests for the opt-in adaptive worker controller (slow-start then AIMD hill-climb
-over segment concurrency) and the plumbing that carries the flag into the downloader. Pure
-controller tests only. No threads, sockets, or real downloads; the clock is supplied
-explicitly."""
+"""Unit tests for the opt-in adaptive worker controller (AIMD hill-climb over segment
+concurrency) and the plumbing that carries the flag into the downloader. Pure controller
+tests only. No threads, sockets, or real downloads; the clock is supplied explicitly."""
 
 from __future__ import annotations
 
@@ -55,31 +54,31 @@ def run_tick(c: AdaptiveWorkerController, rate: float, now: float, errors: int =
     return c.update(now), now
 
 
-def test_starts_at_min_six_cap() -> None:
-    assert AdaptiveWorkerController(cap=16).target == 6
-    assert AdaptiveWorkerController(cap=8).target == 6
-    assert AdaptiveWorkerController(cap=4).target == 4  # min(6, cap)
+def test_starts_at_cap() -> None:
+    assert AdaptiveWorkerController(cap=16).target == 16
+    assert AdaptiveWorkerController(cap=4).target == 4
     assert AdaptiveWorkerController(cap=1).target == 2  # clamped up to ADAPTIVE_MIN
+    assert AdaptiveWorkerController(cap=16, start=6).target == 6  # explicit start wins
 
 
 def test_no_op_before_warmup() -> None:
-    c = AdaptiveWorkerController(cap=16, tick=TICK, window=WINDOW)
+    c = AdaptiveWorkerController(cap=16, start=6, tick=TICK, window=WINDOW)
     assert c.update(0.0) == 6  # arms last_tick before any samples are recorded
     c.record_bytes(500_000, 3.0)
     # a full tick has elapsed since arming, but under half a tick of samples exists -> hold
     assert c.update(4.5) == 6
     c.record_bytes(500_000, 6.5)
-    # warmed up (first sample 3.0, now 8.5) -> slow-start probe fires
-    assert c.update(8.5) == 12
+    # warmed up (first sample 3.0, now 8.5) -> the probe fires
+    assert c.update(8.5) == 8
 
 
-def test_ramps_up_while_speed_improves() -> None:
-    c = AdaptiveWorkerController(cap=16, tick=TICK, window=WINDOW)
+def test_climbs_while_speed_improves() -> None:
+    c = AdaptiveWorkerController(cap=16, start=6, tick=TICK, window=WINDOW)
     now = 0.0
     c.update(now)  # arm
     rate = 1_000_000.0
     now = fill_window(c, rate, now)
-    targets = [c.update(now)]  # first evaluation: slow-start doubles 6 -> 12
+    targets = [c.update(now)]  # first evaluation: 6 -> 8
     for _ in range(6):
         rate *= 2  # sustained >10% windowed gain each tick
         target, now = run_tick(c, rate, now)
@@ -89,23 +88,23 @@ def test_ramps_up_while_speed_improves() -> None:
     assert max(targets) <= 16
 
 
-def test_reverts_on_plateau_and_ends_slow_start() -> None:
-    c = AdaptiveWorkerController(cap=16, tick=TICK, window=WINDOW)
+def test_reverts_on_plateau() -> None:
+    c = AdaptiveWorkerController(cap=16, start=6, tick=TICK, window=WINDOW)
     now = 0.0
     c.update(now)
     rate = 1_000_000.0
     now = fill_window(c, rate, now)
-    assert c.update(now) == 12  # first increase slow-starts 6 -> 12
+    assert c.update(now) == 8  # first increase 6 -> 8
     # hold the rate flat: <10% gain -> the tick reverts the increase to its restore point
     target, now = run_tick(c, rate, now)
     assert target == 6
-    # slow-start is over: the next successful probe is additive (+2), not a double
+    # a paying probe climbs again from the restore point
     target, now = run_tick(c, rate * 4, now)
     assert target == 8
 
 
 def test_halves_and_cooldown_on_error_burst() -> None:
-    c = AdaptiveWorkerController(cap=16, tick=TICK, window=WINDOW)
+    c = AdaptiveWorkerController(cap=16, start=6, tick=TICK, window=WINDOW)
     now = 0.0
     c.update(now)
     now = fill_window(c, 1_000_000.0, now)
@@ -150,7 +149,7 @@ def test_error_burst_floors_at_min() -> None:
 
 def test_tail_guard_suppresses_increase() -> None:
     # with fewer units of work than the target, the tick must hold instead of probing up
-    c = AdaptiveWorkerController(cap=16, tick=TICK, window=WINDOW)
+    c = AdaptiveWorkerController(cap=16, start=6, tick=TICK, window=WINDOW)
     now = 0.0
     c.update(now)  # arm at target 6
     now = fill_window(c, 1_000_000.0, now)
@@ -159,16 +158,16 @@ def test_tail_guard_suppresses_increase() -> None:
 
 def test_tail_guard_none_path_unchanged() -> None:
     # without a tail count, the update still probes upward on the first evaluation
-    c = AdaptiveWorkerController(cap=16, tick=TICK, window=WINDOW)
+    c = AdaptiveWorkerController(cap=16, start=6, tick=TICK, window=WINDOW)
     now = 0.0
     c.update(now)
     now = fill_window(c, 1_000_000.0, now)
-    assert c.update(now) == 12  # first evaluation slow-starts 6 -> 12
+    assert c.update(now) == 8  # first evaluation 6 -> 8
 
 
 def test_tail_guard_still_halves_on_error_burst() -> None:
     # the guard suppresses growth but never blocks an error-burst decrease
-    c = AdaptiveWorkerController(cap=16, tick=TICK, window=WINDOW)
+    c = AdaptiveWorkerController(cap=16, start=6, tick=TICK, window=WINDOW)
     now = 0.0
     c.update(now)
     now = fill_window(c, 1_000_000.0, now)

tests/core/test_dash_collapse.py

@@ -48,6 +48,12 @@ def test_first_segment_gap_after_init_ineligible():
     assert collapsible([(URL, "900-1999")], init_len=863) is False
 
 
+def test_gap_between_segments_ineligible():
+    # the merge path would skip 2000-2999; collapsing the parent would include it
+    segments = [(URL, "863-1999"), (URL, "3000-3999")]
+    assert collapsible(segments, init_len=863) is False
+
+
 def test_overlapping_ranges_ineligible():
     segments = [(URL, "863-2500"), (URL, "2000-3999")]
     assert collapsible(segments, init_len=863) is False

tests/core/test_dash_collapse_integration.py

@@ -27,6 +27,8 @@ from unshackle.core.tracks.track import DownloadContext
 # distinctive parent resource for the collapse case; init is its [0, INIT_LEN) prefix
 PARENT = bytes(i % 256 for i in range(10240))
 INIT_LEN = 512
+# last indexed byte for the clipped case; the parent carries 1024 bytes past it
+CLIP_END = len(PARENT) - 1025
 
 # separate resources for the non-collapse (mixed-URL) case
 INIT2 = bytes(range(100))
@@ -111,6 +113,27 @@ def collapse_mpd(srv) -> str:
 </MPD>"""
 
 
+def clipped_mpd(srv) -> str:
+    # same parent, but the ranges stop short of its end -> the collapsed file must be trimmed
+    # to the last indexed byte, not left holding the parent's trailing bytes.
+    return f"""<?xml version="1.0"?>
+<MPD xmlns="urn:mpeg:dash:schema:mpd:2011" type="static" profiles="urn:mpeg:dash:profile:isoff-on-demand:2011">
+  <BaseURL>{base(srv)}</BaseURL>
+  <Period id="0">
+    <AdaptationSet contentType="video" mimeType="video/mp4" lang="en">
+      <Representation id="v0" codecs="avc1.640028" bandwidth="1000000" width="1920" height="1080">
+        <BaseURL>media.mp4</BaseURL>
+        <SegmentList timescale="1">
+          <Initialization range="0-{INIT_LEN - 1}"/>
+          <SegmentURL mediaRange="{INIT_LEN}-5119"/>
+          <SegmentURL mediaRange="5120-{CLIP_END}"/>
+        </SegmentList>
+      </Representation>
+    </AdaptationSet>
+  </Period>
+</MPD>"""
+
+
 def mixed_mpd(srv) -> str:
     # Two SegmentURLs on distinct paths -> mixed URLs -> predicate False -> segmented path.
     return f"""<?xml version="1.0"?>
@@ -162,6 +185,17 @@ def test_collapse_downloads_parent_whole_in_one_direct_request(server, tmp_path)
     assert not list(tmp_path.glob("**/*.!dev"))
 
 
+def test_collapse_trims_parent_bytes_past_the_last_segment(server, tmp_path):
+    server.routes["/media.mp4"] = PARENT
+    track = make_track(clipped_mpd(server), server)
+    ctx = make_ctx(tmp_path, "clipped.mp4")
+
+    DASH.download_track(track, ctx)
+
+    # the parent's trailing 1024 bytes are not part of any segment, so they must not survive
+    assert ctx.save_path.read_bytes() == PARENT[: CLIP_END + 1]
+
+
 def test_non_collapse_takes_segmented_merge_path(server, tmp_path):
     server.routes["/init.bin"] = INIT2
     server.routes["/part0.bin"] = PART0

tests/core/test_retry_after.py

@@ -0,0 +1,69 @@
+"""RnetSession.get_sleep_time must survive hostile or malformed Retry-After headers."""
+
+from datetime import datetime, timedelta, timezone
+from email.utils import format_datetime
+from types import SimpleNamespace
+
+import pytest
+
+from unshackle.core.session import RnetSession
+
+
+@pytest.fixture
+def session():
+    return RnetSession()
+
+
+def response(retry_after=None):
+    headers = {} if retry_after is None else {"Retry-After": retry_after}
+    return SimpleNamespace(headers=headers)
+
+
+def test_numeric_retry_after_passes_through(session):
+    assert session.get_sleep_time(response("3"), 1) == 3.0
+
+
+def test_huge_numeric_retry_after_is_clamped(session):
+    assert session.get_sleep_time(response("86400"), 1) == session.max_backoff
+
+
+def test_huge_numeric_retry_after_respects_session_cap():
+    session = RnetSession(max_backoff=5.0)
+    assert session.get_sleep_time(response("86400"), 1) == 5.0
+
+
+def test_http_date_retry_after_honoured(session):
+    when = format_datetime(datetime.now(timezone.utc) + timedelta(seconds=10), usegmt=True)
+    assert 5 <= session.get_sleep_time(response(when), 1) <= 10
+
+
+def test_naive_http_date_does_not_raise(session):
+    # "-0000" makes parsedate_to_datetime return a naive datetime
+    when = format_datetime(datetime.now(timezone.utc) + timedelta(seconds=10)).replace("+0000", "-0000")
+    assert 5 <= session.get_sleep_time(response(when), 1) <= 10
+
+
+def test_past_http_date_yields_no_sleep(session):
+    when = format_datetime(datetime.now(timezone.utc) - timedelta(seconds=30), usegmt=True)
+    assert session.get_sleep_time(response(when), 1) <= 0
+
+
+def test_malformed_retry_after_falls_back_to_backoff(session):
+    value = session.get_sleep_time(response("not-a-date"), 1)
+    assert 0 < value <= session.max_backoff
+
+
+def test_empty_retry_after_falls_back_to_backoff(session):
+    value = session.get_sleep_time(response(""), 1)
+    assert 0 < value <= session.max_backoff
+
+
+def test_exponential_backoff_without_retry_after(session):
+    assert session.get_sleep_time(response(), 0) == 0.0
+    base = session.backoff_factor * 4
+    assert base * 0.9 <= session.get_sleep_time(response(), 3) <= base * 1.1
+    assert session.get_sleep_time(response(), 30) == session.max_backoff
+
+
+def test_no_response_still_backs_off(session):
+    assert session.get_sleep_time(None, 30) == session.max_backoff

tests/downloaders/test_failure_surfacing.py

@@ -6,7 +6,7 @@ import pytest
 import requests as rq
 
 from unshackle.core.constants import DOWNLOAD_CANCELLED
-from unshackle.core.downloaders.requests import download, has_range_header, parse_content_range, request_range_start
+from unshackle.core.downloaders.requests import download, has_range_header, parse_content_range, request_range
 from unshackle.core.downloaders.requests import requests as requests_downloader
 
 # the package re-exports the `requests` function, shadowing the module of the same name
@@ -76,6 +76,12 @@ class _Server:
                     self.send_206(conn, BODY[5:], f"bytes 5-{len(BODY) - 1}/{len(BODY)}")
                 elif self.mode == "resume_missing_content_range":
                     self.send_206(conn, BODY)
+                elif self.mode == "range_capped_206":
+                    # honors the requested start but caps the span, as size-capping CDNs do
+                    start, _, end = headers["range"].removeprefix("bytes=").partition("-")
+                    start, end = int(start), int(end)
+                    capped = start + (end - start) // 2
+                    self.send_206(conn, BODY[start : capped + 1], f"bytes {start}-{capped}/{len(BODY)}")
                 else:
                     conn.sendall(
                         b"HTTP/1.1 416 Range Not Satisfiable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"
@@ -197,6 +203,23 @@ def test_slice_segment_rejects_a_206_for_the_wrong_range(tmp_path, server):
     assert len(server.requests) == downloader.MAX_ATTEMPTS
 
 
+@pytest.mark.parametrize("server", ["range_capped_206"], indirect=True)
+def test_slice_segment_rejects_a_206_capped_short_of_the_requested_end(tmp_path, server):
+    # the body is a valid prefix of the slice, so only the Content-Range end catches it;
+    # writing it would finalize a truncated segment and corrupt the merge
+    save_path = tmp_path / "0.mp4"
+    with pytest.raises(OSError):
+        list(
+            download(
+                url=server.url,
+                save_path=save_path,
+                session=rq.Session(),
+                headers={"Range": f"bytes=0-{len(BODY) - 1}"},
+            )
+        )
+    assert not save_path.exists()
+
+
 @pytest.mark.parametrize("server", ["range_unsatisfiable"], indirect=True)
 def test_slice_segment_never_gets_a_resume_range(tmp_path, server):
     save_path = tmp_path / "0.mp4"
@@ -295,9 +318,9 @@ def test_parse_content_range(value, expected):
         ({}, None),
         ({"Accept": "*/*"}, None),
         ({"Range": "bytes=-500"}, None),
-        ({"Range": "bytes=100-"}, 100),
-        ({"range": "bytes=100-227"}, 100),
+        ({"Range": "bytes=100-"}, (100, None)),
+        ({"range": "bytes=100-227"}, (100, 227)),
     ],
 )
-def test_request_range_start(headers, expected):
-    assert request_range_start(headers) == expected
+def test_request_range(headers, expected):
+    assert request_range(headers) == expected

tests/downloaders/test_session_spec.py

@@ -0,0 +1,125 @@
+"""Session spec fidelity across the multiprocess spawn boundary.
+
+``build_session_spec``/``rebuild_session`` are how a spawned child gets the parent's session
+state. The cross-process half needs a live CDN to observe, so these tests pin the two halves
+that can be checked deterministically: the spec is picklable (spawn requires it), and a
+rebuilt session carries the same cookie domains as the parent. A spec that flattens cookies
+to name/value rebuilds every cookie scoped to localhost, so a cookie-authed CDN sees none of
+them and 403s only when ``--download-processes`` is above 1.
+"""
+
+import pickle
+
+from requests import Session
+
+from unshackle.core.downloaders.requests import build_session_spec, rebuild_session
+from unshackle.core.session import RnetSession
+from unshackle.core.session import session as make_session
+
+
+def rnet_session_with_cookies() -> RnetSession:
+    """RnetSession carrying two domain-scoped cookies plus one set from a plain dict.
+
+    The dict-set cookie lands in the adapter's flat map only, never in its domain map, so it
+    is the case a domain-only serialization would silently drop.
+    """
+    ns = make_session("Chrome131")
+    ns.cookies.set("cdn_token", "abc", domain="cdn.example.com")
+    ns.cookies.set("api_token", "def", domain="api.example.com")
+    ns.cookies.update({"loose": "ghi"})
+    return ns
+
+
+def test_rnet_spec_round_trips_cookie_domains() -> None:
+    ns = rnet_session_with_cookies()
+
+    spec = build_session_spec(ns)
+    assert spec is not None
+    rebuilt = rebuild_session(pickle.loads(pickle.dumps(spec)), 1)
+    assert rebuilt is not None
+
+    assert rebuilt.cookies.get_dict_by_domain() == ns.cookies.get_dict_by_domain()
+    assert rebuilt.cookies.get_dict() == ns.cookies.get_dict()
+    assert rebuilt.cookies.get("cdn_token", domain="cdn.example.com") == "abc"
+    assert rebuilt.cookies.get("api_token", domain="api.example.com") == "def"
+    assert rebuilt.cookies.get_dict(domain="localhost") == {}
+
+
+def test_rnet_spec_keeps_dict_set_cookies() -> None:
+    ns = rnet_session_with_cookies()
+
+    spec = build_session_spec(ns)
+    assert spec is not None
+    rebuilt = rebuild_session(spec, 1)
+    assert rebuilt is not None
+
+    assert rebuilt.cookies["loose"] == "ghi"
+
+
+def test_rnet_spec_cookies_carry_domain_information() -> None:
+    ns = rnet_session_with_cookies()
+
+    spec = build_session_spec(ns)
+    assert spec is not None
+
+    cookies = spec["cookies"]
+    assert set(cookies) == {"cdn.example.com", "api.example.com", ""}
+    assert all(isinstance(jar, dict) for jar in cookies.values())
+    assert cookies["cdn.example.com"] == {"cdn_token": "abc"}
+
+
+def test_rnet_spec_is_picklable() -> None:
+    spec = build_session_spec(rnet_session_with_cookies())
+    assert spec is not None
+
+    assert pickle.loads(pickle.dumps(spec)) == spec
+
+
+def test_rnet_spec_without_impersonate_is_not_rebuildable() -> None:
+    assert build_session_spec(RnetSession()) is None
+
+
+def test_rebuilt_requests_session_pool_is_sized_to_worker_count() -> None:
+    """A child rebuilding a requests session must size its pool, not inherit urllib3's default 10."""
+    spec = build_session_spec(Session())
+    assert spec is not None
+
+    rebuilt = rebuild_session(spec, 24)
+    assert rebuilt is not None
+
+    for scheme in ("https://example.com", "http://example.com"):
+        adapter = rebuilt.get_adapter(scheme)
+        assert adapter._pool_maxsize == 24
+        assert adapter._pool_connections == 24
+        assert adapter._pool_block is True
+        assert adapter.poolmanager.connection_pool_kw["maxsize"] == 24
+
+
+def test_rebuilt_rnet_session_has_no_adapters() -> None:
+    """rnet pools internally; the sizing fix must not reach into that path."""
+    spec = build_session_spec(rnet_session_with_cookies())
+    assert spec is not None
+
+    rebuilt = rebuild_session(spec, 24)
+    assert rebuilt is not None
+
+    assert not isinstance(rebuilt, Session)
+    assert not hasattr(rebuilt, "adapters")
+
+
+def test_unrebuildable_spec_stays_none() -> None:
+    assert rebuild_session({"kind": "none"}, 24) is None
+
+
+def test_requests_spec_round_trips_cookie_domains() -> None:
+    rs = Session()
+    rs.cookies.set("cdn_token", "abc", domain="cdn.example.com")
+    rs.cookies.set("api_token", "def", domain="api.example.com")
+
+    spec = build_session_spec(rs)
+    assert spec is not None
+    rebuilt = rebuild_session(pickle.loads(pickle.dumps(spec)), 1)
+    assert rebuilt is not None
+
+    assert rebuilt.cookies.get_dict(domain="cdn.example.com") == {"cdn_token": "abc"}
+    assert rebuilt.cookies.get_dict(domain="api.example.com") == {"api_token": "def"}