diff --git a/docs/changelog/197.bugfix.rst b/docs/changelog/197.bugfix.rst new file mode 100644 index 0000000..72a85c7 --- /dev/null +++ b/docs/changelog/197.bugfix.rst @@ -0,0 +1 @@ +Skip interpreter candidates that return JSON without usable interpreter metadata and continue searching. diff --git a/src/python_discovery/_cached_py_info.py b/src/python_discovery/_cached_py_info.py index 183ad7e..bb640e9 100644 --- a/src/python_discovery/_cached_py_info.py +++ b/src/python_discovery/_cached_py_info.py @@ -3,7 +3,6 @@ from __future__ import annotations import hashlib -import json import logging import os import pkgutil @@ -204,7 +203,7 @@ def _load_cached_py_info( try: py_info = cls.from_dict(content.copy()) stale = (sys_exe := py_info.system_executable) is not None and not Path(sys_exe).is_file() - except (AttributeError, KeyError, OSError, TypeError): + except (AttributeError, KeyError, OSError, TypeError, ValueError): stale = True if stale: py_info_store.remove() @@ -291,13 +290,18 @@ def _run_subprocess( if code != 0: return _query_failure(exe, out, err, code), None out, raw_out, out_starts, out_ends = _extract_between_cookies(out, start_cookie, end_cookie) + if out_starts < 0 or out_ends < 0: + _LOGGER.warning("subprocess %s omitted query markers; stdout %d chars", exe, len(raw_out)) + msg = f"{exe} returned output without query markers" + return RuntimeError(msg), None try: result = cls.from_json(out) if executable_args is None: result.executable = exe - except json.JSONDecodeError as exc: + except (AttributeError, KeyError, TypeError, ValueError) as exc: _LOGGER.warning( - "subprocess %s returned invalid JSON; raw stdout %d chars, start cookie %s, end cookie %s, " + "subprocess %s returned invalid interpreter information; " + "raw stdout %d chars, start cookie %s, end cookie %s, " "parsed output %d chars: %r", exe, len(raw_out), @@ -306,7 +310,7 @@ def _run_subprocess( len(out), out[:200] if out else "", ) - msg = f"{exe} returned invalid JSON (exit code {code}){f', stderr: {err!r}' if err else ''}" + msg = f"{exe} returned invalid interpreter information (exit code {code}){f', stderr: {err!r}' if err else ''}" failure = RuntimeError(msg) failure.__cause__ = exc return failure, None diff --git a/src/python_discovery/_py_info.py b/src/python_discovery/_py_info.py index 4885032..1c9611e 100644 --- a/src/python_discovery/_py_info.py +++ b/src/python_discovery/_py_info.py @@ -446,6 +446,9 @@ def from_dict(cls, data: dict[str, InfoValue]) -> PythonInfo: :param data: dictionary produced by :meth:`to_dict`. """ + if missing := PythonInfo.__annotations__.keys() - data.keys() - {"_cache_exe_discovery", "pypy_version_info"}: + msg = f"missing interpreter information: {', '.join(sorted(missing))}" + raise ValueError(msg) data["version_info"] = VersionInfo(**data["version_info"]) # restore this to a named tuple structure result = cls.__new__(cls) # skip __init__, data replaces the full state result.__dict__ = data.copy() diff --git a/tests/test_cached_py_info.py b/tests/test_cached_py_info.py index 1455d46..3dae27e 100644 --- a/tests/test_cached_py_info.py +++ b/tests/test_cached_py_info.py @@ -101,6 +101,37 @@ def test_run_subprocess_invalid_json(mocker: MockerFixture) -> None: assert result is None +@pytest.mark.parametrize( + "output", + [ + pytest.param("{}", id="empty-object"), + pytest.param( + '{"version_info": {"major": 3, "minor": 11, "micro": 0, "releaselevel": "final", "serial": 0}}', + id="version-only", + ), + pytest.param("[]", id="array"), + ], +) +def test_from_exe_rejects_wrong_shape_json(tmp_path: Path, mocker: MockerFixture, output: str) -> None: + start_cookie, end_cookie = "a" * 32, "b" * 32 + wrapped = f"{start_cookie}{output}{end_cookie}" + mocker.patch( + "python_discovery._cached_py_info.Popen", + side_effect=[_mock_process(wrapped, "", 0), _mock_process(wrapped, "", 0)], + ) + mocker.patch("python_discovery._cached_py_info.gen_cookie", side_effect=[start_cookie, end_cookie] * 2) + assert PythonInfo.from_exe(str(tmp_path / "python"), raise_on_error=False, ignore_cache=True) is None + + +def test_from_exe_rejects_json_without_cookies(tmp_path: Path, mocker: MockerFixture) -> None: + output = PythonInfo().to_json() + mocker.patch( + "python_discovery._cached_py_info.Popen", + side_effect=[_mock_process(output, "", 0), _mock_process(output, "", 0)], + ) + assert PythonInfo.from_exe(str(tmp_path / "python"), raise_on_error=False, ignore_cache=True) is None + + def _mock_process(out: str, err: str, code: int) -> MagicMock: streams: list[IO[str]] = [] for value in (out, err): diff --git a/tests/test_discovery.py b/tests/test_discovery.py index 0a51da3..4ef500b 100644 --- a/tests/test_discovery.py +++ b/tests/test_discovery.py @@ -118,6 +118,39 @@ def test_generic_python_spec_accepts_mixed_case_alias(tmp_path: Path, session_ca assert (selected.executable, selected.implementation) == (str(alias), "CPython") +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX shell script") +@pytest.mark.parametrize( + "output", + [ + pytest.param("{}", id="empty-object"), + pytest.param( + '{"version_info": {"major": 3, "minor": 11, "micro": 0, "releaselevel": "final", "serial": 0}}', + id="version-only", + ), + pytest.param("[]", id="array"), + ], +) +def test_discovery_skips_candidate_with_wrong_shape_json(tmp_path: Path, output: str) -> None: + first, second = tmp_path / "first", tmp_path / "second" + first.mkdir() + second.mkdir() + name = "python_custom" + fake = first / name + fake.write_text(f"#!/bin/sh\nprintf '%s\\n' '{output}'\n", encoding="utf-8") + fake.chmod(0o755) + real = second / name + real.symlink_to(sys.executable) + + interpreter = get_interpreter( + name, + cache=DiskCache(tmp_path / "cache"), + env={"PATH": os.pathsep.join([str(first), str(second)]), "HOME": "/nonexistent"}, + ) + + assert interpreter is not None + assert interpreter.executable == str(real) + + def test_discovery_via_path_in_nonbrowseable_directory(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: bad_perm = tmp_path / "bad_perm" bad_perm.mkdir(mode=0o000)