[PR #3165] Fix legacy ComfyUI batch update #3067

New issue

Open

opened 2026-10-28 21:14:55 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-28 21:14:55 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/ComfyUI-Manager/pull/3165
Author: @BILLFANLL
Created: 8/14/2026
Status: 🔄 Open

Base: manager-v4 ← Head: agent/fix-legacy-update-request

📝 Commits (1)

4ae6f5c Fix legacy ComfyUI batch update

📊 Changes

2 files changed (+41 additions, -5 deletions)

View changed files

📝 comfyui_manager/legacy/manager_server.py (+5 -1)
📝 tests/test_legacy_secgate_other_paths.py (+36 -4)

📄 Description

Summary

Move legacy ComfyUI update queueing into a private helper.
Call that helper from /queue/batch instead of invoking an HTTP handler with None.
Keep the simple-form Content-Type gate on the direct update endpoint.
Add regression coverage for both the legacy batch request and the direct endpoint.

Fixes #2843.

Root cause

The legacy batch handler called update_comfyui(None). After the simple-form POST guard was added, that route handler passed None to reject_simple_form_post, which then tried to read request.content_type and raised AttributeError.

Passing the outer batch request to the direct endpoint is not safe either: the current legacy frontend sends JSON.stringify(batch) without an explicit Content-Type, so browsers use text/plain;charset=UTF-8. The direct endpoint's guard would reject it, while the outer batch handler would still return 200 without queueing the update.

This change avoids handler-to-handler calls and keeps the direct route's existing security behavior unchanged.

Validation

venv/bin/python -m pytest -q tests/test_legacy_secgate_other_paths.py — 46 passed
venv/bin/ruff check comfyui_manager/legacy/manager_server.py tests/test_legacy_secgate_other_paths.py — passed
venv/bin/python -m py_compile comfyui_manager/legacy/manager_server.py tests/test_legacy_secgate_other_paths.py — passed
git diff --check — passed
uv build — sdist and wheel built successfully
Non-E2E unit suite — 348 passed, 12 skipped, with 3 branch-base failures in tests/common/test_git_helper.py where the fake repo exposes .heads while the implementation calls .list_heads()

Security scope

This fixes the None request crash without weakening reject_simple_form_post. It does not claim to change the broader existing Content-Type policy of the body-reading batch endpoint.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/ComfyUI-Manager/pull/3165 **Author:** [@BILLFANLL](https://github.com/BILLFANLL) **Created:** 8/14/2026 **Status:** 🔄 Open **Base:** `manager-v4` ← **Head:** `agent/fix-legacy-update-request` --- ### 📝 Commits (1) - [`4ae6f5c`](https://github.com/Comfy-Org/ComfyUI-Manager/commit/4ae6f5cf58a7f6a9d6bbc7ed8fb05b594858d176) Fix legacy ComfyUI batch update ### 📊 Changes **2 files changed** (+41 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `comfyui_manager/legacy/manager_server.py` (+5 -1) 📝 `tests/test_legacy_secgate_other_paths.py` (+36 -4) </details> ### 📄 Description ## Summary - Move legacy ComfyUI update queueing into a private helper. - Call that helper from `/queue/batch` instead of invoking an HTTP handler with `None`. - Keep the simple-form Content-Type gate on the direct update endpoint. - Add regression coverage for both the legacy batch request and the direct endpoint. Fixes #2843. ## Root cause The legacy batch handler called `update_comfyui(None)`. After the simple-form POST guard was added, that route handler passed `None` to `reject_simple_form_post`, which then tried to read `request.content_type` and raised `AttributeError`. Passing the outer batch request to the direct endpoint is not safe either: the current legacy frontend sends `JSON.stringify(batch)` without an explicit Content-Type, so browsers use `text/plain;charset=UTF-8`. The direct endpoint's guard would reject it, while the outer batch handler would still return 200 without queueing the update. This change avoids handler-to-handler calls and keeps the direct route's existing security behavior unchanged. ## Validation - `venv/bin/python -m pytest -q tests/test_legacy_secgate_other_paths.py` — 46 passed - `venv/bin/ruff check comfyui_manager/legacy/manager_server.py tests/test_legacy_secgate_other_paths.py` — passed - `venv/bin/python -m py_compile comfyui_manager/legacy/manager_server.py tests/test_legacy_secgate_other_paths.py` — passed - `git diff --check` — passed - `uv build` — sdist and wheel built successfully - Non-E2E unit suite — 348 passed, 12 skipped, with 3 branch-base failures in `tests/common/test_git_helper.py` where the fake repo exposes `.heads` while the implementation calls `.list_heads()` ## Security scope This fixes the `None` request crash without weakening `reject_simple_form_post`. It does not claim to change the broader existing Content-Type policy of the body-reading batch endpoint. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-28 21:14:55 +00:00