From 6b8b08646de13d4110824ccfb4f5172218e8083f Mon Sep 17 00:00:00 2001 From: codechrl Date: Tue, 11 Aug 2026 03:09:51 +0000 Subject: [PATCH] fix(addons): re-download addons with a missing manifest maybe_download_addons() treated an addon as already downloaded whenever its directory existed. A download that fails partway leaves an empty directory behind, which is then trusted on every later launch, so confirm_paths() raises InvalidAddonPath: manifest.json is missing and never recovers. Gate the check on manifest.json presence and rmtree the partial directory on failure. Closes #308. (cherry picked from commit 0a8211969ba9be3f03c21fc1bfb89d6433fc9dcf) --- pythonlib/camoufox/addons.py | 11 ++-- pythonlib/tests/test_addons.py | 95 ++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 pythonlib/tests/test_addons.py diff --git a/pythonlib/camoufox/addons.py b/pythonlib/camoufox/addons.py index 191707d..32f36bb 100644 --- a/pythonlib/camoufox/addons.py +++ b/pythonlib/camoufox/addons.py @@ -1,4 +1,5 @@ import os +import shutil from enum import Enum from multiprocessing import Lock from typing import List, Optional @@ -74,14 +75,16 @@ def maybe_download_addons( # Get the addon path addon_path = get_addon_path(addon.name) - # Check if the addon is already extracted - if os.path.exists(addon_path): + # Check if the addon is already extracted. A bare directory is not + # enough: a failed download leaves an empty dir behind, so require the + # manifest that confirm_paths() looks for. + if os.path.exists(os.path.join(addon_path, 'manifest.json')): # Add the existing addon path to addons_list if addons_list is not None: addons_list.append(addon_path) continue - # Addon doesn't exist, create directory and download + # Addon isn't extracted, create directory and download try: os.makedirs(addon_path, exist_ok=True) download_and_extract(addon.value, addon_path, addon.name) @@ -89,4 +92,6 @@ def maybe_download_addons( if addons_list is not None: addons_list.append(addon_path) except Exception as e: + # Drop the partial directory so the next run re-downloads. + shutil.rmtree(addon_path, ignore_errors=True) print(f"Failed to download and extract {addon.name}: {e}") diff --git a/pythonlib/tests/test_addons.py b/pythonlib/tests/test_addons.py new file mode 100644 index 0000000..ecef4ca --- /dev/null +++ b/pythonlib/tests/test_addons.py @@ -0,0 +1,95 @@ +""" +Tests for camoufox.addons default-addon download/caching. + +Regression guard for #308: a partial/failed first download leaves an empty +addon directory behind. The old "already downloaded" check was a bare +os.path.exists(dir), so that empty dir was trusted forever and every later +launch raised InvalidAddonPath ("manifest.json is missing"), unrecoverable +short of manually deleting the cache. + +Run with: + cd pythonlib && python -m pytest tests/test_addons.py -v +""" + +import os +import sys + +import pytest + +# Make `import camoufox` resolve to the in-tree pythonlib without an install. +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from camoufox import addons as addons_mod # noqa: E402 +from camoufox.addons import DefaultAddons, maybe_download_addons # noqa: E402 + +UBO = DefaultAddons.UBO.name + + +@pytest.fixture +def addons_dir(tmp_path, monkeypatch): + # Point the addon store at a throwaway dir so no real cache is touched. + root = tmp_path / "addons" + monkeypatch.setattr(addons_mod, "get_addon_path", lambda name: str(root / name)) + return root + + +def _write_manifest(url, extract_path, name): + os.makedirs(extract_path, exist_ok=True) + with open(os.path.join(extract_path, "manifest.json"), "w") as f: + f.write("{}") + + +def test_partial_dir_is_redownloaded(addons_dir, monkeypatch): + # Leftover empty dir from a failed first download. + partial = addons_dir / UBO + partial.mkdir(parents=True) + assert not (partial / "manifest.json").exists() + + calls = [] + + def fake(url, extract_path, name): + calls.append(name) + _write_manifest(url, extract_path, name) + + monkeypatch.setattr(addons_mod, "download_and_extract", fake) + + out = [] + maybe_download_addons([DefaultAddons.UBO], out) + + # An empty dir must trigger a re-download, not be trusted. + assert calls == [UBO] + assert (partial / "manifest.json").exists() + assert out == [str(partial)] + + +def test_extracted_addon_is_not_redownloaded(addons_dir, monkeypatch): + path = addons_dir / UBO + path.mkdir(parents=True) + (path / "manifest.json").write_text("{}") + + def boom(*a, **k): + raise AssertionError("must not re-download an already-extracted addon") + + monkeypatch.setattr(addons_mod, "download_and_extract", boom) + + out = [] + maybe_download_addons([DefaultAddons.UBO], out) + assert out == [str(path)] + + +def test_failed_download_removes_partial_dir(addons_dir, monkeypatch): + path = addons_dir / UBO + + def fail(url, extract_path, name): + os.makedirs(extract_path, exist_ok=True) # partial write, then die + raise RuntimeError("network died mid-download") + + monkeypatch.setattr(addons_mod, "download_and_extract", fail) + + out = [] + maybe_download_addons([DefaultAddons.UBO], out) + + # The partial dir must be gone so the next run re-downloads instead of + # trusting an addon that has no manifest.json. + assert not path.exists() + assert out == []