diff --git a/garminconnect/client.py b/garminconnect/client.py index 55c92b9..b1e6883 100644 --- a/garminconnect/client.py +++ b/garminconnect/client.py @@ -18,6 +18,7 @@ import re import threading import time +from collections.abc import Iterator, Mapping from pathlib import Path from typing import Any, cast @@ -185,6 +186,61 @@ def _build_basic_auth(client_id: str) -> str: return "Basic " + base64.b64encode(f"{client_id}:".encode()).decode() +def _iter_file_objects(kwargs: dict[str, Any]) -> Iterator[Any]: + """Yield file-like objects referenced by request kwargs (files/data).""" + files = kwargs.get("files") + if isinstance(files, Mapping): + values = list(files.values()) + elif isinstance(files, (list, tuple)): + values = [v for _, v in files] + else: + values = [] + for value in values: + # requests accepts fileobj or (name, fileobj[, content_type[, headers]]) + fileobj = ( + value[1] if isinstance(value, (tuple, list)) and len(value) >= 2 else value + ) + if hasattr(fileobj, "read"): + yield fileobj + data = kwargs.get("data") + if hasattr(data, "read"): + yield data + + +def _capture_file_positions( + kwargs: dict[str, Any], +) -> list[tuple[Any, int]] | None: + """Record the stream position of each file-like body before a request. + + Returns None when any body cannot be repositioned, meaning a retry + would re-read from EOF and silently send empty/truncated content. + """ + positions: list[tuple[Any, int]] = [] + for fileobj in _iter_file_objects(kwargs): + try: + if not fileobj.seekable(): + return None + positions.append((fileobj, fileobj.tell())) + except (OSError, ValueError, AttributeError): + return None + data = kwargs.get("data") + if data is not None and not hasattr(data, "read") and isinstance(data, Iterator): + # Streamed iterable body (e.g. a generator): attempt #1 consumes it + # and it cannot be rewound, so a retry must not be attempted. + return None + return positions + + +def _restore_file_positions(positions: list[tuple[Any, int]]) -> bool: + """Rewind file-like bodies to their pre-request positions. False on failure.""" + try: + for fileobj, pos in positions: + fileobj.seek(pos) + except (OSError, ValueError, AttributeError): + return False + return True + + def _decode_jwt_payload(token: str) -> dict[str, Any] | None: """Decode a JWT payload without verifying the signature. @@ -1532,12 +1588,23 @@ def _run_request(self, method: str, path: str, **kwargs: Any) -> Any: headers.update(custom_headers) sess = self._api_session + # Snapshot stream positions of any file bodies so a 401 retry can + # rewind them; attempt #1 reads file handles to EOF, and re-sending + # the same kwargs would otherwise upload an empty/truncated body. + file_positions = _capture_file_positions(kwargs) resp = sess.request(method, url, headers=headers, **kwargs) if resp.status_code == 401: with self._token_lock: self._refresh_session() - resp = sess.request(method, url, headers=self.get_api_headers(), **kwargs) + if file_positions is not None and _restore_file_positions(file_positions): + headers = self.get_api_headers() + headers.update(custom_headers) + resp = sess.request(method, url, headers=headers, **kwargs) + else: + # Unseekable/unrewindable file body: retrying would silently + # send an empty part. Fall through so the 401 raises below. + _LOGGER.debug("Skipping 401 retry: request body is not rewindable") if resp.status_code == 204: diff --git a/tests/test_garmin_unit.py b/tests/test_garmin_unit.py index 29c19c8..5a9ef66 100644 --- a/tests/test_garmin_unit.py +++ b/tests/test_garmin_unit.py @@ -1639,6 +1639,151 @@ def test_rejects_path_with_traversal_or_query( c._run_request("GET", bad_path) +# --------------------------------------------------------------------------- +# _run_request: 401 retry must rewind file bodies and keep custom headers +# --------------------------------------------------------------------------- + + +class Test401RetryFileRewind: + """A 401 retry must not re-send a consumed file handle (empty upload).""" + + def _client(self, monkeypatch, request_fn): + g = garminconnect.Garmin() + c = g.client + monkeypatch.setattr(c, "get_api_headers", dict) + monkeypatch.setattr(c, "_refresh_session", lambda: None) + monkeypatch.setattr(c._api_session, "request", request_fn) + return c + + def _consuming_request(self, responses, read_sizes): + """Fake session.request that drains file bodies like requests does.""" + + def request(method, url, **kwargs): + files = kwargs.get("files") or {} + for value in files.values(): + fileobj = value[1] if isinstance(value, tuple) else value + read_sizes.append(len(fileobj.read())) + return responses.pop(0) + + return request + + def test_retry_rewinds_file_body(self, monkeypatch): + read_sizes: list[int] = [] + responses = [_FakeResp(401, {}), _FakeResp(200, {"ok": True})] + c = self._client( + monkeypatch, self._consuming_request(responses, read_sizes) + ) + payload = b"FITDATA" * 100 + resp = c._run_request( + "POST", "upload", files={"file": ("a.fit", io.BytesIO(payload))} + ) + assert resp.status_code == 200 + # Attempt #1 and the retry must both read the full payload. + assert read_sizes == [len(payload), len(payload)] + + def test_retry_restores_initial_position_not_zero(self, monkeypatch): + read_sizes: list[int] = [] + responses = [_FakeResp(401, {}), _FakeResp(200, {"ok": True})] + c = self._client( + monkeypatch, self._consuming_request(responses, read_sizes) + ) + stream = io.BytesIO(b"HEADER" + b"BODY" * 10) + stream.seek(6) # caller intentionally skips a prefix + c._run_request("POST", "upload", files={"file": ("a.fit", stream)}) + assert read_sizes[0] == read_sizes[1] == 40 + + def test_unseekable_body_skips_retry_and_raises(self, monkeypatch): + class UnseekableStream(io.BytesIO): + def seekable(self): + return False + + calls = [] + + def request(method, url, **kwargs): + calls.append(url) + return _FakeResp(401, {}) + + c = self._client(monkeypatch, request) + with pytest.raises(garminconnect.GarminConnectConnectionError): + c._run_request( + "POST", + "upload", + files={"file": ("a.fit", UnseekableStream(b"DATA"))}, + ) + # No blind retry that would have sent an empty body. + assert len(calls) == 1 + + def test_retry_rewinds_mapping_files(self, monkeypatch): + # requests accepts any Mapping for files (via to_key_val_list), so + # position capture must not be limited to plain dicts. + from types import MappingProxyType + + read_sizes: list[int] = [] + responses = [_FakeResp(401, {}), _FakeResp(200, {"ok": True})] + c = self._client( + monkeypatch, self._consuming_request(responses, read_sizes) + ) + payload = b"FITDATA" * 100 + files = MappingProxyType({"file": ("a.fit", io.BytesIO(payload))}) + resp = c._run_request("POST", "upload", files=files) + assert resp.status_code == 200 + assert read_sizes == [len(payload), len(payload)] + + def test_generator_data_skips_retry_and_raises(self, monkeypatch): + # A streamed (generator) body is consumed by attempt #1 and cannot be + # rewound; the 401 must raise instead of resending the remainder. + calls = [] + + def request(method, url, **kwargs): + calls.append(url) + # Drain the streamed body like requests does. + for _ in kwargs.get("data") or (): + pass + return _FakeResp(401, {}) + + c = self._client(monkeypatch, request) + + def body(): + yield b"chunk1" + yield b"chunk2" + + with pytest.raises(garminconnect.GarminConnectConnectionError): + c._run_request("POST", "upload", data=body()) + assert len(calls) == 1 + + def test_restore_file_positions_false_when_seek_missing(self): + # A duck-typed stream with seekable()/tell() but no seek() must make + # restoration fail (AttributeError caught) rather than raise. + class NoSeekStream: + def read(self, n: int = -1) -> bytes: + return b"" + + def seekable(self) -> bool: + return True + + def tell(self) -> int: + return 0 + + assert client_mod._restore_file_positions([(NoSeekStream(), 0)]) is False + + def test_retry_keeps_custom_headers(self, monkeypatch): + seen_headers = [] + responses = [_FakeResp(401, {}), _FakeResp(200, {"ok": True})] + + def request(method, url, **kwargs): + seen_headers.append(kwargs.get("headers") or {}) + return responses.pop(0) + + c = self._client(monkeypatch, request) + c._run_request( + "POST", + "upload", + headers={"NK": "NT", "User-Agent": "GCM-iOS-5.7.2.1"}, + ) + assert seen_headers[1].get("NK") == "NT" + assert seen_headers[1].get("User-Agent") == "GCM-iOS-5.7.2.1" + + # --------------------------------------------------------------------------- # Error-message sanitization # ---------------------------------------------------------------------------