fix(task-2): refuse multi-line / trailing-caret TEXT and ATTDEF strings before write
`text`, ATTDEF `prompt` and `default` were only isinstance(str)-checked, so
'A\nB' and '10^' passed validation and ezdxf's DXFAttr fixer
(`fix_one_line_text`) rewrote them on `add_text` / `add_attdef` at DEBUG
log level — the declared ATTRIB value and the one the P&ID reader gets back
differed silently, and COM (raw string) diverged from ezdxf. Same hole the
tag check closed in 4158575, left open on its three siblings.
A `_one_line(value, where)` helper now refuses '\n', '\r', any other C0
control, and a trailing '^' (ezdxf's own `is_valid_one_line_text` rule),
naming `entities[i]: 'text'` / `attdefs[i]: 'prompt'|'default'`. Tests cover
the refusals, the strings ezdxf keeps verbatim ('10^2', 'Ø20 H7', '%%C20'),
and an ezdxf round-trip proving accepted values are stored unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
co-authored by
Claude Opus 5
parent
41585757c9
commit
0a0be740bd
+26
-10
@@ -89,6 +89,29 @@ def _layer(spec: dict, where: str) -> str:
|
|||||||
return layer
|
return layer
|
||||||
|
|
||||||
|
|
||||||
|
def _one_line(value, where: str) -> str:
|
||||||
|
"""A single-line DXF string (TEXT content, ATTDEF prompt/default).
|
||||||
|
|
||||||
|
ezdxf's DXFAttr fixer (``fix_one_line_text``) silently strips line breaks
|
||||||
|
and a trailing ``^`` on write, logged at DEBUG only, so the declared value
|
||||||
|
and the value read back would differ; COM writes the raw string, so the two
|
||||||
|
engines would also diverge. Refuse it here, before any write, using ezdxf's
|
||||||
|
own ``is_valid_one_line_text`` rule plus the other C0 controls DXF text
|
||||||
|
cannot carry.
|
||||||
|
"""
|
||||||
|
if not isinstance(value, str):
|
||||||
|
raise TypeError(f"{where} must be a string")
|
||||||
|
controls = sorted({c for c in value if ord(c) < 32})
|
||||||
|
if controls:
|
||||||
|
shown = ", ".join(repr(c) for c in controls)
|
||||||
|
raise TypeError(
|
||||||
|
f"{where} must be a single line without control characters (contains {shown})"
|
||||||
|
)
|
||||||
|
if value.endswith("^"):
|
||||||
|
raise TypeError(f"{where} must not end with '^' (DXF strips a trailing caret)")
|
||||||
|
return value
|
||||||
|
|
||||||
|
|
||||||
def validate_entity_specs(entities) -> list[dict]:
|
def validate_entity_specs(entities) -> list[dict]:
|
||||||
"""Normalise ``block_define`` primitive specs; refuse the whole list on the first defect."""
|
"""Normalise ``block_define`` primitive specs; refuse the whole list on the first defect."""
|
||||||
if not isinstance(entities, (list, tuple)) or not entities:
|
if not isinstance(entities, (list, tuple)) or not entities:
|
||||||
@@ -128,11 +151,8 @@ def validate_entity_specs(entities) -> list[dict]:
|
|||||||
bulges = [_scalar(b, f"{where}: bulges[{j}]") for j, b in enumerate(bulges)]
|
bulges = [_scalar(b, f"{where}: bulges[{j}]") for j, b in enumerate(bulges)]
|
||||||
norm.update(points=pts, closed=closed, bulges=list(bulges) if bulges else None)
|
norm.update(points=pts, closed=closed, bulges=list(bulges) if bulges else None)
|
||||||
elif kind == "text":
|
elif kind == "text":
|
||||||
text = spec.get("text")
|
|
||||||
if not isinstance(text, str):
|
|
||||||
raise TypeError(f"{where}: 'text' must be a string")
|
|
||||||
norm.update(
|
norm.update(
|
||||||
text=text,
|
text=_one_line(spec.get("text"), f"{where}: 'text'"),
|
||||||
x=_number(spec, "x", where),
|
x=_number(spec, "x", where),
|
||||||
y=_number(spec, "y", where),
|
y=_number(spec, "y", where),
|
||||||
height=_number(spec, "height", where),
|
height=_number(spec, "height", where),
|
||||||
@@ -163,13 +183,9 @@ def validate_attdef_specs(attdefs) -> list[dict]:
|
|||||||
if tag in seen:
|
if tag in seen:
|
||||||
raise TypeError(f"{where}: duplicate tag {tag!r}")
|
raise TypeError(f"{where}: duplicate tag {tag!r}")
|
||||||
seen.add(tag)
|
seen.add(tag)
|
||||||
prompt = spec.get("prompt", tag)
|
prompt = _one_line(spec.get("prompt", tag), f"{where}: 'prompt'")
|
||||||
default = spec.get("default", "")
|
default = _one_line(spec.get("default", ""), f"{where}: 'default'")
|
||||||
invisible = spec.get("invisible", False)
|
invisible = spec.get("invisible", False)
|
||||||
if not isinstance(prompt, str):
|
|
||||||
raise TypeError(f"{where}: 'prompt' must be a string")
|
|
||||||
if not isinstance(default, str):
|
|
||||||
raise TypeError(f"{where}: 'default' must be a string")
|
|
||||||
if not isinstance(invisible, bool):
|
if not isinstance(invisible, bool):
|
||||||
raise TypeError(f"{where}: 'invisible' must be a boolean")
|
raise TypeError(f"{where}: 'invisible' must be a boolean")
|
||||||
out.append(
|
out.append(
|
||||||
|
|||||||
@@ -50,6 +50,10 @@ def test_each_type_normalises_to_floats_and_layer_zero():
|
|||||||
({"type": "circle", "cx": 0, "cy": 0, "r": float("nan")}, "finite"),
|
({"type": "circle", "cx": 0, "cy": 0, "r": float("nan")}, "finite"),
|
||||||
({"type": "text", "text": "M", "x": 0, "y": 0, "height": -1}, "height"),
|
({"type": "text", "text": "M", "x": 0, "y": 0, "height": -1}, "height"),
|
||||||
({"type": "text", "text": 5, "x": 0, "y": 0, "height": 1}, "text"),
|
({"type": "text", "text": 5, "x": 0, "y": 0, "height": 1}, "text"),
|
||||||
|
({"type": "text", "text": "A\nB", "x": 0, "y": 0, "height": 1}, "'text'"),
|
||||||
|
({"type": "text", "text": "A\rB", "x": 0, "y": 0, "height": 1}, "'text'"),
|
||||||
|
({"type": "text", "text": "A\x01B", "x": 0, "y": 0, "height": 1}, "'text'"),
|
||||||
|
({"type": "text", "text": "10^", "x": 0, "y": 0, "height": 1}, "'text'"),
|
||||||
({"type": "text", "text": "M", "x": 0, "y": 0, "height": 1, "align": "top"}, "align"),
|
({"type": "text", "text": "M", "x": 0, "y": 0, "height": 1, "align": "top"}, "align"),
|
||||||
({"type": "polyline", "points": [[0, 0]]}, "points"),
|
({"type": "polyline", "points": [[0, 0]]}, "points"),
|
||||||
({"type": "polyline", "points": [[0, 0], [1]]}, "points[1]"),
|
({"type": "polyline", "points": [[0, 0], [1]]}, "points[1]"),
|
||||||
@@ -113,6 +117,11 @@ def test_attdefs_normalise_and_default():
|
|||||||
({"tag": "TAG", "x": 0, "y": 0, "height": 0}, "height"),
|
({"tag": "TAG", "x": 0, "y": 0, "height": 0}, "height"),
|
||||||
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "invisible": 1}, "invisible"),
|
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "invisible": 1}, "invisible"),
|
||||||
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "default": 3}, "default"),
|
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "default": 3}, "default"),
|
||||||
|
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "prompt": 3}, "prompt"),
|
||||||
|
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "prompt": "P\nQ"}, "'prompt'"),
|
||||||
|
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "default": "L-1\r\n"}, "'default'"),
|
||||||
|
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "default": "L-1^"}, "'default'"),
|
||||||
|
({"tag": "TAG", "x": 0, "y": 0, "height": 1, "default": "L\x1f1"}, "'default'"),
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
def test_bad_attdef_names_the_index_and_key(bad, fragment):
|
def test_bad_attdef_names_the_index_and_key(bad, fragment):
|
||||||
@@ -144,6 +153,58 @@ def test_trailing_newline_cannot_smuggle_a_duplicate_tag():
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("value", ["A\nB", "A\rB", "10^", "A\tB", "L-1\r\n"])
|
||||||
|
def test_one_line_strings_are_refused_before_ezdxf_would_rewrite_them(value):
|
||||||
|
"""ezdxf's `fix_one_line_text` strips line breaks and a trailing caret on
|
||||||
|
`add_text` / `add_attdef` (DEBUG log only), so 'A\\nB' lands as 'AB' and the
|
||||||
|
ATTRIB value the P&ID reader gets back differs from the declared one; COM
|
||||||
|
writes the raw string, so the engines would diverge too. The gate is ours,
|
||||||
|
typed, and indexed — the same hole the tag check closed, on its siblings."""
|
||||||
|
from ezdxf.lldxf.validator import is_valid_one_line_text
|
||||||
|
|
||||||
|
# Either ezdxf's own rule rejects it or it is a C0 control DXF text cannot carry.
|
||||||
|
assert not is_valid_one_line_text(value) or any(ord(c) < 32 for c in value)
|
||||||
|
with pytest.raises(TypeError, match=r"entities\[0\]: 'text'"):
|
||||||
|
validate_entity_specs([{"type": "text", "text": value, "x": 0, "y": 0, "height": 2}])
|
||||||
|
with pytest.raises(TypeError, match=r"attdefs\[0\]: 'prompt'"):
|
||||||
|
validate_attdef_specs([{"tag": "TAG", "x": 0, "y": 0, "height": 2, "prompt": value}])
|
||||||
|
with pytest.raises(TypeError, match=r"attdefs\[0\]: 'default'"):
|
||||||
|
validate_attdef_specs([{"tag": "TAG", "x": 0, "y": 0, "height": 2, "default": value}])
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("value", ["10^2", "Ø20 H7", "%%C20", "L-101", ""])
|
||||||
|
def test_one_line_strings_that_ezdxf_keeps_verbatim_pass(value):
|
||||||
|
"""A caret inside the text (`10^2`), unicode and `%%C` survive both engines."""
|
||||||
|
from ezdxf.lldxf.validator import is_valid_one_line_text
|
||||||
|
|
||||||
|
assert is_valid_one_line_text(value)
|
||||||
|
out = validate_entity_specs([{"type": "text", "text": value, "x": 0, "y": 0, "height": 2}])
|
||||||
|
assert out[0]["text"] == value
|
||||||
|
att = validate_attdef_specs(
|
||||||
|
[{"tag": "TAG", "x": 0, "y": 0, "height": 2, "prompt": value, "default": value}]
|
||||||
|
)
|
||||||
|
assert att[0]["prompt"] == value and att[0]["default"] == value
|
||||||
|
|
||||||
|
|
||||||
|
def test_one_line_gate_writes_through_ezdxf_unchanged():
|
||||||
|
"""Round-trip proof: what the gate accepts, ezdxf stores verbatim (no
|
||||||
|
DXFAttr fixer rewrite), so declared == read back."""
|
||||||
|
import ezdxf
|
||||||
|
|
||||||
|
doc = ezdxf.new()
|
||||||
|
blk = doc.blocks.new("T")
|
||||||
|
[spec] = validate_entity_specs(
|
||||||
|
[{"type": "text", "text": "10^2 Ø", "x": 0, "y": 0, "height": 2}]
|
||||||
|
)
|
||||||
|
[att] = validate_attdef_specs(
|
||||||
|
[{"tag": "TAG", "x": 0, "y": 0, "height": 2, "prompt": "P Q", "default": "L-101"}]
|
||||||
|
)
|
||||||
|
t = blk.add_text(spec["text"])
|
||||||
|
a = blk.add_attdef(att["tag"], text=att["default"], dxfattribs={"prompt": att["prompt"]})
|
||||||
|
assert t.dxf.text == "10^2 Ø"
|
||||||
|
assert a.dxf.text == "L-101" and a.dxf.prompt == "P Q"
|
||||||
|
|
||||||
|
|
||||||
def test_solid_vertices_use_dxf_bowtie_order():
|
def test_solid_vertices_use_dxf_bowtie_order():
|
||||||
quad = [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 1.0)]
|
quad = [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 1.0)]
|
||||||
assert solid_vertices(quad) == [(0.0, 0.0), (1.0, 0.0), (0.0, 1.0), (1.0, 1.0)]
|
assert solid_vertices(quad) == [(0.0, 0.0), (1.0, 0.0), (0.0, 1.0), (1.0, 1.0)]
|
||||||
|
|||||||
Reference in New Issue
Block a user