From 8f266a49486b22b2016550866a9c1ae0351eb209 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Thu, 6 Aug 2026 20:00:47 +0500 Subject: [PATCH 1/3] fix(integrations): wrap a non-UTF-8 catalog response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_fetch_single_catalog` decodes the response body with `.decode("utf-8")` before handing it to `json.loads`. A non-UTF-8 body therefore raises `UnicodeDecodeError`, which is a sibling of `json.JSONDecodeError` under `ValueError` rather than a subclass of it, so neither the `URLError` nor the `JSONDecodeError` handler catches it. The raw exception escapes `_get_merged_integrations`, whose `except IntegrationCatalogError` is specifically designed to warn and skip a bad catalog and carry on with the remaining ones. One catalog served over a misconfigured proxy or truncated mid-multibyte-sequence thus takes down `specify integration search` entirely instead of degrading to a warning. Wrap it in `IntegrationCatalogError`, matching the convention already used for the same decode in `authentication/azure_devops.py`, which lists `UnicodeDecodeError` alongside `JSONDecodeError`. Note that the cache-read path in this same method already tolerates this via its `UnicodeError` clause; only the network path was unguarded. Two regression tests: one pins the wrapped-error contract on the fetch, and one covers the behaviour that actually motivates it — a broken catalog is skipped with a warning while a healthy sibling catalog still resolves. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/integrations/catalog.py | 9 ++ .../integrations/test_integration_catalog.py | 117 ++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index b3be8a84e3..e18d30a6fa 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -237,6 +237,15 @@ def _fetch_single_catalog( raise IntegrationCatalogError( f"Failed to fetch catalog from {entry.url}: {exc}" ) + except UnicodeDecodeError as exc: + # A non-UTF-8 response body fails at .decode() before json.loads() + # ever runs, so JSONDecodeError below does not cover it (the two are + # sibling ValueError subclasses, not parent/child). Without this the + # raw UnicodeDecodeError escapes _get_merged_integrations()'s + # "warn and skip this catalog" handler and kills the whole command. + raise IntegrationCatalogError( + f"Catalog from {entry.url} is not valid UTF-8: {exc}" + ) except json.JSONDecodeError as exc: raise IntegrationCatalogError( f"Invalid JSON in catalog from {entry.url}: {exc}" diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index 68e8970c42..c8d67356f7 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -392,6 +392,123 @@ def fake_urlopen(req, timeout=10): with pytest.raises(IntegrationCatalogError, match="exceeds maximum size"): cat._fetch_single_catalog(entry, force_refresh=True) + def _patch_open_url_bytes(self, monkeypatch, bodies): + """Patch ``open_url`` to serve raw *bodies* keyed by URL substring. + + The catalog fetch decodes the response itself, so these stubs yield raw + bytes rather than the JSON-encoded payloads ``_patch_urlopen`` produces. + """ + + class _RawResponse: + def __init__(self, data, url): + self._data = data + self._url = url + self._offset = 0 + + def read(self, size=-1): + if size == -1: + chunk = self._data[self._offset:] + self._offset = len(self._data) + else: + chunk = self._data[self._offset:self._offset + size] + self._offset += len(chunk) + return chunk + + def geturl(self): + return self._url + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + def fake_open_url(url, timeout=10, **kwargs): + for marker, body in bodies.items(): + if marker in url: + return _RawResponse(body, url) + raise AssertionError(f"unexpected URL requested: {url}") + + import specify_cli.authentication.http as _auth_http + monkeypatch.setattr(_auth_http, "open_url", fake_open_url) + + def test_fetch_wraps_non_utf8_catalog_response(self, tmp_path, monkeypatch): + """Regression: a non-UTF-8 response body must raise IntegrationCatalogError. + + ``.decode("utf-8")`` runs before ``json.loads``, so the resulting + UnicodeDecodeError is not a JSONDecodeError and slipped past both + handlers as a raw traceback. + """ + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("USERPROFILE", str(tmp_path)) + monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False) + (tmp_path / ".specify").mkdir(exist_ok=True) + cat = IntegrationCatalog(tmp_path) + + self._patch_open_url_bytes( + monkeypatch, + {"catalog.json": b'{"schema_version": "1.0", "name": "\xff\xfe"}'}, + ) + + entry = IntegrationCatalogEntry( + url="https://example.com/catalog.json", + name="test", + priority=1, + install_allowed=True, + ) + + with pytest.raises(IntegrationCatalogError, match="not valid UTF-8"): + cat._fetch_single_catalog(entry, force_refresh=True) + + def test_search_skips_non_utf8_catalog(self, tmp_path, monkeypatch, capsys): + """A single non-UTF-8 catalog must not take down the whole search. + + ``_get_merged_integrations`` is built to warn and continue on a bad + catalog; an unwrapped UnicodeDecodeError defeated that entirely. + """ + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("USERPROFILE", str(tmp_path)) + monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False) + specify = tmp_path / ".specify" + specify.mkdir(exist_ok=True) + (specify / "integration-catalogs.yml").write_text( + "catalogs:\n" + " - name: broken\n" + " url: https://example.com/broken.json\n" + " priority: 1\n" + " - name: healthy\n" + " url: https://example.com/healthy.json\n" + " priority: 2\n", + encoding="utf-8", + ) + + healthy = json.dumps( + { + "schema_version": "1.0", + "integrations": { + "acme-coder": { + "name": "Acme Coder", + "version": "1.0.0", + "description": "Acme integration", + } + }, + } + ).encode("utf-8") + + self._patch_open_url_bytes( + monkeypatch, + { + "broken.json": b'{"schema_version": "1.0", "name": "\xff\xfe"}', + "healthy.json": healthy, + }, + ) + + cat = IntegrationCatalog(tmp_path) + results = cat.search() + + assert "acme-coder" in [r["id"] for r in results] + assert "broken" in capsys.readouterr().err + def test_search_by_tag(self, tmp_path, monkeypatch): monkeypatch.setenv("HOME", str(tmp_path)) monkeypatch.setenv("USERPROFILE", str(tmp_path)) From 9037d774eb9656ae67451fe565864ec4d019decc Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Fri, 7 Aug 2026 20:50:47 +0500 Subject: [PATCH 2/3] test(integrations): use the shared urlopen routing fixture The raw-bytes helper patched `open_url` wholesale, which skipped the real URL validation and redirect handling inside it. This module already imports `route_opener_open_through_urlopen`, the repo's shared fixture that routes `build_opener().open()` back through `urlopen` for exactly this reason, so patching `urlopen` instead keeps the stub effective while still exercising `open_url` itself. Renamed to `_patch_urlopen_bytes` to sit alongside the existing `_patch_urlopen`, whose signature it now mirrors. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/integrations/catalog.py | 9 --------- tests/integrations/test_integration_catalog.py | 18 ++++++++++-------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index e18d30a6fa..b3be8a84e3 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -237,15 +237,6 @@ def _fetch_single_catalog( raise IntegrationCatalogError( f"Failed to fetch catalog from {entry.url}: {exc}" ) - except UnicodeDecodeError as exc: - # A non-UTF-8 response body fails at .decode() before json.loads() - # ever runs, so JSONDecodeError below does not cover it (the two are - # sibling ValueError subclasses, not parent/child). Without this the - # raw UnicodeDecodeError escapes _get_merged_integrations()'s - # "warn and skip this catalog" handler and kills the whole command. - raise IntegrationCatalogError( - f"Catalog from {entry.url} is not valid UTF-8: {exc}" - ) except json.JSONDecodeError as exc: raise IntegrationCatalogError( f"Invalid JSON in catalog from {entry.url}: {exc}" diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index c8d67356f7..9b02632992 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -392,11 +392,12 @@ def fake_urlopen(req, timeout=10): with pytest.raises(IntegrationCatalogError, match="exceeds maximum size"): cat._fetch_single_catalog(entry, force_refresh=True) - def _patch_open_url_bytes(self, monkeypatch, bodies): - """Patch ``open_url`` to serve raw *bodies* keyed by URL substring. + def _patch_urlopen_bytes(self, monkeypatch, bodies): + """Patch urlopen to serve raw *bodies* keyed by URL substring. - The catalog fetch decodes the response itself, so these stubs yield raw - bytes rather than the JSON-encoded payloads ``_patch_urlopen`` produces. + Mirrors ``_patch_urlopen`` but passes the bytes through verbatim: these + tests need a body that is not valid UTF-8, which ``json.dumps`` cannot + produce. """ class _RawResponse: @@ -423,14 +424,15 @@ def __enter__(self): def __exit__(self, *a): pass - def fake_open_url(url, timeout=10, **kwargs): + def fake_urlopen(req, timeout=10): + url = req if isinstance(req, str) else req.full_url for marker, body in bodies.items(): if marker in url: return _RawResponse(body, url) raise AssertionError(f"unexpected URL requested: {url}") import specify_cli.authentication.http as _auth_http - monkeypatch.setattr(_auth_http, "open_url", fake_open_url) + monkeypatch.setattr(_auth_http.urllib.request, "urlopen", fake_urlopen) def test_fetch_wraps_non_utf8_catalog_response(self, tmp_path, monkeypatch): """Regression: a non-UTF-8 response body must raise IntegrationCatalogError. @@ -445,7 +447,7 @@ def test_fetch_wraps_non_utf8_catalog_response(self, tmp_path, monkeypatch): (tmp_path / ".specify").mkdir(exist_ok=True) cat = IntegrationCatalog(tmp_path) - self._patch_open_url_bytes( + self._patch_urlopen_bytes( monkeypatch, {"catalog.json": b'{"schema_version": "1.0", "name": "\xff\xfe"}'}, ) @@ -495,7 +497,7 @@ def test_search_skips_non_utf8_catalog(self, tmp_path, monkeypatch, capsys): } ).encode("utf-8") - self._patch_open_url_bytes( + self._patch_urlopen_bytes( monkeypatch, { "broken.json": b'{"schema_version": "1.0", "name": "\xff\xfe"}', From a0e469b4f5d97b906f5757c1e8ab7a80133007be Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Fri, 7 Aug 2026 21:24:51 +0500 Subject: [PATCH 3/3] fix(integrations): restore the non-UTF-8 handler The previous commit reverted the source change by accident while reworking the tests, leaving the regression tests passing against an unfixed module. Restores the `except UnicodeDecodeError` clause. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/integrations/catalog.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index b3be8a84e3..e18d30a6fa 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -237,6 +237,15 @@ def _fetch_single_catalog( raise IntegrationCatalogError( f"Failed to fetch catalog from {entry.url}: {exc}" ) + except UnicodeDecodeError as exc: + # A non-UTF-8 response body fails at .decode() before json.loads() + # ever runs, so JSONDecodeError below does not cover it (the two are + # sibling ValueError subclasses, not parent/child). Without this the + # raw UnicodeDecodeError escapes _get_merged_integrations()'s + # "warn and skip this catalog" handler and kills the whole command. + raise IntegrationCatalogError( + f"Catalog from {entry.url} is not valid UTF-8: {exc}" + ) except json.JSONDecodeError as exc: raise IntegrationCatalogError( f"Invalid JSON in catalog from {entry.url}: {exc}"