mirror of
https://github.com/unshackle-dl/unshackle.git
synced 2026-11-15 13:35:05 +00:00
fix(downloader): correct truncation, ranged sizing, and session carry
Check a sequential download against the complete length whenever one is known, and settle the unknown-total case on the 416. An empty body of unknown length, a 206 with no complete length, and a 206 starting past the resume offset all passed the size checks before and were renamed into place. Four restart paths spent no attempt, so a host that always answered the same way cycled forever.
This commit is contained in:
imSp4rky
@@ -0,0 +1,335 @@
|
||||
"""Removing a header or a cookie from an RnetSession must stop it reaching the wire.
|
||||
|
||||
``client.update(headers=...)`` merges into the rnet client default headers and has no
|
||||
removal, and the client keeps its own cookie store. These tests read what a real local
|
||||
server received, because the session dicts pass even with the leak in place.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import threading
|
||||
from http.cookiejar import Cookie, CookieJar
|
||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||
from typing import Iterator
|
||||
|
||||
import pytest
|
||||
import rnet
|
||||
|
||||
from unshackle.core.session import RnetCookieAdapter, RnetSession
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
class Recorder(BaseHTTPRequestHandler):
|
||||
protocol_version = "HTTP/1.1"
|
||||
received: list[dict[str, str]] = []
|
||||
cookies_sent: list[str] = []
|
||||
|
||||
def do_GET(self) -> None:
|
||||
type(self).received.append({k.lower(): v for k, v in self.headers.items()})
|
||||
# rnet sends one Cookie header per cookie on HTTP/1.1, so a dict of the headers
|
||||
# keeps only the last one and hides a cookie that leaked.
|
||||
type(self).cookies_sent.append("; ".join(self.headers.get_all("Cookie") or []))
|
||||
body = b"ok"
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Length", str(len(body)))
|
||||
if self.path == "/login":
|
||||
self.send_header("Set-Cookie", "sid=abc123; Path=/")
|
||||
elif self.path.startswith("/set/"):
|
||||
self.send_header("Set-Cookie", f"{self.path[len('/set/') :]}; Path=/")
|
||||
self.end_headers()
|
||||
self.wfile.write(body)
|
||||
|
||||
def log_message(self, *args: object) -> None:
|
||||
pass
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def server() -> Iterator[str]:
|
||||
Recorder.received = []
|
||||
Recorder.cookies_sent = []
|
||||
httpd = ThreadingHTTPServer(("127.0.0.1", 0), Recorder)
|
||||
threading.Thread(target=httpd.serve_forever, daemon=True).start()
|
||||
try:
|
||||
yield f"http://127.0.0.1:{httpd.server_port}"
|
||||
finally:
|
||||
httpd.shutdown()
|
||||
httpd.server_close()
|
||||
|
||||
|
||||
def make_cookie(name: str, value: str, domain: str) -> Cookie:
|
||||
return Cookie(
|
||||
version=0,
|
||||
name=name,
|
||||
value=value,
|
||||
port=None,
|
||||
port_specified=False,
|
||||
domain=domain,
|
||||
domain_specified=True,
|
||||
domain_initial_dot=domain.startswith("."),
|
||||
path="/",
|
||||
path_specified=True,
|
||||
secure=False,
|
||||
expires=None,
|
||||
discard=False,
|
||||
comment=None,
|
||||
comment_url=None,
|
||||
rest={},
|
||||
)
|
||||
|
||||
|
||||
def test_popped_header_stops_being_sent(server: str) -> None:
|
||||
session = RnetSession()
|
||||
session.headers.update({"X-Auth": "secret", "X-Keep": "yes"})
|
||||
|
||||
session.get(f"{server}/one")
|
||||
assert Recorder.received[-1]["x-auth"] == "secret"
|
||||
|
||||
session.headers.pop("X-Auth", None)
|
||||
session.get(f"{server}/two")
|
||||
assert "x-auth" not in Recorder.received[-1]
|
||||
assert Recorder.received[-1]["x-keep"] == "yes"
|
||||
|
||||
|
||||
def test_deleted_header_stops_being_sent(server: str) -> None:
|
||||
session = RnetSession()
|
||||
session.headers["X-Auth"] = "secret"
|
||||
|
||||
session.get(f"{server}/one")
|
||||
assert "x-auth" in Recorder.received[-1]
|
||||
|
||||
del session.headers["X-Auth"]
|
||||
session.get(f"{server}/two")
|
||||
assert "x-auth" not in Recorder.received[-1]
|
||||
|
||||
|
||||
def test_clear_removes_every_header(server: str) -> None:
|
||||
session = RnetSession()
|
||||
session.headers.update({"X-Auth": "secret", "X-Trace": "on"})
|
||||
|
||||
session.get(f"{server}/one")
|
||||
assert "x-auth" in Recorder.received[-1]
|
||||
|
||||
session.headers.clear()
|
||||
session.get(f"{server}/two")
|
||||
assert "x-auth" not in Recorder.received[-1]
|
||||
assert "x-trace" not in Recorder.received[-1]
|
||||
|
||||
|
||||
def test_header_removal_keeps_cookies(server: str) -> None:
|
||||
"""A removed header must not cost the session the cookies it has already earned."""
|
||||
session = RnetSession()
|
||||
session.headers["X-Auth"] = "secret"
|
||||
|
||||
session.get(f"{server}/login")
|
||||
session.get(f"{server}/before")
|
||||
assert Recorder.received[-1].get("cookie") == "sid=abc123"
|
||||
|
||||
session.headers.pop("X-Auth", None)
|
||||
session.get(f"{server}/after")
|
||||
assert Recorder.received[-1].get("cookie") == "sid=abc123"
|
||||
|
||||
|
||||
def test_surviving_headers_keep_the_impersonate_order(server: str) -> None:
|
||||
"""A rebuilt client must overlay the preset headers, not replace them."""
|
||||
session = RnetSession(impersonate=rnet.Impersonate.Chrome131)
|
||||
session.headers.update({"X-Auth": "secret", "X-Keep": "yes"})
|
||||
|
||||
session.get(f"{server}/one")
|
||||
before = [k for k in Recorder.received[-1] if k != "x-auth"]
|
||||
|
||||
session.headers.pop("X-Auth", None)
|
||||
session.get(f"{server}/two")
|
||||
assert [k for k in Recorder.received[-1]] == before
|
||||
|
||||
|
||||
def test_header_removal_keeps_cookies_for_a_host_not_yet_visited(server: str) -> None:
|
||||
"""A cookie unshackle set itself must survive the rebuild.
|
||||
|
||||
``rebuild_client`` can only read the rnet cookie jar per origin, and it knows only the
|
||||
origins the session has requested. Cookies loaded from a cookie file, or set through
|
||||
``session.cookies``, are for a host the session often reaches later.
|
||||
"""
|
||||
session = RnetSession()
|
||||
session.headers["X-Auth"] = "secret"
|
||||
session.get(f"{server}/one")
|
||||
|
||||
session.cookies.set("sid", "abc123", domain="example.test")
|
||||
session.headers.pop("X-Auth", None)
|
||||
|
||||
raw = session._client.get_cookies("https://example.test")
|
||||
assert raw is not None and b"sid=abc123" in raw
|
||||
|
||||
|
||||
def test_rebuild_survives_an_origin_arriving_mid_rebuild(server: str, monkeypatch) -> None:
|
||||
"""``request`` records origins from the download threads while the rebuild reads them.
|
||||
|
||||
Iterating the live set raises ``RuntimeError: Set changed size during iteration`` out of the
|
||||
header removal. Adding an origin from inside the copy stands in for that race.
|
||||
"""
|
||||
session = RnetSession()
|
||||
session.headers["X-Auth"] = "secret"
|
||||
session.get(f"{server}/one")
|
||||
|
||||
original = RnetSession.__dict__["copy_cookies"].__func__
|
||||
|
||||
def racing_copy(source, target, origin: str) -> None:
|
||||
session._origins.add(f"https://late-{len(session._origins)}.test")
|
||||
original(source, target, origin)
|
||||
|
||||
monkeypatch.setattr(RnetSession, "copy_cookies", staticmethod(racing_copy))
|
||||
|
||||
session.headers.pop("X-Auth", None)
|
||||
|
||||
session.get(f"{server}/two")
|
||||
assert "x-auth" not in Recorder.received[-1]
|
||||
|
||||
|
||||
def test_clear_drops_a_server_set_cookie(server: str) -> None:
|
||||
"""``cookies.clear()`` must reach the rnet cookie store, not only the local dicts."""
|
||||
session = RnetSession()
|
||||
|
||||
session.get(f"{server}/set/sid=abc123")
|
||||
session.get(f"{server}/before")
|
||||
assert Recorder.cookies_sent[-1] == "sid=abc123"
|
||||
|
||||
session.cookies.clear()
|
||||
session.get(f"{server}/after")
|
||||
assert Recorder.cookies_sent[-1] == ""
|
||||
|
||||
|
||||
def test_clear_leaves_the_session_able_to_take_new_cookies(server: str) -> None:
|
||||
"""A cleared session must still hold the next cookie, and only that one."""
|
||||
session = RnetSession()
|
||||
|
||||
session.get(f"{server}/set/sid=abc123")
|
||||
session.cookies.clear()
|
||||
|
||||
session.get(f"{server}/set/fresh=1")
|
||||
session.get(f"{server}/after")
|
||||
assert Recorder.cookies_sent[-1] == "fresh=1"
|
||||
|
||||
|
||||
def test_delete_drops_only_the_named_server_set_cookie(server: str) -> None:
|
||||
"""``del cookies[name]`` must drop that name from the store and leave the rest."""
|
||||
session = RnetSession()
|
||||
|
||||
session.get(f"{server}/set/sid=abc123")
|
||||
session.get(f"{server}/set/other=keep")
|
||||
|
||||
del session.cookies["sid"]
|
||||
|
||||
session.get(f"{server}/after")
|
||||
assert Recorder.cookies_sent[-1] == "other=keep"
|
||||
|
||||
|
||||
def test_clear_by_name_drops_a_server_set_cookie(server: str) -> None:
|
||||
"""``clear(name=...)`` is the requests spelling a service reaches for."""
|
||||
session = RnetSession()
|
||||
|
||||
session.get(f"{server}/set/sid=abc123")
|
||||
session.get(f"{server}/before")
|
||||
assert Recorder.cookies_sent[-1] == "sid=abc123"
|
||||
|
||||
session.cookies.clear(name="sid")
|
||||
session.get(f"{server}/after")
|
||||
assert Recorder.cookies_sent[-1] == ""
|
||||
|
||||
|
||||
def test_clear_before_the_client_exists_empties_the_jar_and_sends_nothing(server: str) -> None:
|
||||
"""``clear()`` on a session that has made no request must empty every cookie view.
|
||||
|
||||
``dl.save_cookies`` writes ``cookies.jar`` back to the cookie file and only ever merges,
|
||||
so a cleared cookie left in the jar returns to disk.
|
||||
"""
|
||||
session = RnetSession()
|
||||
jar = CookieJar()
|
||||
jar.set_cookie(make_cookie("sid", "abc123", "127.0.0.1"))
|
||||
session.cookies.update(jar)
|
||||
session.cookies.set("other", "keep", domain="127.0.0.1")
|
||||
|
||||
session.cookies.clear()
|
||||
|
||||
assert dict(session.cookies) == {}
|
||||
assert session.cookies.get_dict() == {}
|
||||
assert list(session.cookies.jar) == []
|
||||
|
||||
session.get(f"{server}/after")
|
||||
assert Recorder.cookies_sent[-1] == ""
|
||||
|
||||
|
||||
def test_a_later_header_rebuild_does_not_resurrect_cleared_cookies(server: str) -> None:
|
||||
"""``rebuild_client`` replays the buffered cookies and copies the old jar per origin.
|
||||
|
||||
A ``clear()`` that left either populated hands the cookie back on the next header removal.
|
||||
"""
|
||||
session = RnetSession()
|
||||
session.headers["X-Auth"] = "secret"
|
||||
|
||||
session.get(f"{server}/set/sid=abc123")
|
||||
session.cookies.clear()
|
||||
|
||||
session.headers.pop("X-Auth", None)
|
||||
|
||||
session.get(f"{server}/after")
|
||||
assert Recorder.cookies_sent[-1] == ""
|
||||
|
||||
|
||||
def test_a_request_during_the_rebuild_still_carries_headers_and_cookies(server: str, monkeypatch) -> None:
|
||||
"""A request from another thread must never see a client with neither headers nor cookies.
|
||||
|
||||
``rebuild_client`` publishes the new client before it carries the headers and the cookies,
|
||||
so a request in that window goes out unauthenticated.
|
||||
"""
|
||||
session = RnetSession()
|
||||
session.headers.update({"X-Auth": "secret", "X-Trace": "on"})
|
||||
|
||||
session.get(f"{server}/set/sid=abc123")
|
||||
session.get(f"{server}/before")
|
||||
assert Recorder.cookies_sent[-1] == "sid=abc123"
|
||||
|
||||
original = RnetCookieAdapter.flush_to_client
|
||||
rebuilding: list[bool] = []
|
||||
|
||||
def flush_with_a_request(self) -> None:
|
||||
if rebuilding:
|
||||
session.get(f"{server}/mid")
|
||||
original(self)
|
||||
|
||||
monkeypatch.setattr(RnetCookieAdapter, "flush_to_client", flush_with_a_request)
|
||||
rebuilding.append(True)
|
||||
session.headers.pop("X-Trace", None)
|
||||
|
||||
assert Recorder.received[-1].get("x-auth") == "secret"
|
||||
assert Recorder.cookies_sent[-1] == "sid=abc123"
|
||||
|
||||
|
||||
def test_the_rebuilt_client_is_published_only_when_it_is_ready(server: str, monkeypatch) -> None:
|
||||
"""The new client must go on ``_client`` last, after the headers, the flush and the copy."""
|
||||
session = RnetSession()
|
||||
session.headers["X-Auth"] = "secret"
|
||||
session.get(f"{server}/set/sid=abc123")
|
||||
|
||||
old = session._client
|
||||
seen: list[object] = []
|
||||
rebuilding: list[bool] = []
|
||||
|
||||
original_flush = RnetCookieAdapter.flush_to_client
|
||||
original_copy = RnetSession.__dict__["copy_cookies"].__func__
|
||||
|
||||
def recording_flush(self) -> None:
|
||||
if rebuilding:
|
||||
seen.append(session._client)
|
||||
original_flush(self)
|
||||
|
||||
def recording_copy(source, target, origin: str) -> None:
|
||||
seen.append(session._client)
|
||||
original_copy(source, target, origin)
|
||||
|
||||
monkeypatch.setattr(RnetCookieAdapter, "flush_to_client", recording_flush)
|
||||
monkeypatch.setattr(RnetSession, "copy_cookies", staticmethod(recording_copy))
|
||||
rebuilding.append(True)
|
||||
session.headers.pop("X-Auth", None)
|
||||
|
||||
assert seen and all(client is old for client in seen)
|
||||
assert session._client is not old
|
||||
@@ -4,11 +4,17 @@ only the adapter mounts."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from copy import copy
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
import requests
|
||||
from requests.adapters import HTTPAdapter
|
||||
from urllib3.util.retry import Retry
|
||||
|
||||
from unshackle.core.service import TimeoutHTTPAdapter, grow_session_pool
|
||||
from unshackle.core.service import DEFAULT_TIMEOUT, TimeoutHTTPAdapter, grow_session_pool
|
||||
from unshackle.core.utils.sslciphers import SSLCiphers
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
@@ -20,43 +26,166 @@ class FakeRnetSession:
|
||||
raise AssertionError("grow_session_pool must not touch a non-requests session")
|
||||
|
||||
|
||||
def test_small_pool_is_remounted_on_both_schemes() -> None:
|
||||
def sent_timeout(adapter: HTTPAdapter) -> Any:
|
||||
"""The timeout an adapter hands to HTTPAdapter.send when the caller passes none."""
|
||||
seen: dict[str, Any] = {}
|
||||
|
||||
def record(self: object, request: object, **kwargs: Any) -> str:
|
||||
seen.update(kwargs)
|
||||
return "sent"
|
||||
|
||||
with patch.object(HTTPAdapter, "send", record):
|
||||
assert adapter.send(object(), timeout=None) == "sent"
|
||||
|
||||
return seen["timeout"]
|
||||
|
||||
|
||||
def test_subclass_adapter_keeps_its_class_and_tls_context() -> None:
|
||||
"""A service-mounted SSLCiphers must survive the growth with its TLS context.
|
||||
|
||||
CBS, friDay, iP, MY5 and PMTP all mount SSLCiphers on the service session and none
|
||||
override get_session(). Replacing it with a plain adapter drops DEFAULT:@SECLEVEL=2 and
|
||||
check_hostname=False, so metadata succeeds and every segment then fails at TLS or 403.
|
||||
"""
|
||||
session = requests.Session()
|
||||
session.mount("https://", SSLCiphers(security_level=2))
|
||||
old = session.get_adapter("https://x")
|
||||
context = old._ssl_context
|
||||
assert old._pool_maxsize < 64
|
||||
|
||||
grow_session_pool(session, 64)
|
||||
|
||||
grown = session.get_adapter("https://x")
|
||||
assert grown is old
|
||||
assert isinstance(grown, SSLCiphers)
|
||||
assert grown._ssl_context is context
|
||||
assert grown.poolmanager.connection_pool_kw["ssl_context"] is context
|
||||
assert grown._pool_maxsize == 64
|
||||
assert grown._pool_block is True
|
||||
|
||||
|
||||
def test_a_distinct_http_adapter_is_grown_not_overwritten() -> None:
|
||||
"""Every mounted adapter grows, and no mount overwrites another.
|
||||
|
||||
The https:// adapter must not land on the http:// prefix, and a service that mounts on a
|
||||
narrow prefix must be grown as well.
|
||||
"""
|
||||
session = requests.Session()
|
||||
secure = SSLCiphers(security_level=2)
|
||||
narrow = SSLCiphers(security_level=2)
|
||||
plain = TimeoutHTTPAdapter(pool_connections=10, pool_maxsize=10)
|
||||
session.mount("https://", secure)
|
||||
session.mount("https://cbs.example/", narrow)
|
||||
session.mount("http://", plain)
|
||||
|
||||
grow_session_pool(session, 64)
|
||||
|
||||
assert session.get_adapter("https://x") is secure
|
||||
assert session.get_adapter("https://cbs.example/a") is narrow
|
||||
assert session.get_adapter("http://x") is plain
|
||||
assert session.get_adapter("http://x") is not session.get_adapter("https://x")
|
||||
for adapter in (secure, narrow, plain):
|
||||
assert adapter._pool_maxsize == 64
|
||||
assert adapter._pool_connections == 64
|
||||
|
||||
|
||||
def test_shared_adapter_is_grown_in_place_on_both_schemes() -> None:
|
||||
session = requests.Session()
|
||||
adapter = TimeoutHTTPAdapter(pool_connections=10, pool_maxsize=10, max_retries=Retry(total=5))
|
||||
session.mount("https://", adapter)
|
||||
session.mount("http://", adapter)
|
||||
retries = adapter.max_retries
|
||||
timeout = adapter.default_timeout
|
||||
|
||||
grow_session_pool(session, 64)
|
||||
|
||||
https = session.get_adapter("https://x")
|
||||
http = session.get_adapter("http://x")
|
||||
assert isinstance(https, TimeoutHTTPAdapter)
|
||||
assert https is http
|
||||
assert https is not old
|
||||
assert https is adapter
|
||||
assert session.get_adapter("http://x") is adapter
|
||||
assert type(https) is TimeoutHTTPAdapter
|
||||
assert https.max_retries is retries
|
||||
assert https.default_timeout == timeout
|
||||
assert https._pool_maxsize == 64
|
||||
assert https._pool_connections == 64
|
||||
assert https._pool_block is True
|
||||
assert https.max_retries is old.max_retries
|
||||
assert https.poolmanager.connection_pool_kw["maxsize"] == 64
|
||||
|
||||
|
||||
def test_cached_proxy_manager_is_rebuilt_at_the_new_size() -> None:
|
||||
"""proxy_manager_for caches one manager per proxy URL, sized when it was first built.
|
||||
|
||||
authenticate() and get_tracks() run before the growth, so a service behind --proxy has
|
||||
already filled that cache. Leaving it there keeps every proxied segment on the small pool.
|
||||
"""
|
||||
session = requests.Session()
|
||||
adapter = TimeoutHTTPAdapter(pool_connections=4, pool_maxsize=4)
|
||||
session.mount("https://", adapter)
|
||||
proxy = "http://proxy.example:8080"
|
||||
stale = adapter.proxy_manager_for(proxy)
|
||||
assert stale.connection_pool_kw["maxsize"] == 4
|
||||
|
||||
grow_session_pool(session, 64)
|
||||
|
||||
fresh = adapter.proxy_manager_for(proxy)
|
||||
assert fresh.connection_pool_kw["maxsize"] == 64
|
||||
assert fresh is not stale
|
||||
|
||||
|
||||
def test_large_enough_pool_is_left_alone() -> None:
|
||||
session = requests.Session()
|
||||
session.mount("https://", HTTPAdapter(pool_connections=64, pool_maxsize=64))
|
||||
old = session.get_adapter("https://x")
|
||||
pool = old.poolmanager
|
||||
|
||||
grow_session_pool(session, 64)
|
||||
|
||||
assert session.get_adapter("https://x") is old
|
||||
assert old.poolmanager is pool, "an adapter that needs no growth must keep its warm connections"
|
||||
|
||||
|
||||
def test_equal_size_pool_is_left_alone() -> None:
|
||||
session = requests.Session()
|
||||
session.mount("https://", HTTPAdapter(pool_maxsize=32))
|
||||
old = session.get_adapter("https://x")
|
||||
pool = old.poolmanager
|
||||
|
||||
grow_session_pool(session, 32)
|
||||
|
||||
assert session.get_adapter("https://x") is old
|
||||
assert old.poolmanager is pool
|
||||
|
||||
|
||||
@pytest.mark.parametrize("session", [object(), FakeRnetSession()])
|
||||
def test_non_requests_session_is_ignored(session: object) -> None:
|
||||
grow_session_pool(session, 64)
|
||||
|
||||
|
||||
def test_copied_adapter_keeps_the_default_timeout() -> None:
|
||||
"""default_timeout must survive copy(), which no_retry_session runs per segment.
|
||||
|
||||
copy() runs HTTPAdapter.__getstate__/__setstate__, which carry only the names in
|
||||
__attrs__. Without default_timeout there, the copy quietly falls back to DEFAULT_TIMEOUT
|
||||
on the segment path and drops the service timeout.
|
||||
|
||||
The last two lines pin the getattr fallback in send(). A subclass that redefines
|
||||
__attrs__ from HTTPAdapter.__attrs__, as SSLCiphers does, drops default_timeout again.
|
||||
"""
|
||||
twin = copy(TimeoutHTTPAdapter(timeout=(3, 300), pool_connections=4, pool_maxsize=4))
|
||||
assert twin.default_timeout == (3, 300)
|
||||
assert sent_timeout(twin) == (3, 300)
|
||||
|
||||
del twin.default_timeout
|
||||
assert sent_timeout(twin) == DEFAULT_TIMEOUT
|
||||
|
||||
|
||||
def test_growing_does_not_revert_a_custom_default_timeout() -> None:
|
||||
session = requests.Session()
|
||||
session.mount("https://", TimeoutHTTPAdapter(timeout=(1, 5), pool_connections=4, pool_maxsize=4))
|
||||
old = session.get_adapter("https://x")
|
||||
|
||||
grow_session_pool(session, 64)
|
||||
|
||||
grown = session.get_adapter("https://x")
|
||||
assert grown.default_timeout == (1, 5)
|
||||
assert grown is old
|
||||
assert copy(grown).default_timeout == (1, 5), "the per-segment copy must keep the service timeout"
|
||||
|
||||
Reference in New Issue
Block a user