fix(skills): pin text-mode file I/O to UTF-8 in comfyui and pdf skill scripts
The bundled comfyui and pdf skills read and write text files with the locale-default codec. Both declare platforms: [linux, macos, windows], so these paths run on hosts where that codec is not UTF-8 (cp1252 on US Windows, cp936 on Chinese Windows, ASCII under LC_ALL=C). Readers (the live bugs): - run_workflow.py load_schema() and the main() workflow read parse user-authored JSON. A non-ASCII label crashes json.load with UnicodeDecodeError under a non-UTF-8 locale, and a file saved from a Windows GUI editor carries a UTF-8 BOM that json.load rejects with JSONDecodeError. Both are read as utf-8-sig, which is BOM-tolerant and identical to utf-8 on BOM-less input. This differs fromadecb0d1a, which used plain utf-8 for the pdf form JSON; those payloads are agent-authored and BOM-free by construction, these are not. - hardware_check.py reads /proc/version and /proc/meminfo. Both are Linux-gated so Windows never reaches them, but the C locale defaults to ASCII, so they pin plain utf-8. No BOM is possible on /proc. Writers (not currently broken): - extract_form_structure.py and extract_form_field_info.py write their JSON with json.dump, whose default ensure_ascii=True keeps the bytes pure ASCII. Pinned anyway because the codec is the writer's contract, not a property of what the caller happens to dump. wf_path.open() is a Path.open() site that check-windows-footguns.py deliberately does not flag (per the rule comment: "Path.open() is ALSO affected ... and can be audited separately"). It is fixed here because it is the same bug 156 lines from a site the checker does flag, and line 623 of the same file already uses read_text(encoding="utf-8"). Adds tests/skills/test_comfyui_skill.py with contract assertions plus two live regressions that run load_schema in a child interpreter under LC_ALL=C with PYTHONUTF8=0, and extends the office skill tests with writer contract assertions. All 8 new tests fail without this change. Note that pyproject.toml exempts skills/** from ruff PLW1514 (unspecified-encoding) because skill scripts are partly user-authored. This change does not touch that exemption; the sites are fixed by hand, the same wayadecb0d1adid.
This commit is contained in:
parent
20fece3b42
commit
50f742f8ed
|
|
@ -68,7 +68,7 @@ def is_wsl() -> bool:
|
|||
if "microsoft" in platform.release().lower() or "wsl" in platform.release().lower():
|
||||
return True
|
||||
try:
|
||||
with open("/proc/version", "r") as fh:
|
||||
with open("/proc/version", "r", encoding="utf-8") as fh:
|
||||
return "microsoft" in fh.read().lower()
|
||||
except OSError:
|
||||
return False
|
||||
|
|
@ -227,7 +227,7 @@ def total_system_ram_gb() -> float:
|
|||
return 0.0
|
||||
if sysname == "Linux":
|
||||
try:
|
||||
with open("/proc/meminfo", "r") as fh:
|
||||
with open("/proc/meminfo", "r", encoding="utf-8") as fh:
|
||||
for line in fh:
|
||||
if line.startswith("MemTotal:"):
|
||||
kb = int(line.split()[1])
|
||||
|
|
|
|||
|
|
@ -450,7 +450,7 @@ def _inline_schema(workflow: dict) -> dict:
|
|||
|
||||
def load_schema(schema_path: str | None, workflow: dict) -> dict:
|
||||
if schema_path:
|
||||
with open(schema_path) as f:
|
||||
with open(schema_path, encoding="utf-8-sig") as f:
|
||||
return json.load(f)
|
||||
return _inline_schema(workflow)
|
||||
|
||||
|
|
@ -606,7 +606,7 @@ def main(argv: list[str] | None = None) -> int:
|
|||
emit_json({"error": f"Workflow file not found: {args.workflow}"})
|
||||
return 1
|
||||
try:
|
||||
with wf_path.open() as f:
|
||||
with wf_path.open(encoding="utf-8-sig") as f:
|
||||
workflow_raw = json.load(f)
|
||||
workflow = unwrap_workflow(workflow_raw)
|
||||
except ValueError as e:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,125 @@
|
|||
"""Invariant tests for the bundled comfyui skill.
|
||||
|
||||
Covers skills/creative/comfyui — the diffusion workflow runner. Tests assert
|
||||
contracts (locale-independent file reads), not snapshots of skill content.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO = Path(__file__).resolve().parent.parent.parent
|
||||
SCRIPTS = REPO / "skills" / "creative" / "comfyui" / "scripts"
|
||||
|
||||
# Text reads that must not depend on the host locale. The workflow and schema
|
||||
# JSON are user-authored files (exported by ComfyUI or hand-edited), so they
|
||||
# are read BOM-tolerantly: Notepad prepends U+FEFF, which makes json.load
|
||||
# raise JSONDecodeError. See the jobs.json regression in #66607. The /proc
|
||||
# reads are Linux-gated and never carry a BOM, so they pin plain utf-8.
|
||||
_ENCODING_SENSITIVE_READS = [
|
||||
("hardware_check.py", 'with open("/proc/version", "r", encoding="utf-8") as fh:'),
|
||||
("hardware_check.py", 'with open("/proc/meminfo", "r", encoding="utf-8") as fh:'),
|
||||
("run_workflow.py", 'with open(schema_path, encoding="utf-8-sig") as f:'),
|
||||
("run_workflow.py", 'with wf_path.open(encoding="utf-8-sig") as f:'),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel_path,expected", _ENCODING_SENSITIVE_READS)
|
||||
def test_readers_are_locale_independent(rel_path, expected):
|
||||
"""Every text read of a user-supplied or system file pins its codec."""
|
||||
source = (SCRIPTS / rel_path).read_text(encoding="utf-8")
|
||||
assert expected in source, f"{rel_path}: locale-dependent read of a UTF-8 payload"
|
||||
|
||||
|
||||
def _run_under_c_locale(snippet: str) -> subprocess.CompletedProcess:
|
||||
"""Execute a snippet in a child interpreter forced to a non-UTF-8 locale.
|
||||
|
||||
The default text codec is resolved at interpreter startup, so the locale
|
||||
has to be set on the child's environment. Patching os.environ in-process
|
||||
would not change locale.getpreferredencoding(). PYTHONUTF8=0 disables
|
||||
PEP 540 UTF-8 mode, which would otherwise mask the bug entirely.
|
||||
"""
|
||||
env = dict(os.environ)
|
||||
env.update({
|
||||
"LC_ALL": "C",
|
||||
"LANG": "C",
|
||||
"PYTHONUTF8": "0",
|
||||
"PYTHONIOENCODING": "utf-8",
|
||||
})
|
||||
return subprocess.run(
|
||||
[sys.executable, "-c", snippet],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
timeout=60,
|
||||
)
|
||||
|
||||
|
||||
def test_load_schema_reads_non_ascii_under_non_utf8_locale(tmp_path):
|
||||
"""A schema with non-ASCII labels loads under the C locale.
|
||||
|
||||
Without the explicit encoding the C-locale default codec is ASCII, so
|
||||
json.load crashes with UnicodeDecodeError on any CJK/Cyrillic label.
|
||||
"""
|
||||
schema_path = tmp_path / "schema.json"
|
||||
schema_path.write_bytes(
|
||||
json.dumps(
|
||||
{"prompt": {"label": "プロンプト", "type": "string"}},
|
||||
ensure_ascii=False,
|
||||
).encode("utf-8")
|
||||
)
|
||||
|
||||
result = _run_under_c_locale(
|
||||
textwrap.dedent(
|
||||
f"""
|
||||
import sys
|
||||
sys.path.insert(0, {str(SCRIPTS)!r})
|
||||
from run_workflow import load_schema
|
||||
|
||||
schema = load_schema({str(schema_path)!r}, {{}})
|
||||
assert schema["prompt"]["label"] == "\\u30d7\\u30ed\\u30f3\\u30d7\\u30c8", schema
|
||||
print("SUCCESS")
|
||||
"""
|
||||
)
|
||||
)
|
||||
assert result.returncode == 0, (
|
||||
f"load_schema failed under non-UTF-8 locale:\n{result.stderr}"
|
||||
)
|
||||
assert "SUCCESS" in result.stdout
|
||||
|
||||
|
||||
def test_load_schema_tolerates_utf8_bom(tmp_path):
|
||||
"""A schema saved by a Windows GUI editor (UTF-8 BOM) still parses.
|
||||
|
||||
json.load rejects a leading U+FEFF with JSONDecodeError, so a BOM-blind
|
||||
read turns "user edited the file in Notepad" into a hard failure.
|
||||
"""
|
||||
schema_path = tmp_path / "schema.json"
|
||||
schema_path.write_bytes(
|
||||
b"\xef\xbb\xbf" + json.dumps({"prompt": {"type": "string"}}).encode("utf-8")
|
||||
)
|
||||
|
||||
result = _run_under_c_locale(
|
||||
textwrap.dedent(
|
||||
f"""
|
||||
import sys
|
||||
sys.path.insert(0, {str(SCRIPTS)!r})
|
||||
from run_workflow import load_schema
|
||||
|
||||
schema = load_schema({str(schema_path)!r}, {{}})
|
||||
assert schema["prompt"]["type"] == "string", schema
|
||||
print("SUCCESS")
|
||||
"""
|
||||
)
|
||||
)
|
||||
assert result.returncode == 0, (
|
||||
f"load_schema rejected a BOM-prefixed schema:\n{result.stderr}"
|
||||
)
|
||||
assert "SUCCESS" in result.stdout
|
||||
Loading…
Reference in New Issue