mirror of
https://github.com/Comfy-Org/ComfyUI.git
synced 2026-08-19 15:55:21 +08:00
Cap tag-filter lists at 100 entries, all spellings
Review finding: unbounded tag lists fan out into one correlated EXISTS per tag on both page and count statements. Cap each list at 100 normalized entries with 400 INVALID_TAG_FILTER naming the parameter. Applies to the legacy spellings as well — a deliberate, decided exception to the old-names-behave-identically rule, since a cap only on new names would leave the same fan-out reachable through the aliases.
This commit is contained in:
@@ -125,6 +125,13 @@ class InvalidTagFilterError(Exception):
|
||||
self.details = details
|
||||
|
||||
|
||||
# Applies to every tag-filter list, legacy spellings included — unlike the
|
||||
# combination validation below, which only runs when a new-name parameter is
|
||||
# present. Deliberate exception to legacy byte-identity (decision 2026-08-05):
|
||||
# an unbounded list fans out into one EXISTS per tag on page and count queries.
|
||||
MAX_TAG_FILTER_TAGS = 100
|
||||
|
||||
|
||||
def _resolve_tag_filters(
|
||||
q: schemas_in.ListAssetsQuery | schemas_in.TagsRefineQuery,
|
||||
) -> tuple[list[str], list[str], list[str]]:
|
||||
@@ -146,6 +153,24 @@ def _resolve_tag_filters(
|
||||
tags_any = normalize_tags(q.tags_any)
|
||||
tags_none = normalize_tags(q.tags_none)
|
||||
|
||||
for param_name, values in (
|
||||
("include_tags", include_tags),
|
||||
("exclude_tags", exclude_tags),
|
||||
("tags_all", tags_all),
|
||||
("tags_any", tags_any),
|
||||
("tags_none", tags_none),
|
||||
):
|
||||
if len(values) > MAX_TAG_FILTER_TAGS:
|
||||
raise InvalidTagFilterError(
|
||||
f"'{param_name}' lists {len(values)} tags; the maximum is "
|
||||
f"{MAX_TAG_FILTER_TAGS}.",
|
||||
{
|
||||
"parameter": param_name,
|
||||
"count": len(values),
|
||||
"max": MAX_TAG_FILTER_TAGS,
|
||||
},
|
||||
)
|
||||
|
||||
if not (tags_all or tags_any or tags_none):
|
||||
return include_tags, [], exclude_tags
|
||||
|
||||
|
||||
@@ -719,6 +719,28 @@ def test_list_assets_tag_values_case_sensitive(http, api_base, asset_factory, ma
|
||||
assert names_for({"tags_all": f"unit-tests,{scope},{upper}", "tags_none": lower}) == {a["name"]}
|
||||
|
||||
|
||||
def test_tag_list_cap_applies_to_all_spellings(http, api_base):
|
||||
"""The 100-tag cap covers legacy spellings too — the one deliberate
|
||||
exception to legacy byte-identity (decision 2026-08-05)."""
|
||||
big = ",".join(f"cap-{i}" for i in range(101))
|
||||
for param in ("tags_any", "include_tags"):
|
||||
r = http.get(api_base + "/api/assets", params={param: big}, timeout=120)
|
||||
body = r.json()
|
||||
assert r.status_code == 400, body
|
||||
assert body["error"]["code"] == "INVALID_TAG_FILTER"
|
||||
assert body["error"]["details"]["parameter"] == param
|
||||
assert body["error"]["details"]["max"] == 100
|
||||
|
||||
exact = ",".join(f"cap-{i}" for i in range(100))
|
||||
r = http.get(api_base + "/api/assets", params={"tags_any": exact}, timeout=120)
|
||||
assert r.status_code == 200, r.json()
|
||||
|
||||
# The cap counts normalized (deduped) tags, not raw CSV items.
|
||||
dups = ",".join("cap-dup" for _ in range(150))
|
||||
r = http.get(api_base + "/api/assets", params={"tags_any": dups}, timeout=120)
|
||||
assert r.status_code == 200, r.json()
|
||||
|
||||
|
||||
def test_resolve_tag_filters_no_deprecation_warning():
|
||||
"""The deprecated-field warning is for API clients; the server's own remap
|
||||
shim must not fire it on every request."""
|
||||
|
||||
Reference in New Issue
Block a user