From 31d3cd4b46f4130907e201500d8974ce1e5d2bfd Mon Sep 17 00:00:00 2001 From: Migael Date: Thu, 3 Sep 2026 16:50:13 +0200 Subject: [PATCH] fix(dev-admin): a version check that did not happen says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The endpoint answered HTTP 200 with latest == current whenever the call to PyPI failed, and the toolbar renders that as a green "Latest: vX — You are up to date!". A developer several releases behind, on a machine with no route out, was told the opposite of the truth. The toolbar already had the right message for this — "Could not check for updates (offline?)" on fetch's .catch — and could never reach it, because the server turned the failure into a success. latest is now None when the check could not be made, with a short error beside it, and the toolbar reads that before it compares versions. Reaching PyPI and getting a body with no version in it goes down the same path: it is the same lie by another route. The old test called PyPI for real and asserted latest was always a string, which is the bug written down as an expectation. It is now stubbed, and joined by the offline case, the unreadable-answer case, and one that holds the client to branching on the null before the up-to-date comparison. Verified end to end: a project pinned to 3.13.125 answers latest 3.13.131 with the network up, and {"latest": null, "error": ...} with the registry unreachable. It used to answer 3.13.125 both times. --- tests/test_dev_admin.py | 100 +++++++++++++++++++++++++++-- tina4_python/dev_admin/__init__.py | 43 +++++++++++-- 2 files changed, 134 insertions(+), 9 deletions(-) diff --git a/tests/test_dev_admin.py b/tests/test_dev_admin.py index 7e9d6a07..ba1b5f6e 100644 --- a/tests/test_dev_admin.py +++ b/tests/test_dev_admin.py @@ -402,6 +402,26 @@ def test_version_check_handler_registered(self): assert method == "GET" +def _fake_pypi(payload): + """Stand in for ``urllib.request.urlopen`` with a canned PyPI body.""" + import json as _json + + class _Resp: + def read(self): + return _json.dumps(payload).encode() + + def __enter__(self): + return self + + def __exit__(self, *_): + return False + + def _open(*_args, **_kwargs): + return _Resp() + + return _open + + class TestAPIHandlers: """Test API handler functions with mock request/response.""" @@ -738,13 +758,85 @@ async def test_connections_save_preserves_tina4_prefix( assert "OTHER_VAR=keep-me" in written @pytest.mark.asyncio - async def test_version_check_handler(self, mock_req, mock_resp): + async def test_version_check_handler(self, mock_req, mock_resp, monkeypatch): + """A successful check reports what PyPI published. + + Stubbed rather than live: the old version of this test called PyPI for + real and asserted ``latest`` was always a string, which is exactly the + bug — offline, the handler invented one. + """ + import urllib.request from tina4_python.dev_admin import _api_version_check + + monkeypatch.setattr( + urllib.request, "urlopen", _fake_pypi({"info": {"version": "9.9.9"}}) + ) result = await _api_version_check(mock_req, mock_resp) - assert "current" in result - assert "latest" in result assert isinstance(result["current"], str) - assert isinstance(result["latest"], str) + assert result["latest"] == "9.9.9" + assert "error" not in result + + @pytest.mark.asyncio + async def test_version_check_reports_a_failure_instead_of_claiming_up_to_date( + self, mock_req, mock_resp, monkeypatch + ): + """The toolbar renders ``latest == current`` as "You are up to date!". + + So a check that never reached PyPI must not answer with it. A developer + several releases behind, on a machine with no route out, was being told + the opposite of the truth. + """ + import urllib.error + import urllib.request + from tina4_python.dev_admin import _api_version_check + + def explode(*_args, **_kwargs): + raise urllib.error.URLError("no route to host") + + monkeypatch.setattr(urllib.request, "urlopen", explode) + result = await _api_version_check(mock_req, mock_resp) + + assert result["latest"] is None, ( + "a check that did not happen must not answer with a version" + ) + assert result["latest"] != result["current"] + assert result.get("error"), "the reason has to reach the client" + + @pytest.mark.asyncio + async def test_version_check_does_not_invent_a_version_from_an_unreadable_answer( + self, mock_req, mock_resp, monkeypatch + ): + """Reaching PyPI is not the same as learning the version. + + An answer with no version field used to fall back to ``current`` down + the same path as being offline, which is the same lie by another route. + """ + import urllib.request + from tina4_python.dev_admin import _api_version_check + + monkeypatch.setattr(urllib.request, "urlopen", _fake_pypi({"info": {}})) + result = await _api_version_check(mock_req, mock_resp) + + assert result["latest"] is None + assert result.get("error") + + def test_toolbar_reads_a_missing_latest_as_a_failed_check(self): + """The client half of the same defect. + + The toolbar already had a "Could not check for updates (offline?)" + branch, on ``fetch``'s ``.catch``, which could never fire because the + server answered 200. It has to act on the null the server now sends. + """ + from tina4_python.dev_admin import toolbar_js + + js = toolbar_js() + assert "couldNotCheck" in js, "no branch for a check that did not happen" + assert "if (!latest) { couldNotCheck" in js, ( + "a missing latest must be handled before the up-to-date comparison" + ) + assert js.index("if (!latest)") < js.index("if (latest === current)"), ( + "the up-to-date branch must not run first — null would fall into it" + ) @pytest.mark.asyncio async def test_query_multi_statement_rollback(self, mock_req, mock_resp, tmp_path, monkeypatch): diff --git a/tina4_python/dev_admin/__init__.py b/tina4_python/dev_admin/__init__.py index 273e2f59..a4e91454 100644 --- a/tina4_python/dev_admin/__init__.py +++ b/tina4_python/dev_admin/__init__.py @@ -1933,10 +1933,20 @@ def _route_count() -> int: async def _api_version_check(request, response): - """Proxy version check to PyPI to avoid browser CORS errors.""" + """Proxy the version check to PyPI to avoid browser CORS errors. + + A check that did not happen says so. This used to fall back to + ``latest = current`` on any failure, which the toolbar renders as a green + "You are up to date!" — so a developer several releases behind, on a + machine with no route out, was told the opposite of the truth. The toolbar + already had the right message for this case and could never reach it, + because the failure arrived as a success. + + ``latest`` is ``None`` when the check could not be made, and ``error`` + carries the short reason. + """ import urllib.request current = __version__ - latest = current try: req = urllib.request.Request( "https://pypi.org/pypi/tina4-python/json", @@ -1944,9 +1954,24 @@ async def _api_version_check(request, response): ) with urllib.request.urlopen(req, timeout=5) as resp: data = json.loads(resp.read().decode()) - latest = data.get("info", {}).get("version", current) - except Exception: - pass # Offline or timeout — return current as latest + latest = data.get("info", {}).get("version") + if not latest: + # Reached PyPI, got something we cannot read a version out of. + return response( + { + "current": current, + "latest": None, + "error": "PyPI did not report a version", + } + ) + except Exception as exc: + return response( + { + "current": current, + "latest": None, + "error": "{}: {}".format(type(exc).__name__, exc), + } + ) return response({"current": current, "latest": latest}) @@ -2139,6 +2164,13 @@ def toolbar_js() -> str: el.className = 't4-ok'; el.innerHTML = 'Latest: v' + latest + ' — You are up to date!'; } + // A check that did not happen is not a clean bill of health. The server + // sends latest: null when it could not reach PyPI, and saying so is the + // whole point -- "up to date" here would be a guess dressed as a fact. + function couldNotCheck(el, why) { + el.className = 't4-err'; + el.textContent = 'Could not check for updates' + (why ? ' (' + why + ')' : ''); + } function checkVersion() { if (modal.style.display === 'block') { modal.style.display = 'none'; return; } modal.style.display = 'block'; @@ -2147,6 +2179,7 @@ def toolbar_js() -> str: el.textContent = 'Checking for updates...'; fetch('/__dev/api/version-check').then(function (r) { return r.json(); }).then(function (d) { var latest = d.latest, current = d.current; + if (!latest) { couldNotCheck(el, d.error); return; } if (latest === current) { upToDate(el, latest); return; } var cP = current.split('.').map(Number), lP = latest.split('.').map(Number); var isNewer = false, i, c, l;