From 10ede352ffdabb6173fa72bf508f537370d2a868 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2026 07:38:14 -0500 Subject: [PATCH 1/5] Avoid infinite loop when cached mirror redirect keeps failing The registry mirror iterator caches the HEAD redirect for one hour, but the cache hit path never advanced _visited_mirrors, and the cached payload did not include X-PIO-Mirror. When a prior run cached a redirect to a mirror that now fails (for example an SSL error on the download), subsequent iterations kept hitting the same cache key and returning the same failing mirror forever, so tool installs looped until interrupted. Store X-PIO-Mirror in the cached payload, and on cache hit only reuse the entry if it names a mirror not already tried; otherwise fall through to a fresh HEAD with bypass set. Stale cache entries written before this change lack X-PIO-Mirror and fall through naturally, so the fix self heals without a manual cache clear. --- platformio/registry/mirror.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/platformio/registry/mirror.py b/platformio/registry/mirror.py index 240e9a814a..4d48791471 100644 --- a/platformio/registry/mirror.py +++ b/platformio/registry/mirror.py @@ -43,10 +43,17 @@ def __next__(self): if result is not None: try: headers = json.loads(result) - return ( - headers["Location"], - headers["X-PIO-Content-SHA256"], - ) + mirror = headers.get("X-PIO-Mirror") + # Only use the cached entry if it identifies a mirror we + # have not already tried. Without this, a cached redirect + # to a failing mirror would be returned forever because + # _visited_mirrors never advances on the cache-hit path. + if mirror and mirror not in self._visited_mirrors: + self._visited_mirrors.append(mirror) + return ( + headers["Location"], + headers["X-PIO-Content-SHA256"], + ) except (ValueError, KeyError): pass @@ -84,6 +91,7 @@ def __next__(self): "X-PIO-Content-SHA256": response.headers.get( "X-PIO-Content-SHA256" ), + "X-PIO-Mirror": response.headers.get("X-PIO-Mirror"), } ), "1h", From 038544faaea442b2bfa38c4a0e9efb9a6ed8adac Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2026 07:44:11 -0500 Subject: [PATCH 2/5] Add unit tests for registry mirror iterator cache handling Cover the cache hit path advancing _visited_mirrors, stale cache entries without X-PIO-Mirror falling through to a fresh HEAD, and new HEAD responses persisting X-PIO-Mirror so later runs can tell whether the cached redirect still names an untried mirror. Each test fails against the previous iterator implementation. --- tests/registry/__init__.py | 13 +++ tests/registry/test_mirror.py | 171 ++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 tests/registry/__init__.py create mode 100644 tests/registry/test_mirror.py diff --git a/tests/registry/__init__.py b/tests/registry/__init__.py new file mode 100644 index 0000000000..b051490361 --- /dev/null +++ b/tests/registry/__init__.py @@ -0,0 +1,13 @@ +# Copyright (c) 2014-present PlatformIO +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/tests/registry/test_mirror.py b/tests/registry/test_mirror.py new file mode 100644 index 0000000000..06aedc11ff --- /dev/null +++ b/tests/registry/test_mirror.py @@ -0,0 +1,171 @@ +# Copyright (c) 2014-present PlatformIO +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json + +import pytest + +from platformio.cache import ContentCache +from platformio.registry import mirror +from platformio.registry.mirror import RegistryFileMirrorIterator + + +class _FakeContentCache: + """In memory stand in for ContentCache used by the iterator tests.""" + + _store: dict = {} + + def __init__(self, _namespace=None): + pass + + def __enter__(self): + return self + + def __exit__(self, *_exc): + return False + + def get(self, key): + return type(self)._store.get(key) + + def set(self, key, data, _valid): + type(self)._store[key] = data + + @staticmethod + def key_from_args(*args): + return ContentCache.key_from_args(*args) + + +class _FakeResponse: + def __init__(self, status_code, headers): + self.status_code = status_code + self.headers = headers + + +class _FakeHTTPClient: + def __init__(self, responses): + self._responses = list(responses) + self.requests = [] + + def send_request(self, method, path, **kwargs): + self.requests.append((method, path, kwargs)) + return self._responses.pop(0) + + +@pytest.fixture +def fake_cache(monkeypatch): + _FakeContentCache._store = {} + monkeypatch.setattr(mirror, "ContentCache", _FakeContentCache) + return _FakeContentCache + + +def _install_fake_http(monkeypatch, responses): + http_client = _FakeHTTPClient(responses) + monkeypatch.setattr( + RegistryFileMirrorIterator, "get_http_client", lambda _self: http_client + ) + return http_client + + +def test_cache_hit_advances_visited_mirrors(fake_cache, monkeypatch): + # A cached mirror must be recorded as visited on the cache hit path, + # otherwise the next iteration would recompute the same cache key and + # replay the same redirect forever when the mirror keeps failing. + url = "https://dl.example.com/packages/tool.tar.gz" + fake_cache._store[ContentCache.key_from_args("head", url, [])] = json.dumps( + { + "Location": "https://mirror-a.example.com/tool.tar.gz", + "X-PIO-Content-SHA256": "aaa", + "X-PIO-Mirror": "mirror-a", + } + ) + http_client = _install_fake_http( + monkeypatch, + [ + _FakeResponse( + 302, + { + "Location": "https://mirror-b.example.com/tool.tar.gz", + "X-PIO-Mirror": "mirror-b", + "X-PIO-Content-SHA256": "bbb", + }, + ) + ], + ) + + iterator = RegistryFileMirrorIterator(url) + first = next(iterator) + second = next(iterator) + + assert first == ("https://mirror-a.example.com/tool.tar.gz", "aaa") + assert second == ("https://mirror-b.example.com/tool.tar.gz", "bbb") + assert http_client.requests, "second iteration should issue a HEAD request" + assert http_client.requests[-1][2]["params"] == {"bypass": "mirror-a"} + + +def test_cache_entry_without_mirror_header_falls_through(fake_cache, monkeypatch): + # Entries written before X-PIO-Mirror was cached lack the field; the + # iterator must treat them as unusable and request a fresh redirect so + # the fix self heals existing caches. + url = "https://dl.example.com/packages/tool.tar.gz" + fake_cache._store[ContentCache.key_from_args("head", url, [])] = json.dumps( + { + "Location": "https://mirror-a.example.com/tool.tar.gz", + "X-PIO-Content-SHA256": "aaa", + } + ) + _install_fake_http( + monkeypatch, + [ + _FakeResponse( + 302, + { + "Location": "https://mirror-b.example.com/tool.tar.gz", + "X-PIO-Mirror": "mirror-b", + "X-PIO-Content-SHA256": "bbb", + }, + ) + ], + ) + + iterator = RegistryFileMirrorIterator(url) + + assert next(iterator) == ("https://mirror-b.example.com/tool.tar.gz", "bbb") + + +def test_head_response_caches_mirror_header(fake_cache, monkeypatch): + # The iterator must persist X-PIO-Mirror so later runs can tell whether + # the cached redirect still names a mirror that has not been tried. + url = "https://dl.example.com/packages/tool.tar.gz" + _install_fake_http( + monkeypatch, + [ + _FakeResponse( + 302, + { + "Location": "https://mirror-a.example.com/tool.tar.gz", + "X-PIO-Mirror": "mirror-a", + "X-PIO-Content-SHA256": "aaa", + }, + ) + ], + ) + + iterator = RegistryFileMirrorIterator(url) + next(iterator) + + cached = json.loads( + fake_cache._store[ContentCache.key_from_args("head", url, [])] + ) + assert cached["X-PIO-Mirror"] == "mirror-a" + assert cached["Location"] == "https://mirror-a.example.com/tool.tar.gz" From d014de9e828447277eb7fad479c3b69e5a1ecc7a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2026 07:49:11 -0500 Subject: [PATCH 3/5] Add type annotations to mirror iterator tests --- tests/registry/test_mirror.py | 41 +++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/tests/registry/test_mirror.py b/tests/registry/test_mirror.py index 06aedc11ff..b1554cb4e6 100644 --- a/tests/registry/test_mirror.py +++ b/tests/registry/test_mirror.py @@ -13,6 +13,7 @@ # limitations under the License. import json +from typing import Any import pytest @@ -24,52 +25,54 @@ class _FakeContentCache: """In memory stand in for ContentCache used by the iterator tests.""" - _store: dict = {} + _store: dict[str, str] = {} - def __init__(self, _namespace=None): + def __init__(self, _namespace: str | None = None) -> None: pass - def __enter__(self): + def __enter__(self) -> "_FakeContentCache": return self - def __exit__(self, *_exc): + def __exit__(self, *_exc: Any) -> bool: return False - def get(self, key): + def get(self, key: str) -> str | None: return type(self)._store.get(key) - def set(self, key, data, _valid): + def set(self, key: str, data: str, _valid: str) -> None: type(self)._store[key] = data @staticmethod - def key_from_args(*args): + def key_from_args(*args: Any) -> str: return ContentCache.key_from_args(*args) class _FakeResponse: - def __init__(self, status_code, headers): + def __init__(self, status_code: int, headers: dict[str, str]) -> None: self.status_code = status_code self.headers = headers class _FakeHTTPClient: - def __init__(self, responses): + def __init__(self, responses: list[_FakeResponse]) -> None: self._responses = list(responses) - self.requests = [] + self.requests: list[tuple[str, str, dict[str, Any]]] = [] - def send_request(self, method, path, **kwargs): + def send_request(self, method: str, path: str, **kwargs: Any) -> _FakeResponse: self.requests.append((method, path, kwargs)) return self._responses.pop(0) @pytest.fixture -def fake_cache(monkeypatch): +def fake_cache(monkeypatch: pytest.MonkeyPatch) -> type[_FakeContentCache]: _FakeContentCache._store = {} monkeypatch.setattr(mirror, "ContentCache", _FakeContentCache) return _FakeContentCache -def _install_fake_http(monkeypatch, responses): +def _install_fake_http( + monkeypatch: pytest.MonkeyPatch, responses: list[_FakeResponse] +) -> _FakeHTTPClient: http_client = _FakeHTTPClient(responses) monkeypatch.setattr( RegistryFileMirrorIterator, "get_http_client", lambda _self: http_client @@ -77,7 +80,9 @@ def _install_fake_http(monkeypatch, responses): return http_client -def test_cache_hit_advances_visited_mirrors(fake_cache, monkeypatch): +def test_cache_hit_advances_visited_mirrors( + fake_cache: type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch +) -> None: # A cached mirror must be recorded as visited on the cache hit path, # otherwise the next iteration would recompute the same cache key and # replay the same redirect forever when the mirror keeps failing. @@ -113,7 +118,9 @@ def test_cache_hit_advances_visited_mirrors(fake_cache, monkeypatch): assert http_client.requests[-1][2]["params"] == {"bypass": "mirror-a"} -def test_cache_entry_without_mirror_header_falls_through(fake_cache, monkeypatch): +def test_cache_entry_without_mirror_header_falls_through( + fake_cache: type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch +) -> None: # Entries written before X-PIO-Mirror was cached lack the field; the # iterator must treat them as unusable and request a fresh redirect so # the fix self heals existing caches. @@ -143,7 +150,9 @@ def test_cache_entry_without_mirror_header_falls_through(fake_cache, monkeypatch assert next(iterator) == ("https://mirror-b.example.com/tool.tar.gz", "bbb") -def test_head_response_caches_mirror_header(fake_cache, monkeypatch): +def test_head_response_caches_mirror_header( + fake_cache: type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch +) -> None: # The iterator must persist X-PIO-Mirror so later runs can tell whether # the cached redirect still names a mirror that has not been tried. url = "https://dl.example.com/packages/tool.tar.gz" From 381ee96d037290e9e6c6b180e6a4a8d1c1024bbd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2026 07:50:31 -0500 Subject: [PATCH 4/5] Use typing.X annotations for 3.6 compatibility --- tests/registry/test_mirror.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/registry/test_mirror.py b/tests/registry/test_mirror.py index b1554cb4e6..90f37954f4 100644 --- a/tests/registry/test_mirror.py +++ b/tests/registry/test_mirror.py @@ -13,7 +13,7 @@ # limitations under the License. import json -from typing import Any +from typing import Any, Dict, List, Optional, Tuple, Type import pytest @@ -25,9 +25,9 @@ class _FakeContentCache: """In memory stand in for ContentCache used by the iterator tests.""" - _store: dict[str, str] = {} + _store: Dict[str, str] = {} - def __init__(self, _namespace: str | None = None) -> None: + def __init__(self, _namespace: Optional[str] = None) -> None: pass def __enter__(self) -> "_FakeContentCache": @@ -36,7 +36,7 @@ def __enter__(self) -> "_FakeContentCache": def __exit__(self, *_exc: Any) -> bool: return False - def get(self, key: str) -> str | None: + def get(self, key: str) -> Optional[str]: return type(self)._store.get(key) def set(self, key: str, data: str, _valid: str) -> None: @@ -48,15 +48,15 @@ def key_from_args(*args: Any) -> str: class _FakeResponse: - def __init__(self, status_code: int, headers: dict[str, str]) -> None: + def __init__(self, status_code: int, headers: Dict[str, str]) -> None: self.status_code = status_code self.headers = headers class _FakeHTTPClient: - def __init__(self, responses: list[_FakeResponse]) -> None: + def __init__(self, responses: List[_FakeResponse]) -> None: self._responses = list(responses) - self.requests: list[tuple[str, str, dict[str, Any]]] = [] + self.requests: List[Tuple[str, str, Dict[str, Any]]] = [] def send_request(self, method: str, path: str, **kwargs: Any) -> _FakeResponse: self.requests.append((method, path, kwargs)) @@ -64,14 +64,14 @@ def send_request(self, method: str, path: str, **kwargs: Any) -> _FakeResponse: @pytest.fixture -def fake_cache(monkeypatch: pytest.MonkeyPatch) -> type[_FakeContentCache]: +def fake_cache(monkeypatch: pytest.MonkeyPatch) -> Type[_FakeContentCache]: _FakeContentCache._store = {} monkeypatch.setattr(mirror, "ContentCache", _FakeContentCache) return _FakeContentCache def _install_fake_http( - monkeypatch: pytest.MonkeyPatch, responses: list[_FakeResponse] + monkeypatch: pytest.MonkeyPatch, responses: List[_FakeResponse] ) -> _FakeHTTPClient: http_client = _FakeHTTPClient(responses) monkeypatch.setattr( @@ -81,7 +81,7 @@ def _install_fake_http( def test_cache_hit_advances_visited_mirrors( - fake_cache: type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch + fake_cache: Type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch ) -> None: # A cached mirror must be recorded as visited on the cache hit path, # otherwise the next iteration would recompute the same cache key and @@ -119,7 +119,7 @@ def test_cache_hit_advances_visited_mirrors( def test_cache_entry_without_mirror_header_falls_through( - fake_cache: type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch + fake_cache: Type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch ) -> None: # Entries written before X-PIO-Mirror was cached lack the field; the # iterator must treat them as unusable and request a fresh redirect so @@ -151,7 +151,7 @@ def test_cache_entry_without_mirror_header_falls_through( def test_head_response_caches_mirror_header( - fake_cache: type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch + fake_cache: Type[_FakeContentCache], monkeypatch: pytest.MonkeyPatch ) -> None: # The iterator must persist X-PIO-Mirror so later runs can tell whether # the cached redirect still names a mirror that has not been tried. From 5eb85697e6675c5535d078fd4b43ad14d12b7c84 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2026 07:52:24 -0500 Subject: [PATCH 5/5] Satisfy pylint and black on mirror iterator tests --- tests/registry/test_mirror.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/registry/test_mirror.py b/tests/registry/test_mirror.py index 90f37954f4..2720add7d3 100644 --- a/tests/registry/test_mirror.py +++ b/tests/registry/test_mirror.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +# pylint: disable=protected-access,redefined-outer-name + import json from typing import Any, Dict, List, Optional, Tuple, Type @@ -173,8 +175,6 @@ def test_head_response_caches_mirror_header( iterator = RegistryFileMirrorIterator(url) next(iterator) - cached = json.loads( - fake_cache._store[ContentCache.key_from_args("head", url, [])] - ) + cached = json.loads(fake_cache._store[ContentCache.key_from_args("head", url, [])]) assert cached["X-PIO-Mirror"] == "mirror-a" assert cached["Location"] == "https://mirror-a.example.com/tool.tar.gz"