diff --git a/comfy_cli/comfy_client.py b/comfy_cli/comfy_client.py index 1723afe4..b360db06 100644 --- a/comfy_cli/comfy_client.py +++ b/comfy_cli/comfy_client.py @@ -24,10 +24,9 @@ from typing import Any from comfy_cli.http import NoRedirectHandler, build_http_only_opener, target_auth_headers +from comfy_cli.http import assert_safe_url as _assert_safe_url from comfy_cli.target import Target -_LOOPBACK_HOSTS = {"localhost", "127.0.0.1", "::1", "[::1]"} - # Transient HTTP failures during polling should back off and retry, not abort. # 429 (rate limit) is retried for any method — the request was rejected, not # processed, so even a POST is safe to repeat. Transient 5xx is retried for @@ -98,24 +97,6 @@ class Unauthenticated(Exception): _OPENER = build_http_only_opener(NoRedirectHandler()) -def _assert_safe_url(url: str) -> None: - """Reject plaintext HTTP for non-loopback hosts. - - Anything carrying a Bearer token over the wire must be HTTPS unless the - host is a loopback address (where there's no network to sniff). - """ - parsed = urllib.parse.urlsplit(url) - if parsed.scheme == "https": - return - host = (parsed.hostname or "").lower() - if host in _LOOPBACK_HOSTS: - return - raise ValueError( - f"refusing to send request to non-https, non-loopback URL: {url} " - "(set COMFY_CLOUD_BASE_URL to an https:// endpoint)" - ) - - @dataclass class SubmitResult: prompt_id: str diff --git a/comfy_cli/command/models/search.py b/comfy_cli/command/models/search.py index 0ab8b0ce..d8a797b2 100644 --- a/comfy_cli/command/models/search.py +++ b/comfy_cli/command/models/search.py @@ -34,7 +34,7 @@ import typer from comfy_cli import tracking -from comfy_cli.http import authed_urlopen +from comfy_cli.http import ResponseTooLarge from comfy_cli.output import get_renderer, rprint from comfy_cli.output.sanitize import sanitize_markup @@ -143,16 +143,17 @@ def _http_get_json(url: str, target, timeout: float = 30.0) -> Any: """Issue an authenticated GET and decode JSON. Raises urllib/JSON errors verbatim. Response body is capped at ``_MAX_RESPONSE_BYTES`` to bound memory use on a - misbehaving server. A ``ValueError`` is raised if the cap is exceeded. + misbehaving server; exceeding it raises ``ResponseTooLarge``, which every + caller routes to an envelope error alongside the urllib/JSON families. """ - with authed_urlopen(url, target, timeout=timeout) as resp: - # ``read(N)`` returns up to N bytes; reading N+1 lets us distinguish - # "fits exactly" from "exceeds cap" without buffering the whole stream - # twice on the happy path. - body = resp.read(_MAX_RESPONSE_BYTES + 1) - if len(body) > _MAX_RESPONSE_BYTES: - raise ValueError(f"response from {url} exceeds {_MAX_RESPONSE_BYTES} byte cap") - return json.loads(body) + from comfy_cli.http import request_json + + _, body = request_json(url, target, timeout=timeout, max_bytes=_MAX_RESPONSE_BYTES) + if body is None: + # Callers route JSONDecodeError to an envelope error; an empty or + # unparseable body must surface the same way, not crash on body.get(). + raise json.JSONDecodeError("empty or unparseable response body", "", 0) + return body def _emit_http_error(e: urllib.error.HTTPError, *, renderer, target, message: str, hint: str) -> NoReturn: @@ -208,7 +209,7 @@ def list_folders_cmd( if target.is_cloud else "run `comfy launch` to start a local server", ) - except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: + except (urllib.error.URLError, OSError, json.JSONDecodeError, ResponseTooLarge) as e: renderer.error( code="server_not_running" if not target.is_cloud else "cloud_http_error", message=f"failed to fetch {url}: {e}", @@ -301,7 +302,7 @@ def list_folder_cmd( details={"status": e.code, "folder": folder}, ) raise typer.Exit(code=1) from e - except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: + except (urllib.error.URLError, OSError, json.JSONDecodeError, ResponseTooLarge) as e: renderer.error( code="cloud_http_error" if target.is_cloud else "server_not_running", message=f"failed to fetch {url}: {e}", @@ -425,6 +426,11 @@ def _cloud_search( qs = urllib.parse.urlencode(params) url = target.url("assets") + "?" + qs body = _http_get_json(url, target) + if not isinstance(body, dict): + # Callers route JSONDecodeError to an envelope error; a non-object + # top-level body (list/scalar) must surface the same way, not crash + # on body.get(). + raise json.JSONDecodeError(f"unexpected response shape (not an object) from {url}", "", 0) assets = body.get("assets") or [] rows = [_asset_to_row(a) for a in assets if isinstance(a, dict)] return rows, int(body.get("total") or len(rows)) @@ -591,7 +597,7 @@ def search_cmd( message=f"HTTP {e.code} during models search", hint="check auth (`comfy cloud whoami`) or network", ) - except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: + except (urllib.error.URLError, OSError, json.JSONDecodeError, ResponseTooLarge) as e: renderer.error( code="cloud_http_error" if target.is_cloud else "server_not_running", message=f"models search failed: {e}", @@ -674,6 +680,8 @@ def show_cmd( url = target.url("assets") + "?" + qs try: body = _http_get_json(url, target) + if not isinstance(body, dict): + raise json.JSONDecodeError(f"unexpected response shape (not an object) from {url}", "", 0) except urllib.error.HTTPError as e: renderer.error( code="cloud_http_error", @@ -682,7 +690,7 @@ def show_cmd( details={"status": e.code}, ) raise typer.Exit(code=1) from e - except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: + except (urllib.error.URLError, OSError, json.JSONDecodeError, ResponseTooLarge) as e: renderer.error(code="cloud_http_error", message=f"models show failed: {e}") raise typer.Exit(code=1) from e diff --git a/comfy_cli/command/workflow.py b/comfy_cli/command/workflow.py index e437ab93..a0b41422 100644 --- a/comfy_cli/command/workflow.py +++ b/comfy_cli/command/workflow.py @@ -28,6 +28,12 @@ import typer from comfy_cli import tracking + +# Aliased at module scope rather than lazy-imported: a class used in ``except`` +# clauses at module scope cannot be resolved lazily. ``comfy_cli.http`` is +# stdlib-only and tiny, and ``search.py``/``jobs.py`` already pull +# ``urllib.request`` in at import time, so the precedent exists. +from comfy_cli.http import ResponseTooLarge as _ResponseTooLarge from comfy_cli.output import get_renderer, rprint app = typer.Typer(no_args_is_help=True, help="Slot-based editing of frontend-format ComfyUI workflows.") @@ -605,10 +611,6 @@ def notes_cmd( _HTTP_MAX_BYTES = 64 * 1024 * 1024 -class _ResponseTooLarge(Exception): - """A response exceeded the surface's byte cap — refuse to truncate.""" - - # Per-operation guidance for an oversize cloud response. ``save``/``delete`` # have already sent their request by the time the response is read, so the # server-side write may well have landed — say so rather than implying it did not. @@ -815,7 +817,7 @@ def _userdata_file_url(target, key: str, query: dict | None = None) -> str: def _http_request( url: str, target, *, method: str = "GET", body: dict | None = None, timeout: float = 30.0 -) -> tuple[int, dict | None]: +) -> tuple[int, dict | list | None]: """Authed HTTP call returning (status, parsed_json_or_none). Raises urllib errors verbatim so callers can surface the right error code, and ``_ResponseTooLarge`` when the body exceeds ``_HTTP_MAX_BYTES`` — an diff --git a/comfy_cli/http.py b/comfy_cli/http.py index a894afdb..9fececb5 100644 --- a/comfy_cli/http.py +++ b/comfy_cli/http.py @@ -1,8 +1,31 @@ """Shared HTTP helpers with an auth-leak-safe redirect policy.""" +import json import urllib.error +import urllib.parse import urllib.request +_LOOPBACK_HOSTS = {"localhost", "127.0.0.1", "::1", "[::1]"} + + +def assert_safe_url(url: str) -> None: + """Reject plaintext HTTP for non-loopback hosts. + + Anything carrying a credential (``X-API-Key`` / Bearer token) over the + wire must be HTTPS unless the host is a loopback address (where there's + no network to sniff). + """ + parsed = urllib.parse.urlsplit(url) + if parsed.scheme == "https": + return + host = (parsed.hostname or "").lower() + if host in _LOOPBACK_HOSTS: + return + raise ValueError( + f"refusing to send request to non-https, non-loopback URL: {url} " + "(set COMFY_CLOUD_BASE_URL to an https:// endpoint)" + ) + class NoRedirectHandler(urllib.request.HTTPRedirectHandler): """Refuse to follow HTTP redirects. @@ -180,3 +203,57 @@ def authed_urlopen( HTTPError instead of replaying credentials at the redirect target.""" req = build_authed_request(url, target, method=method, data=data, content_type=content_type) return _AUTHED_OPENER.open(req, timeout=timeout) + + +class ResponseTooLarge(Exception): + """A response exceeded the caller's byte cap — refuse to truncate.""" + + +def request_json( + url: str, + target, + *, + method: str = "GET", + body: dict | None = None, + timeout: float = 30.0, + max_bytes: int, +) -> tuple[int, dict | list | None]: + """Authed HTTP call returning (status, parsed_json_or_none). + + Raises urllib errors verbatim so callers can map them to envelope codes, + and ``ResponseTooLarge`` when the body exceeds ``max_bytes`` — an oversize + body must not masquerade as an unparseable one. An empty or unparseable + (bad JSON / non-UTF-8 / too-deeply-nested) body parses to ``None``; + ``UnicodeDecodeError`` is a ``ValueError`` but *not* a ``JSONDecodeError``, + so it needs naming here or it escapes uncaught. + + ``max_bytes`` is keyword-required with no default so every caller keeps + owning its own cap constant. Auth headers never go out over the wire + without this: redirects are refused via the shared no-redirect opener (a + 30x can't replay the credential at another host), and the URL itself + must be HTTPS or loopback before a credential is attached. + """ + if max_bytes < 1: + raise ValueError(f"max_bytes must be >= 1, got {max_bytes}") + headers = target_auth_headers(target) + if headers: + assert_safe_url(url) + data = json.dumps(body).encode("utf-8") if body is not None else None + req = urllib.request.Request(url, data=data, method=method) + for k, v in headers.items(): + req.add_header(k, v) + if data is not None: + req.add_header("Content-Type", "application/json") + with _AUTHED_OPENER.open(req, timeout=timeout) as resp: + status = resp.status + # Read one byte past the cap so a full body is distinguishable from a truncated one. + raw = resp.read(max_bytes + 1) + if len(raw) > max_bytes: + # Keep the message descriptive: search interpolates it into its envelope. + raise ResponseTooLarge(f"response from {url} exceeds {max_bytes} byte cap") + if not raw: + return status, None + try: + return status, json.loads(raw) + except (json.JSONDecodeError, UnicodeDecodeError, RecursionError): + return status, None diff --git a/tests/comfy_cli/command/models/test_search.py b/tests/comfy_cli/command/models/test_search.py index 72f23a5d..35d479de 100644 --- a/tests/comfy_cli/command/models/test_search.py +++ b/tests/comfy_cli/command/models/test_search.py @@ -203,7 +203,9 @@ def _patch_urlopen(monkeypatch: pytest.MonkeyPatch, routes: dict[str, Any]): """Wire urlopen to a URL→body lookup. Body is JSON-encoded. Substring matching: the first registered URL substring that matches wins. - Unknown URLs raise so we never silently pass on a typo'd path. + Unknown URLs raise so we never silently pass on a typo'd path. A ``bytes`` + payload is served verbatim, so a test can hand back a body that is not + valid JSON (or not valid UTF-8) at all. """ calls = [] @@ -214,13 +216,14 @@ def _fake(req, timeout=None): if needle in url: if isinstance(payload, Exception): raise payload - body = json.dumps(payload).encode() + body = payload if isinstance(payload, bytes) else json.dumps(payload).encode() return _fake_resp(body) raise AssertionError(f"unexpected URL hit by mock: {url}") - # ``_http_get_json`` now opens through the shared no-redirect opener in - # ``comfy_cli.http``; the fake still receives a ``Request`` object, so the - # route-matching above is unchanged. + # ``_http_get_json`` routes every request through the shared + # ``comfy_cli.http.request_json``, which opens via the module's + # ``_AUTHED_OPENER`` (built with NoRedirectHandler); the fake still + # receives a ``Request`` object, so the route-matching above is unchanged. import comfy_cli.http as http_mod monkeypatch.setattr(http_mod._AUTHED_OPENER, "open", _fake) @@ -273,6 +276,82 @@ def test_local_http_error_uses_server_not_running(self, local_target, monkeypatc assert env["error"]["details"]["body"] == "boom" +class TestMalformedResponseEnvelopes: + """Regression: an oversize or undecodable body must be an envelope, not a traceback. + + Before the shared ``request_json`` migration, ``_http_get_json`` raised a + bare ``ValueError`` past the cap and let ``UnicodeDecodeError`` escape from + ``json.loads`` — neither is in the callers' ``(URLError, OSError, + JSONDecodeError)`` tuple, so both crashed the CLI with a traceback and no + ``--json`` envelope at all. + """ + + def test_oversize_response_yields_envelope_not_traceback(self, cloud_target, monkeypatch, capsys): + monkeypatch.setattr(search_cmd, "_MAX_RESPONSE_BYTES", 4) + _patch_urlopen(monkeypatch, {"/api/experiment/models": _CLOUD_FOLDERS}) + env = _run(["list-folders", "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + # The helper's message is interpolated into the envelope, so it stays informative. + assert "byte cap" in env["error"]["message"] + + def test_oversize_response_local_uses_server_not_running(self, local_target, monkeypatch, capsys): + monkeypatch.setattr(search_cmd, "_MAX_RESPONSE_BYTES", 4) + _patch_urlopen(monkeypatch, {"127.0.0.1:8188/models": _LOCAL_FOLDERS}) + env = _run(["list-folders", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "server_not_running" + + def test_non_utf8_response_yields_envelope_not_traceback(self, cloud_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/api/experiment/models": b"\xff\xfe\x00not json"}) + env = _run(["list-folders", "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + + def test_unparseable_response_yields_envelope_not_traceback(self, cloud_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/api/experiment/models": b"not json"}) + env = _run(["list-folders", "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + + def test_literal_null_body_yields_envelope(self, cloud_target, monkeypatch, capsys): + # The one intentional behavior change of the shared-helper migration: + # ``request_json`` cannot distinguish a literal JSON ``null`` from an + # unparseable body, so both become None and _http_get_json raises. Before, + # `list-folders` reported ok:true with count 0 for a `null` body — which + # silently presented a malformed response as "this backend has no folders". + _patch_urlopen(monkeypatch, {"/api/experiment/models": b"null"}) + env = _run(["list-folders", "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + + def test_empty_response_yields_envelope_not_attribute_error(self, cloud_target, monkeypatch, capsys): + # `models show` calls body.get() on the parsed result; an empty body used + # to reach that as None. It now surfaces as a decode-family envelope. + _patch_urlopen(monkeypatch, {"/api/assets": b""}) + env = _run(["show", "flux1-dev", "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + + @pytest.mark.parametrize( + ("args", "route"), + [ + (["list-folders", "--where", "cloud"], "/api/experiment/models"), + (["list-folder", "loras", "--where", "cloud"], "/api/experiment/models/loras"), + (["search", "--where", "cloud"], "/api/assets"), + (["show", "flux1-dev", "--where", "cloud"], "/api/assets"), + ], + ) + def test_every_call_site_routes_oversize_to_envelope(self, args, route, cloud_target, monkeypatch, capsys): + # Each of the four handlers wrapping _http_get_json must catch + # ResponseTooLarge; an uncaught one would be a traceback, not an envelope. + monkeypatch.setattr(search_cmd, "_MAX_RESPONSE_BYTES", 4) + _patch_urlopen(monkeypatch, {route: {"assets": [], "total": 0}}) + env = _run(args, capsys) + assert env["ok"] is False, env + assert env["error"]["code"] == "cloud_http_error" + + # --------------------------------------------------------------------------- # list-folder # --------------------------------------------------------------------------- diff --git a/tests/comfy_cli/test_http.py b/tests/comfy_cli/test_http.py index d5edf866..30d44d3c 100644 --- a/tests/comfy_cli/test_http.py +++ b/tests/comfy_cli/test_http.py @@ -1,4 +1,5 @@ import http.client +import json import types import urllib.error import urllib.request @@ -9,9 +10,11 @@ import comfy_cli.http as http_mod from comfy_cli.http import ( NoRedirectHandler, + ResponseTooLarge, authed_urlopen, build_authed_request, no_redirect_urlopen, + request_json, target_auth_headers, ) from comfy_cli.target import Target @@ -244,3 +247,209 @@ def test_target_auth_headers_cloud_uncredentialed_is_empty(): nothing rather than an empty/``None`` credential header.""" target = Target(kind="cloud", base_url="https://cloud.example") assert target_auth_headers(target) == {} + + +# --------------------------------------------------------------------------- +# request_json — the shared capped-read JSON helper +# --------------------------------------------------------------------------- + + +def _fake_resp(body: bytes, status: int = 200): + """Minimal urlopen-compatible response. ``read(n)`` truncates like the real one.""" + + class _Resp: + def __init__(self): + self.status = status + + def __enter__(self): + return self + + def __exit__(self, *args): + return False + + def read(self, n: int | None = None): + return body if n is None else body[:n] + + return _Resp() + + +def _patch_urlopen(monkeypatch: pytest.MonkeyPatch, payload, status: int = 200): + """Route every ``request_json`` call to ``payload`` (bytes) and record the Requests seen. + + ``request_json`` opens through the shared ``_AUTHED_OPENER`` (built with + ``NoRedirectHandler``), not the bare ``urllib.request.urlopen`` function, + so the fake must patch the opener's ``open`` method. + """ + seen: list[urllib.request.Request] = [] + + def _fake(req, timeout=None): + seen.append(req) + if isinstance(payload, Exception): + raise payload + return _fake_resp(payload, status) + + monkeypatch.setattr(http_mod._AUTHED_OPENER, "open", _fake) + return seen + + +@pytest.fixture +def cloud_target(): + return Target(kind="cloud", base_url="https://cloud.example", path_prefix="/api", api_key="test-api-key") + + +@pytest.fixture +def local_target(): + # Stray credentials on purpose: a local target must never emit them. + return Target(kind="local", base_url="http://127.0.0.1:8188", path_prefix="", api_key="stray", auth_token="stray") + + +def test_request_json_oversize_raises_response_too_large(monkeypatch, cloud_target): + # An oversize body must NOT masquerade as an unparseable one (which would + # silently degrade to ``None`` and look like an empty response). + _patch_urlopen(monkeypatch, b'{"data": []}') + with pytest.raises(ResponseTooLarge) as exc_info: + request_json("https://cloud.example/api/thing", cloud_target, max_bytes=4) + # The message is interpolated into search's envelope, so it must stay descriptive. + msg = str(exc_info.value) + assert "https://cloud.example/api/thing" in msg + assert "4" in msg + + +def test_request_json_body_exactly_at_cap_still_parses(monkeypatch, cloud_target): + # Boundary: len(raw) == cap is a *complete* body, not a truncated one. + body = b'{"a": 1}' + _patch_urlopen(monkeypatch, body) + assert request_json("https://cloud.example/api/thing", cloud_target, max_bytes=len(body)) == (200, {"a": 1}) + + +def test_request_json_empty_body_returns_none(monkeypatch, cloud_target): + _patch_urlopen(monkeypatch, b"", status=204) + assert request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) == (204, None) + + +def test_request_json_non_utf8_body_returns_none(monkeypatch, cloud_target): + # UnicodeDecodeError is a ValueError but *not* a JSONDecodeError; if it were + # not named in the except clause it would escape request_json uncaught. + _patch_urlopen(monkeypatch, b"\xff\xfe\x00not json") + assert request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) == (200, None) + + +def test_request_json_unparseable_body_returns_none(monkeypatch, cloud_target): + _patch_urlopen(monkeypatch, b"not json") + assert request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) == (200, None) + + +def test_request_json_list_body_parses(monkeypatch, cloud_target): + # Both model-listing endpoints return top-level JSON arrays. + _patch_urlopen(monkeypatch, b'["checkpoints", "loras"]') + assert request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) == ( + 200, + ["checkpoints", "loras"], + ) + + +def test_request_json_get_sends_no_body_and_no_content_type(monkeypatch, cloud_target): + seen = _patch_urlopen(monkeypatch, b"{}") + request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) + req = seen[0] + assert req.get_method() == "GET" + assert req.data is None + assert req.get_header("Content-type") is None + + +def test_request_json_post_attaches_json_body_and_content_type(monkeypatch, cloud_target): + seen = _patch_urlopen(monkeypatch, b"{}", status=201) + status, _ = request_json( + "https://cloud.example/api/thing", cloud_target, method="POST", body={"name": "wf"}, max_bytes=1024 + ) + assert status == 201 + req = seen[0] + assert req.get_method() == "POST" + assert json.loads(req.data) == {"name": "wf"} + assert req.get_header("Content-type") == "application/json" + + +def test_request_json_attaches_auth_headers_for_cloud(monkeypatch, cloud_target): + seen = _patch_urlopen(monkeypatch, b"{}") + request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) + # urllib title-cases header names on the Request object. + assert seen[0].get_header("X-api-key") == "test-api-key" + + +def test_request_json_attaches_no_auth_headers_for_local(monkeypatch, local_target): + # Same defense-in-depth gate as target_auth_headers: a local target never + # gets a credential, so a stray token can't leak to a plaintext server. + seen = _patch_urlopen(monkeypatch, b"{}") + request_json("http://127.0.0.1:8188/models", local_target, max_bytes=1024) + req = seen[0] + # Assert on the whole header bag, not two named keys: a named-key check + # would pass vacuously if urllib ever changed how it cases header names. + assert req.headers == {} + + +def test_request_json_raises_urllib_errors_verbatim(monkeypatch, cloud_target): + # Callers map these to envelope codes themselves, so they must not be swallowed. + err = urllib.error.HTTPError("https://cloud.example/api/thing", 503, "boom", http.client.HTTPMessage(), None) + _patch_urlopen(monkeypatch, err) + with pytest.raises(urllib.error.HTTPError) as exc_info: + request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) + assert exc_info.value.code == 503 + + +def test_request_json_max_bytes_is_keyword_required(cloud_target): + # No default: each caller keeps owning its own cap constant. + with pytest.raises(TypeError): + request_json("https://cloud.example/api/thing", cloud_target) + + +@pytest.mark.parametrize("max_bytes", [0, -1]) +def test_request_json_rejects_non_positive_max_bytes(cloud_target, max_bytes): + with pytest.raises(ValueError, match="max_bytes"): + request_json("https://cloud.example/api/thing", cloud_target, max_bytes=max_bytes) + + +def test_request_json_opens_via_shared_opener_with_no_redirect_handler(): + # A 30x with an authenticated request in flight must not be followed — + # NoRedirectHandler on the shared opener is what refuses it. Assert the + # opener request_json actually uses carries that handler, so a future + # revert to the bare default opener (which follows redirects and copies + # headers onto the new request) is caught here rather than in production. + assert any(isinstance(h, NoRedirectHandler) for h in http_mod._AUTHED_OPENER.handlers) + + +def test_request_json_refuses_plaintext_http_for_cloud_target(monkeypatch, cloud_target): + # A credential must never go out over cleartext HTTP to a non-loopback + # host — even if some misconfiguration points COMFY_CLOUD_BASE_URL at + # http://. No urlopen call should happen at all in this case. + seen = _patch_urlopen(monkeypatch, b"{}") + with pytest.raises(ValueError, match="non-https"): + request_json("http://cloud.example/api/thing", cloud_target, max_bytes=1024) + assert seen == [] + + +def test_request_json_allows_plaintext_http_for_loopback(monkeypatch, local_target): + # Loopback has no network to sniff, so plaintext is fine — this is the + # normal case for local ComfyUI. + _patch_urlopen(monkeypatch, b"{}") + assert request_json("http://127.0.0.1:8188/models", local_target, max_bytes=1024) == (200, {}) + + +def test_request_json_no_auth_headers_skips_https_gate(monkeypatch): + # A cloud target with no credentials at all (e.g. logged out) attaches no + # headers, so there's nothing to protect — the https/loopback gate must + # not block that request. + target = Target(kind="cloud", base_url="http://cloud.example", path_prefix="/api") + _patch_urlopen(monkeypatch, b"{}") + assert request_json("http://cloud.example/api/thing", target, max_bytes=1024) == (200, {}) + + +def test_request_json_recursion_error_returns_none(monkeypatch, cloud_target): + # A pathologically nested body can exhaust the interpreter stack in + # json.loads; RecursionError is neither JSONDecodeError nor + # UnicodeDecodeError, so it needs naming here or it escapes uncaught like + # the other two. Forced directly (rather than via real deep nesting, + # which CPython's C-accelerated decoder tolerates to very large depths) + # so this test stays fast and deterministic. + _patch_urlopen(monkeypatch, b'{"a": 1}') + monkeypatch.setattr(http_mod.json, "loads", lambda *a, **kw: (_ for _ in ()).throw(RecursionError())) + assert request_json("https://cloud.example/api/thing", cloud_target, max_bytes=1024) == (200, None)