fix(export): a control character in a label must not abort the export (#2897)
A C0 control character in a node label or id (from a mangled extraction or an odd filename) crashed the GraphML export (XML 1.0 forbids C0 except tab/LF/CR) and the Obsidian export (EINVAL on Windows paths), aborting the whole artifact. Fold a control-char scrub into the existing per-format coercion hooks so only the illegal characters are stripped; tab/LF/CR and non-ASCII letters are preserved, and to_json (and its byte-identity round-trip) is untouched.
This commit is contained in:
abhay-codes07
committed by
safishamsi
safishamsi
parent
b54a1f00fd
commit
917e331ab4
+47
-4
@@ -502,6 +502,31 @@ def to_cypher(G: nx.Graph, output_path: str) -> None:
|
||||
generate_html = to_html
|
||||
|
||||
|
||||
# Characters XML 1.0 cannot carry: the C0 controls except tab, LF and CR.
|
||||
_XML_ILLEGAL_RE = re.compile("[\x00-\x08\x0b\x0c\x0e-\x1f]")
|
||||
|
||||
|
||||
def _strip_xml_illegal(s: str) -> str:
|
||||
"""Drop characters XML 1.0 cannot represent, leaving tab/LF/CR intact.
|
||||
|
||||
``nx.write_graphml`` raises ``ValueError("All strings must be XML
|
||||
compatible: Unicode or ASCII, no NULL bytes or control characters")`` on any
|
||||
of them and aborts the whole export over a single label. Labels arrive
|
||||
unfiltered from the corpus, so this is ordinary content rather than hostile
|
||||
input: an ANSI escape in a markdown heading pasted from a terminal capture,
|
||||
or the form feed some Python/Emacs sources use as a section separator
|
||||
(#2897).
|
||||
"""
|
||||
return _XML_ILLEGAL_RE.sub("", s)
|
||||
|
||||
|
||||
# C0 controls and DEL, folded to a space when building a filename stem. Windows
|
||||
# rejects them in a path outright with OSError EINVAL, so one of them in a label
|
||||
# aborted a whole Obsidian vault export; POSIX would accept the name but leave a
|
||||
# note nothing can comfortably open (#2897).
|
||||
_CONTROL_TO_SPACE_RE = re.compile("[\x00-\x1f\x7f]")
|
||||
|
||||
|
||||
def _cap_filename(s: str, limit: int = 200) -> str:
|
||||
"""Cap a filename stem to ``limit`` UTF-8 bytes so it stays under the 255-byte
|
||||
filesystem limit even after the ``.md`` extension and dedup suffix are added
|
||||
@@ -531,7 +556,11 @@ def _obsidian_safe_stem(label: str, limit: int = 200) -> str:
|
||||
cleaned = re.sub(
|
||||
r'[\\/*?:"<>|#^[\]]',
|
||||
"",
|
||||
label.replace("\r\n", " ").replace("\r", " ").replace("\n", " "),
|
||||
# CR/LF were already folded to spaces here; every other C0 control now
|
||||
# goes the same way. They are not merely awkward in a filename — Windows
|
||||
# rejects them outright, so a single one aborted the whole vault export
|
||||
# rather than spoiling one note (#2897).
|
||||
_CONTROL_TO_SPACE_RE.sub(" ", label),
|
||||
).strip()
|
||||
cleaned = re.sub(r"\.(md|mdx|qmd|markdown)$", "", cleaned, flags=re.IGNORECASE)
|
||||
# Obsidian treats a leading-dot filename as a hidden file (#2205). Only
|
||||
@@ -1127,12 +1156,26 @@ def to_graphml(
|
||||
def _graphml_safe(val):
|
||||
if val is None:
|
||||
return ""
|
||||
if isinstance(val, bool) or isinstance(val, (int, float, str)):
|
||||
if isinstance(val, bool) or isinstance(val, (int, float)):
|
||||
return val # GraphML-native scalars pass through unchanged
|
||||
if isinstance(val, str):
|
||||
# Scalar, but still has to be XML-representable — see
|
||||
# _strip_xml_illegal. This is the line that turns "one label carried
|
||||
# an ANSI escape" from a lost export into a lost escape character.
|
||||
return _strip_xml_illegal(val)
|
||||
try:
|
||||
return json.dumps(val, default=str, sort_keys=True)
|
||||
return _strip_xml_illegal(json.dumps(val, default=str, sort_keys=True))
|
||||
except (TypeError, ValueError):
|
||||
return str(val)
|
||||
return _strip_xml_illegal(str(val))
|
||||
|
||||
# Node IDs become the `id` attribute of every <node> and edge endpoint, so
|
||||
# they must be XML-representable too. Normalised ids never carry a control
|
||||
# character, but a caller can hand us a hand-built graph, and a crash here
|
||||
# loses the export just as completely as one in the values.
|
||||
_id_remap = {n: _strip_xml_illegal(n) for n in H.nodes if isinstance(n, str)}
|
||||
_id_remap = {k: v for k, v in _id_remap.items() if k != v}
|
||||
if _id_remap:
|
||||
H = nx.relabel_nodes(H, _id_remap, copy=True)
|
||||
|
||||
for key, val in list(H.graph.items()):
|
||||
H.graph[key] = _graphml_safe(val)
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
"""A control character in a label must not abort an export.
|
||||
|
||||
Labels reach the exporters unfiltered from the corpus. A markdown heading pasted
|
||||
from a terminal capture carries ANSI escapes (`\\x1b`), and the form feed some
|
||||
Python/Emacs sources use as a section separator is `\\x0c`. Both are ordinary
|
||||
content, and detection, extraction and the graph build all accept them happily.
|
||||
|
||||
Two exporters then died on the whole graph:
|
||||
|
||||
* `to_graphml` -> `ValueError: All strings must be XML compatible: Unicode or
|
||||
ASCII, no NULL bytes or control characters` (XML 1.0 permits tab, LF and CR
|
||||
and no other C0 control);
|
||||
* `to_obsidian` -> `OSError: [Errno 22] Invalid argument` on Windows, which
|
||||
rejects control characters in a path outright, so one bad label cost the
|
||||
entire vault rather than one note.
|
||||
|
||||
`to_cypher` already stripped them and `to_html` already routes labels through
|
||||
`security.sanitize_label`, so the codebase knew the hazard — those two paths
|
||||
just did not. (#2897)
|
||||
"""
|
||||
import json
|
||||
import xml.etree.ElementTree as ET
|
||||
|
||||
import pytest
|
||||
|
||||
from graphify.build import build_from_json
|
||||
from graphify.export import (
|
||||
_obsidian_safe_stem,
|
||||
to_cypher,
|
||||
to_graphml,
|
||||
to_json,
|
||||
to_obsidian,
|
||||
)
|
||||
|
||||
# Legal in XML and in a filename — these must survive untouched.
|
||||
KEEP = ["\t", "\n", "\r"]
|
||||
# Rejected by XML 1.0, and by Windows in a path.
|
||||
BREAK = ["\x00", "\x07", "\x08", "\x0b", "\x0c", "\x1b", "\x1f"]
|
||||
|
||||
|
||||
def _graph(label):
|
||||
return build_from_json({
|
||||
"nodes": [
|
||||
{"id": "a", "label": label, "file_type": "document", "source_file": "d.md"},
|
||||
{"id": "b", "label": "plain", "file_type": "code", "source_file": "b.py"},
|
||||
],
|
||||
"edges": [{"source": "a", "target": "b", "relation": "references",
|
||||
"confidence": "INFERRED", "confidence_score": 0.85,
|
||||
"source_file": "d.md"}],
|
||||
"hyperedges": [],
|
||||
})
|
||||
|
||||
|
||||
COMMUNITIES = {0: ["a", "b"]}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GraphML
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("ch", BREAK)
|
||||
def test_graphml_survives_a_control_character(tmp_path, ch):
|
||||
out = tmp_path / "g.graphml"
|
||||
to_graphml(_graph(f"Build {ch}log capture"), COMMUNITIES, str(out))
|
||||
ET.fromstring(out.read_text(encoding="utf-8")) # must be well-formed XML
|
||||
|
||||
|
||||
@pytest.mark.parametrize("ch", KEEP)
|
||||
def test_graphml_keeps_the_whitespace_xml_allows(tmp_path, ch):
|
||||
"""Tab, LF and CR are valid XML and carry meaning in a label; the fix must
|
||||
not sweep them up with the rest."""
|
||||
out = tmp_path / "g.graphml"
|
||||
to_graphml(_graph("a" + ch + "b"), COMMUNITIES, str(out))
|
||||
ET.fromstring(out.read_text(encoding="utf-8")) # still well-formed
|
||||
# A writer may normalise CR to LF, so only the two that round-trip
|
||||
# literally are asserted to survive verbatim.
|
||||
if ch != "\r":
|
||||
assert ("a" + ch + "b") in out.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_graphml_survives_a_control_character_in_a_node_id(tmp_path):
|
||||
"""IDs become XML attributes too."""
|
||||
G = build_from_json({
|
||||
"nodes": [{"id": "we\x0bird", "label": "x", "file_type": "code",
|
||||
"source_file": "a.py"}],
|
||||
"edges": [], "hyperedges": [],
|
||||
})
|
||||
out = tmp_path / "g.graphml"
|
||||
to_graphml(G, {0: ["we\x0bird"]}, str(out))
|
||||
ET.fromstring(out.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
def test_graphml_still_carries_the_readable_part_of_the_label(tmp_path):
|
||||
out = tmp_path / "g.graphml"
|
||||
to_graphml(_graph("Build \x1b[31mlog\x1b[0m capture"), COMMUNITIES, str(out))
|
||||
text = out.read_text(encoding="utf-8")
|
||||
assert "log" in text and "capture" in text
|
||||
assert "\x1b" not in text
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Obsidian
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("ch", BREAK)
|
||||
def test_obsidian_export_survives_a_control_character(tmp_path, ch):
|
||||
count = to_obsidian(_graph(f"Release {ch} Notes"), COMMUNITIES,
|
||||
str(tmp_path / "vault"))
|
||||
assert count >= 2
|
||||
assert list((tmp_path / "vault").glob("*.md"))
|
||||
|
||||
|
||||
@pytest.mark.parametrize("ch", BREAK + KEEP)
|
||||
def test_no_stem_ever_contains_a_control_character(ch):
|
||||
stem = _obsidian_safe_stem(f"Release {ch} Notes")
|
||||
assert not any(ord(c) < 32 or ord(c) == 127 for c in stem), repr(stem)
|
||||
|
||||
|
||||
def test_stem_keeps_the_words_around_the_control_character():
|
||||
assert _obsidian_safe_stem("Release \x0c Notes").startswith("Release")
|
||||
assert "Notes" in _obsidian_safe_stem("Release \x0c Notes")
|
||||
|
||||
|
||||
def test_a_label_that_is_only_control_characters_still_yields_a_name():
|
||||
stem = _obsidian_safe_stem("\x00\x0b\x1b")
|
||||
assert stem and not any(ord(c) < 32 for c in stem)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The exporters that already coped must keep coping
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_cypher_and_json_are_unaffected(tmp_path):
|
||||
G = _graph("Build \x1b[31mlog\x1b[0m capture")
|
||||
to_cypher(G, str(tmp_path / "g.cypher"))
|
||||
to_json(G, COMMUNITIES, str(tmp_path / "g.json"))
|
||||
json.loads((tmp_path / "g.json").read_text(encoding="utf-8"))
|
||||
assert "\x1b" not in (tmp_path / "g.cypher").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_a_clean_label_round_trips_unchanged(tmp_path):
|
||||
"""The fix must be invisible for ordinary labels."""
|
||||
G = _graph("Perfectly Ordinary Heading")
|
||||
out = tmp_path / "g.graphml"
|
||||
to_graphml(G, COMMUNITIES, str(out))
|
||||
assert "Perfectly Ordinary Heading" in out.read_text(encoding="utf-8")
|
||||
assert _obsidian_safe_stem("Perfectly Ordinary Heading") == "Perfectly Ordinary Heading"
|
||||
Reference in New Issue
Block a user