fix(detect): stop silently dropping rules from a non-UTF-8 ignore file (#2798)
Ignore files were read with errors="ignore", so a mis-encoded byte in a rule (e.g. a cp1252 accented directory name) was deleted, turning the pattern into one that matches nothing — an exclusion that silently failed open. Both rule-read sites now decode UTF-8-BOM first, then fall back to the host codepage and latin-1 (never raising, never dropping bytes), with a one-time warning; the rule survives intact instead of being truncated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
5929edf984
commit
7d855bbc98
+49
-2
@@ -1122,6 +1122,53 @@ def _git_info_exclude(vcs_root: Path) -> Path | None:
|
||||
return exclude if exclude.is_file() else None
|
||||
|
||||
|
||||
_warned_ignore_encodings: set[str] = set()
|
||||
|
||||
|
||||
def _read_ignore_text(path: Path) -> str:
|
||||
"""Read an ignore file, preferring UTF-8 but never silently dropping a rule.
|
||||
|
||||
These files were read with ``errors="ignore"``, which turns a mis-encoded
|
||||
byte into *no* byte. An ignore file saved in the host's ANSI codepage — the
|
||||
historical Notepad default on Windows, and still what ``Set-Content`` writes
|
||||
without ``-Encoding`` — is not valid UTF-8, so ``Or\xe7amento/`` decoded to
|
||||
the pattern ``Oramento/``. That matches nothing, and nothing said so: the
|
||||
directory was scanned despite an explicit exclusion, which for a rule
|
||||
covering documents or PDFs means they reach the semantic pass anyway.
|
||||
|
||||
So: UTF-8 (BOM-tolerant) first, since that is what the format should be and
|
||||
what every other reader here assumes. Only if that fails do we fall back to
|
||||
the host encoding, then to latin-1, which cannot fail and maps every byte to
|
||||
a codepoint — a rule spelled in some third encoding still comes out wrong,
|
||||
but it comes out *whole*, and the warning names the file so it is fixable.
|
||||
Decoding never raises, matching the previous contract.
|
||||
"""
|
||||
raw = path.read_bytes()
|
||||
try:
|
||||
return raw.decode("utf-8-sig")
|
||||
except UnicodeDecodeError:
|
||||
pass
|
||||
import locale
|
||||
import sys as _sys
|
||||
fallback = locale.getpreferredencoding(False) or "latin-1"
|
||||
for enc in (fallback, "latin-1"):
|
||||
try:
|
||||
text = raw.decode(enc)
|
||||
except (UnicodeDecodeError, LookupError):
|
||||
continue
|
||||
key = str(path)
|
||||
if key not in _warned_ignore_encodings:
|
||||
_warned_ignore_encodings.add(key)
|
||||
print(
|
||||
f"[graphify] WARNING: {path} is not valid UTF-8; read it as "
|
||||
f"{enc} instead. Re-save it as UTF-8 — patterns with non-ASCII "
|
||||
"characters may not match as written.",
|
||||
file=_sys.stderr,
|
||||
)
|
||||
return text
|
||||
return raw.decode("utf-8", errors="ignore")
|
||||
|
||||
|
||||
def _load_dir_own_ignore(d: Path, *, gitignore: bool = True) -> list[tuple[Path, str]]:
|
||||
"""Read .gitignore/.graphifyignore directly inside *d* (not its ancestors).
|
||||
|
||||
@@ -1142,7 +1189,7 @@ def _load_dir_own_ignore(d: Path, *, gitignore: bool = True) -> list[tuple[Path,
|
||||
for fname in ((".gitignore", ".graphifyignore") if gitignore else (".graphifyignore",)):
|
||||
ignore_file = d / fname
|
||||
if ignore_file.exists():
|
||||
for raw in ignore_file.read_text(encoding="utf-8-sig", errors="ignore").splitlines():
|
||||
for raw in _read_ignore_text(ignore_file).splitlines():
|
||||
line = _parse_gitignore_line(raw)
|
||||
if line:
|
||||
patterns.append((d, line))
|
||||
@@ -1184,7 +1231,7 @@ def _load_graphifyignore(root: Path, *, gitignore: bool = True) -> list[tuple[Pa
|
||||
# re-include still override it (#1810).
|
||||
info_exclude = _git_info_exclude(ceiling) if gitignore else None
|
||||
if info_exclude is not None:
|
||||
for raw in info_exclude.read_text(encoding="utf-8-sig", errors="ignore").splitlines():
|
||||
for raw in _read_ignore_text(info_exclude).splitlines():
|
||||
line = _parse_gitignore_line(raw)
|
||||
if line:
|
||||
patterns.append((ceiling, line))
|
||||
|
||||
@@ -171,7 +171,7 @@ def test_graphifyignore_matches_nfd_path_with_nfc_pattern(tmp_path):
|
||||
nfd_name = unicodedata.normalize("NFD", nfc_name)
|
||||
assert nfc_name != nfd_name # guard: the two forms really do differ
|
||||
|
||||
(tmp_path / ".graphifyignore").write_text(f"{nfc_name}/\n")
|
||||
(tmp_path / ".graphifyignore").write_text(f"{nfc_name}/\n", encoding="utf-8")
|
||||
secret_dir = tmp_path / nfd_name
|
||||
secret_dir.mkdir()
|
||||
(secret_dir / "contrato.py").write_text("x = 1")
|
||||
@@ -188,7 +188,7 @@ def test_graphifyignore_matches_nfc_path_with_nfd_pattern(tmp_path):
|
||||
nfc_name = unicodedata.normalize("NFC", "Or\u00e7amento")
|
||||
nfd_name = unicodedata.normalize("NFD", nfc_name)
|
||||
|
||||
(tmp_path / ".graphifyignore").write_text(f"{nfd_name}/\n")
|
||||
(tmp_path / ".graphifyignore").write_text(f"{nfd_name}/\n", encoding="utf-8")
|
||||
d = tmp_path / nfc_name
|
||||
d.mkdir()
|
||||
(d / "contrato.py").write_text("x = 1")
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
r"""An ignore file that is not valid UTF-8 must not silently lose its rules.
|
||||
|
||||
`_load_dir_own_ignore` / `_load_graphifyignore` read .gitignore,
|
||||
.graphifyignore and $GIT_DIR/info/exclude with `errors="ignore"`, which turns a
|
||||
mis-encoded byte into no byte at all. A file saved in the host ANSI codepage —
|
||||
Notepad's historical default on Windows, and still what `Set-Content` writes
|
||||
without `-Encoding` — is not valid UTF-8, so a rule reading `Orçamento/`
|
||||
(cp1252 `Or\xe7amento/`) decoded to `Oramento/`, matched nothing, and said
|
||||
nothing. The directory was scanned despite an explicit exclusion.
|
||||
|
||||
That is the failure mode the NFC/NFD tests in test_detect.py already warn about
|
||||
in prose ("the rule silently does nothing — the files get scanned, and
|
||||
docs/PDFs are sent to an LLM despite an explicit exclusion"), reached by a
|
||||
different route.
|
||||
|
||||
These tests write the bytes directly rather than going through `write_text`, so
|
||||
they pin the decoding behaviour on every platform, not just where cp1252 is the
|
||||
default.
|
||||
"""
|
||||
import unicodedata
|
||||
|
||||
import pytest
|
||||
|
||||
from graphify.detect import _read_ignore_text, detect
|
||||
|
||||
NAME = "Orçamento" # "Orçamento" — ç is U+00E7, present in cp1252
|
||||
|
||||
|
||||
def _corpus(tmp_path, ignore_bytes: bytes, dirname: str = NAME):
|
||||
(tmp_path / ".graphifyignore").write_bytes(ignore_bytes)
|
||||
d = tmp_path / dirname
|
||||
d.mkdir()
|
||||
(d / "contrato.py").write_text("x = 1", encoding="utf-8")
|
||||
(tmp_path / "main.py").write_text("print('hi')", encoding="utf-8")
|
||||
return tmp_path
|
||||
|
||||
|
||||
def _scanned(result) -> set[str]:
|
||||
from pathlib import Path
|
||||
return {Path(f).name for f in result["files"]["code"]}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The bug
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_ansi_encoded_rule_still_excludes(tmp_path):
|
||||
"""The reported case: a cp1252 .graphifyignore must still exclude."""
|
||||
_corpus(tmp_path, f"{NAME}/\n".encode("cp1252"))
|
||||
scanned = _scanned(detect(tmp_path))
|
||||
assert "contrato.py" not in scanned, (
|
||||
"a non-UTF-8 ignore rule silently did nothing; scanned=" + repr(scanned))
|
||||
assert "main.py" in scanned
|
||||
|
||||
|
||||
def test_ansi_encoded_rule_warns_once_naming_the_file(tmp_path, capsys):
|
||||
import graphify.detect as detect_mod
|
||||
detect_mod._warned_ignore_encodings.clear()
|
||||
_corpus(tmp_path, f"{NAME}/\n".encode("cp1252"))
|
||||
detect(tmp_path)
|
||||
err = capsys.readouterr().err
|
||||
assert ".graphifyignore" in err and "UTF-8" in err, err
|
||||
|
||||
|
||||
def test_utf8_rule_is_unaffected(tmp_path):
|
||||
"""The control: the format we document keeps working, with no warning."""
|
||||
_corpus(tmp_path, f"{NAME}/\n".encode("utf-8"))
|
||||
assert "contrato.py" not in _scanned(detect(tmp_path))
|
||||
|
||||
|
||||
def test_ascii_rules_are_untouched(tmp_path):
|
||||
(tmp_path / ".graphifyignore").write_bytes(b"vendor/\n")
|
||||
(tmp_path / "vendor").mkdir()
|
||||
(tmp_path / "vendor" / "lib.py").write_text("x = 1", encoding="utf-8")
|
||||
(tmp_path / "main.py").write_text("x = 1", encoding="utf-8")
|
||||
scanned = _scanned(detect(tmp_path))
|
||||
assert scanned == {"main.py"}, scanned
|
||||
|
||||
|
||||
def test_no_warning_for_a_clean_utf8_file(tmp_path, capsys):
|
||||
import graphify.detect as detect_mod
|
||||
detect_mod._warned_ignore_encodings.clear()
|
||||
_corpus(tmp_path, f"{NAME}/\n".encode("utf-8"))
|
||||
detect(tmp_path)
|
||||
assert "not valid UTF-8" not in capsys.readouterr().err
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _read_ignore_text directly
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_utf8_with_bom_is_still_stripped(tmp_path):
|
||||
p = tmp_path / ".graphifyignore"
|
||||
p.write_bytes(b"\xef\xbb\xbfvendor/\n")
|
||||
assert _read_ignore_text(p) == "vendor/\n"
|
||||
|
||||
|
||||
def test_decoding_never_raises_on_arbitrary_bytes(tmp_path):
|
||||
"""The previous contract: reading an ignore file cannot blow up a scan."""
|
||||
p = tmp_path / ".graphifyignore"
|
||||
p.write_bytes(bytes(range(256)))
|
||||
assert isinstance(_read_ignore_text(p), str)
|
||||
|
||||
|
||||
def test_no_byte_is_dropped_from_a_mis_encoded_file(tmp_path):
|
||||
"""The actual regression: every rule survives, even if a third encoding
|
||||
renders it wrong, rather than being silently truncated to nothing."""
|
||||
p = tmp_path / ".graphifyignore"
|
||||
p.write_bytes("café/\nvendor/\n".encode("cp1252"))
|
||||
lines = [ln for ln in _read_ignore_text(p).splitlines() if ln]
|
||||
assert len(lines) == 2, lines
|
||||
assert lines[1] == "vendor/"
|
||||
assert len(lines[0]) == len("café/"), lines[0]
|
||||
|
||||
|
||||
def test_empty_file_is_empty(tmp_path):
|
||||
p = tmp_path / ".graphifyignore"
|
||||
p.write_bytes(b"")
|
||||
assert _read_ignore_text(p) == ""
|
||||
|
||||
|
||||
@pytest.mark.parametrize("form", ["NFC", "NFD"])
|
||||
def test_utf8_rules_still_match_across_normalisation_forms(tmp_path, form):
|
||||
"""The existing NFC/NFD guarantee must survive the new decode path."""
|
||||
pattern = unicodedata.normalize(form, NAME)
|
||||
other = unicodedata.normalize("NFD" if form == "NFC" else "NFC", NAME)
|
||||
_corpus(tmp_path, f"{pattern}/\n".encode("utf-8"), dirname=other)
|
||||
assert "contrato.py" not in _scanned(detect(tmp_path))
|
||||
Reference in New Issue
Block a user