From 2e2fd395c290f1bdbf05456d1f974dc5c5c3c50e Mon Sep 17 00:00:00 2001 From: "Shane B." Date: Thu, 20 Aug 2026 10:13:30 +0200 Subject: [PATCH] Give the exporter's requests thirty seconds, as the app already has The Android app raised this a release ago; the exporter was left on FindMy.py's default of five seconds per request, and nothing here ever passed the parameter that commit made settable. Two halves of one project disagreeing about how patient this protocol needs somebody to be. **Five is a cap on one request, and this makes many.** A sign-in is several round trips, and what follows it is heavier than the sign-in: escrow recovery, then the CloudKit fetches that carry the actual keys. A link slow enough to spend five seconds on any one of them fails partway through, as a bare timeout naming no cause. Passed in three places because there are three constructions, and only some of them are on the path any given run takes: - a first run's account - a later run's account, restored from the stored identity - which is the one nearly every export goes through, and the one a fix applied to the obvious call site would miss - `RemoteAnisetteProvider`, which has its own session, so raising the account's does nothing for it. The Anisette fetch happens inside the login but from the provider's own client. `LocalAnisetteProvider` takes no timeout and is not given one; there is no HTTP in it to spend it on. It is also the default here - Anisette is generated on this machine - so this is about Apple's own hosts, and about `--anisette-url` for anyone who does point it at a server. Five tests, asserting on what the call passes rather than on FindMy.py's internals: the value lands on a private session attribute with no accessor, and a test reaching in there breaks on a rename that changed nothing real. What can regress is this module forgetting to pass it, since the default is silent. Verified by removing each of the three, which reddens one, one and two. One of them pins that the raised value is still above upstream's default, so a future release raising its own past thirty would be a downgrade nothing else here would notice. Found while writing the 1.3.0 release notes, where it had been listed as an exporter fix on the strength of the app's commit touching python/. It touched `pyproject.toml` and `uv.lock` - the pin that makes the parameter exist - and nothing that passes it. Co-Authored-By: Claude Opus 5 --- python/exporter/icloud.py | 34 +++++++++- python/test/test_login_timeout.py | 103 ++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 python/test/test_login_timeout.py diff --git a/python/exporter/icloud.py b/python/exporter/icloud.py index 2eea938d..a1c15fe1 100644 --- a/python/exporter/icloud.py +++ b/python/exporter/icloud.py @@ -163,6 +163,24 @@ class ClientIdentity: """The desktop exporter's, and the default for every entry point here.""" +LOGIN_TIMEOUT_SECONDS = 30 +""" +Seconds any one request may take, in place of FindMy.py's default of five. + +**Five is a per-request cap, and this makes far more than one request.** Signing in is several +round trips, and what follows it - escrow recovery, then the CloudKit fetches that carry the +actual keys - is heavier than the login. A link slow enough to spend five seconds on any one of +them fails partway through with a bare timeout that names no cause. + +Local ADI is the default here and generates its Anisette on this machine, so that part is not +waiting on anybody. It is Apple's own hosts that this is about - and `--anisette-url`, for anyone +who does point it at a server. + +Thirty rather than a larger number because it is what the Android app settled on for the same +sign-in, and the two should not disagree about how patient this protocol needs somebody to be. +""" + + def make_account( anisette_url: str | None = None, libs_path: str | None = None, @@ -200,7 +218,8 @@ def make_account( provider = _make_provider(anisette_url, libs_path, stored, identity) if stored is None: - return AsyncAppleAccount(provider, device_name=identity.device_name) + return AsyncAppleAccount( + provider, device_name=identity.device_name, timeout=LOGIN_TIMEOUT_SECONDS) # Only the ids are restored. The rest of the shape has to be there because the library reads # it, and every field of it is empty on purpose - this is a logged-out account that happens to @@ -215,7 +234,12 @@ def make_account( logger.info("Reusing the stored device identity, so this is not a new device to Apple") - return AsyncAppleAccount(provider, state_info=state, device_name=identity.device_name) + return AsyncAppleAccount( + provider, + state_info=state, + device_name=identity.device_name, + timeout=LOGIN_TIMEOUT_SECONDS, + ) def _make_provider( @@ -226,7 +250,11 @@ def _make_provider( ) -> LocalAnisetteProvider | RemoteAnisetteProvider: """Build the Anisette provider, restoring its provisioning data if there is any.""" if anisette_url is not None: - return RemoteAnisetteProvider(anisette_url, serial=identity.serial) + # Its own session, so raising the account's does nothing for this one: the Anisette fetch + # happens inside the login but from the provider's own client. The local provider below + # takes no timeout, because there is no HTTP in it to spend one on. + return RemoteAnisetteProvider( + anisette_url, serial=identity.serial, timeout=LOGIN_TIMEOUT_SECONDS) saved = (stored or {}).get("anisette") state_blob = None diff --git a/python/test/test_login_timeout.py b/python/test/test_login_timeout.py new file mode 100644 index 00000000..853a8330 --- /dev/null +++ b/python/test/test_login_timeout.py @@ -0,0 +1,103 @@ +""" +How long the exporter is prepared to wait for one request. + +FindMy.py's default is five seconds per request, which is a desktop assumption about a single +call. This makes many: a sign-in is several round trips, and escrow recovery and the CloudKit +fetches after it are heavier than the sign-in. A link slow enough to spend five seconds on any one +of them fails partway through with a bare timeout that names no cause. + +**Asserted on what the call passes, not on FindMy.py's internals.** The timeout ends up on a +private session attribute with no accessor, and a test reaching in there breaks on a rename that +changed nothing real. What can actually regress here is this module forgetting to pass it - which +is the whole bug, since the default is silent. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from exporter import device, icloud + + +@pytest.fixture +def built(monkeypatch): + """Record what `make_account` hands to the library, instead of building a real account.""" + seen: dict[str, Any] = {} + + class RecordingAccount: + def __init__(self, provider, **kwargs): + seen["provider"] = provider + seen["kwargs"] = kwargs + + monkeypatch.setattr(icloud, "AsyncAppleAccount", RecordingAccount) + + return seen + + +@pytest.fixture +def no_stored_identity(monkeypatch): + monkeypatch.setattr(device, "load", lambda *_args, **_kwargs: None) + monkeypatch.setattr(icloud.device, "load", lambda *_args, **_kwargs: None) + + +@pytest.fixture +def a_stored_identity(monkeypatch): + """An install that has run before, which is the path that is easy to miss.""" + stored = {"uid": "a-uid", "devid": "a-devid", "anisette": None} + monkeypatch.setattr(icloud.device, "load", lambda *_args, **_kwargs: stored) + + return stored + + +class TestTheAccount: + def test_a_first_run_gets_the_raised_timeout(self, built, no_stored_identity): + icloud.make_account(provider=object()) + + assert built["kwargs"]["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + def test_a_later_run_gets_it_too(self, built, a_stored_identity): + # Two constructions of the same object, and only one of them was reached by the fix that + # first added a keyword here. The second is the one nearly every export takes. + icloud.make_account(provider=_JsonableProvider()) + + assert built["kwargs"]["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + def test_it_is_longer_than_the_library_default(self): + from findmy.util.http import DEFAULT_TIMEOUT + + # If upstream ever raises its own default past this, passing ours would be a downgrade + # rather than a fix, and nothing else here would notice. + assert icloud.LOGIN_TIMEOUT_SECONDS > DEFAULT_TIMEOUT + + +class TestTheAnisetteProvider: + """ + Its own session, so raising the account's does nothing for this one. + + The Anisette fetch happens inside the login but from the provider's own client. Asserted + through `to_json`, which is the only place the value is visible without reaching into a + private attribute - it is carried there precisely so a restored session keeps it. + """ + + def test_a_remote_server_gets_the_raised_timeout(self): + provider = icloud._make_provider("https://ani.example.invalid", None, None) + + assert provider.to_json()["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + def test_a_restored_session_keeps_it(self): + # `to_json` omits the timeout when it equals the default, so a session restored from a + # blob written without one silently goes back to five seconds. + original = icloud._make_provider("https://ani.example.invalid", None, None) + + restored = type(original).from_json(original.to_json()) + + assert restored.to_json()["timeout"] == icloud.LOGIN_TIMEOUT_SECONDS + + +class _JsonableProvider: + """Stands in for an Anisette provider, which `make_account` serialises into the state blob.""" + + def to_json(self): + return {"type": "aniLocal"}