From 0a0be740bd59c21b221c21ac1896f61d8f7ebf46 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Tue, 15 Sep 2026 13:34:47 +0300 Subject: [PATCH] fix(task-2): refuse multi-line / trailing-caret TEXT and ATTDEF strings before write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- backends/block_specs.py | 36 ++++++++++++++++------- tests/test_block_specs.py | 61 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/backends/block_specs.py b/backends/block_specs.py index 629b20b..3fdc85c 100644 --- a/backends/block_specs.py +++ b/backends/block_specs.py @@ -89,6 +89,29 @@ def _layer(spec: dict, where: str) -> str: 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]: """Normalise ``block_define`` primitive specs; refuse the whole list on the first defect.""" 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)] norm.update(points=pts, closed=closed, bulges=list(bulges) if bulges else None) elif kind == "text": - text = spec.get("text") - if not isinstance(text, str): - raise TypeError(f"{where}: 'text' must be a string") norm.update( - text=text, + text=_one_line(spec.get("text"), f"{where}: 'text'"), x=_number(spec, "x", where), y=_number(spec, "y", where), height=_number(spec, "height", where), @@ -163,13 +183,9 @@ def validate_attdef_specs(attdefs) -> list[dict]: if tag in seen: raise TypeError(f"{where}: duplicate tag {tag!r}") seen.add(tag) - prompt = spec.get("prompt", tag) - default = spec.get("default", "") + prompt = _one_line(spec.get("prompt", tag), f"{where}: 'prompt'") + default = _one_line(spec.get("default", ""), f"{where}: 'default'") 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): raise TypeError(f"{where}: 'invisible' must be a boolean") out.append( diff --git a/tests/test_block_specs.py b/tests/test_block_specs.py index 3cd6e88..ade8aa8 100644 --- a/tests/test_block_specs.py +++ b/tests/test_block_specs.py @@ -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": "text", "text": "M", "x": 0, "y": 0, "height": -1}, "height"), ({"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": "polyline", "points": [[0, 0]]}, "points"), ({"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": 1, "invisible": 1}, "invisible"), ({"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): @@ -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(): 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)]