mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-19 02:19:37 +00:00
fix(export): strip control characters from node and legend labels in graph.svg
A C0 control character in a node or community label (an ANSI escape from a pasted terminal capture, a form feed used as a section separator) made `graphify export svg` write a graph.svg that is not well-formed XML, while the command still reported success. #2897 fixed the same hazard for GraphML and Obsidian and left to_svg out: for a label without a pair of `$` it does not abort, matplotlib only warns about a missing glyph. matplotlib's SVG backend copies each drawn string into a comment (`<!-- text -->`, the default svg.fonttype) and escapes `&`, `<`, `>` and `--`, but not control characters, so xmllint and Python's expat reject the file. Pass the node labels and the legend text through _strip_xml_illegal, the helper to_graphml already uses: tab, LF and CR stay, the other C0 controls are dropped. A non-string label is str()-ed first, as networkx already did. Not covered here: the id a node without a label is drawn under (the next commit), and U+FFFE and U+FFFF, which the helper does not match either. Assisted-by: assayer harness on Claude Code (https://github.com/Yyunozor/assayer-memory-mcp)
This commit is contained in:
2 files changed
+71
-2
No files matched your search
+7
-2
@@ -1389,8 +1389,13 @@ def to_svg(
|
||||
|
||||
nx.draw_networkx_nodes(G, pos, ax=ax, node_color=node_colors,
|
||||
node_size=node_sizes, alpha=0.9)
|
||||
# matplotlib copies every string it draws into graph.svg (as a comment, with the default
|
||||
# svg.fonttype) and escapes only &, <, > and --, so a control character in a label
|
||||
# would leave the file malformed XML while the export reports success (same hazard as #2897).
|
||||
nx.draw_networkx_labels(G, pos, ax=ax,
|
||||
labels={n: G.nodes[n].get("label", n) for n in G.nodes()},
|
||||
labels={n: (_strip_xml_illegal(str(G.nodes[n]["label"]))
|
||||
if "label" in G.nodes[n] else n)
|
||||
for n in G.nodes()},
|
||||
font_size=7, font_color="white")
|
||||
|
||||
# Legend
|
||||
@@ -1398,7 +1403,7 @@ def to_svg(
|
||||
patches = [
|
||||
mpatches.Patch(
|
||||
color=COMMUNITY_COLORS[cid % len(COMMUNITY_COLORS)],
|
||||
label=f"{label} ({len(communities.get(cid, []))})",
|
||||
label=_strip_xml_illegal(f"{label} ({len(communities.get(cid, []))})"),
|
||||
)
|
||||
for cid, label in sorted(community_labels.items())
|
||||
]
|
||||
|
||||
@@ -17,6 +17,13 @@ Two exporters then died on the whole graph:
|
||||
`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)
|
||||
|
||||
`to_svg` was left out of that fix: for a label without a pair of `$` it does not
|
||||
abort, matplotlib only warns about a missing glyph and draws a box. The file is
|
||||
nonetheless not well-formed XML: matplotlib copies each label into it (a
|
||||
`<!-- ... -->` comment with the default `svg.fonttype`) and escapes `&`, `<`, `>`
|
||||
and `--`, not control characters, so a control character produced a `graph.svg`
|
||||
that a strict parser rejects, while the export still reported success.
|
||||
"""
|
||||
import json
|
||||
import xml.etree.ElementTree as ET
|
||||
@@ -30,6 +37,7 @@ from graphify.export import (
|
||||
to_graphml,
|
||||
to_json,
|
||||
to_obsidian,
|
||||
to_svg,
|
||||
)
|
||||
|
||||
# Legal in XML and in a filename — these must survive untouched.
|
||||
@@ -126,6 +134,62 @@ def test_a_label_that_is_only_control_characters_still_yields_a_name():
|
||||
assert stem and not any(ord(c) < 32 for c in stem)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SVG
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _svg(tmp_path, G, community_labels=None):
|
||||
pytest.importorskip("matplotlib")
|
||||
out = tmp_path / "g.svg"
|
||||
to_svg(G, COMMUNITIES, str(out), community_labels=community_labels)
|
||||
return out.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("ch", BREAK)
|
||||
def test_svg_survives_a_control_character(tmp_path, ch):
|
||||
ET.fromstring(_svg(tmp_path, _graph(f"Build {ch}log capture"))) # must be well-formed XML
|
||||
|
||||
|
||||
@pytest.mark.parametrize("ch", BREAK)
|
||||
def test_svg_survives_a_control_character_in_a_community_label(tmp_path, ch):
|
||||
"""The legend is a second path from a label to the file."""
|
||||
svg = _svg(tmp_path, _graph("plain"), {0: f"Core {ch}module"})
|
||||
ET.fromstring(svg)
|
||||
assert "Core" in svg and "module" in svg
|
||||
|
||||
|
||||
@pytest.mark.parametrize("ch", KEEP)
|
||||
def test_svg_keeps_the_whitespace_xml_allows(tmp_path, ch):
|
||||
"""Tab, LF and CR are valid XML; the fix must not sweep them up with the rest."""
|
||||
svg = _svg(tmp_path, _graph(f"Release{ch}Notes"))
|
||||
ET.fromstring(svg)
|
||||
assert "Release" in svg and "Notes" in svg
|
||||
assert "ReleaseNotes" not in svg # nothing was removed between the words
|
||||
|
||||
|
||||
def test_svg_still_carries_the_readable_part_of_the_label(tmp_path):
|
||||
svg = _svg(tmp_path, _graph("Build \x1b[31mlog\x1b[0m capture"))
|
||||
assert "Build [31mlog[0m capture" in svg
|
||||
assert "\x1b" not in svg
|
||||
|
||||
|
||||
def test_svg_clean_labels_are_unchanged(tmp_path):
|
||||
svg = _svg(tmp_path, _graph("Perfectly Ordinary Heading"), {0: "Ordinary Community"})
|
||||
assert "Perfectly Ordinary Heading" in svg
|
||||
assert "Ordinary Community (2)" in svg
|
||||
|
||||
|
||||
def test_svg_draws_a_label_that_is_not_a_string(tmp_path):
|
||||
"""nx.draw_networkx_labels str()s a non-string label on its own; the strip
|
||||
must not turn that into a TypeError, nor into an empty label."""
|
||||
G = _graph("x")
|
||||
G.nodes["a"]["label"] = 5
|
||||
del G.nodes["b"]["label"] # falls back to the node id
|
||||
svg = _svg(tmp_path, G)
|
||||
ET.fromstring(svg)
|
||||
assert "<!-- 5 -->" in svg or ">5</text>" in svg # svg.fonttype: path (default) or none
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The exporters that already coped must keep coping
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in new issue
Block a user