fix(blocks): attribute values are validated before any write; null/bool/objects refused by tag
block_insert and block_set_attributes applied str() to whatever arrived, so a JSON null became the text None and a non-mapping failed after the INSERT was written. validate_attribute_values (backends/block_specs.py) runs first on both engines: strings pass the one-line rule, ints and finite floats are written as plain digits, everything else is a TypeError naming the tag. 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
b753a12bb6
commit
7f79497928
@@ -204,6 +204,38 @@ def validate_attdef_specs(attdefs) -> list[dict]:
|
|||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
def validate_attribute_values(attributes, where: str = "attributes") -> dict[str, str]:
|
||||||
|
"""``{TAG: value}`` for ``block_insert`` / ``block_set_attributes`` as the
|
||||||
|
strings that will be written, refused before any write.
|
||||||
|
|
||||||
|
``str(value)`` used to be applied to whatever arrived, so a JSON ``null``
|
||||||
|
became the text ``None``, ``true`` became ``True`` and an object became its
|
||||||
|
Python repr — silently, on both engines. A string is taken as is (one line,
|
||||||
|
the DXF rule ``_one_line`` enforces); an int or a finite float is written
|
||||||
|
with ``str`` (``101`` → ``"101"``, ``2.5`` → ``"2.5"``); ``bool``, ``None``,
|
||||||
|
lists and objects are refused by tag. ``None`` for the whole mapping means
|
||||||
|
"no attributes".
|
||||||
|
"""
|
||||||
|
if attributes is None:
|
||||||
|
return {}
|
||||||
|
if not isinstance(attributes, dict):
|
||||||
|
raise TypeError(f"{where} must be an object of {{TAG: value}} pairs")
|
||||||
|
out: dict[str, str] = {}
|
||||||
|
for tag, value in attributes.items():
|
||||||
|
if not isinstance(tag, str) or not tag.strip():
|
||||||
|
raise TypeError(f"{where}: every tag must be a non-empty string")
|
||||||
|
label = f"{where}[{tag!r}]"
|
||||||
|
if isinstance(value, str):
|
||||||
|
out[tag] = _one_line(value, label)
|
||||||
|
elif isinstance(value, bool) or value is None or not isinstance(value, (int, float)):
|
||||||
|
raise TypeError(f"{label} must be a string or a number, got {type(value).__name__}")
|
||||||
|
else:
|
||||||
|
if isinstance(value, float) and not math.isfinite(value):
|
||||||
|
raise TypeError(f"{label} must be finite")
|
||||||
|
out[tag] = str(value)
|
||||||
|
return out
|
||||||
|
|
||||||
|
|
||||||
def solid_vertices(points: list[tuple[float, float]]) -> list[tuple[float, float]]:
|
def solid_vertices(points: list[tuple[float, float]]) -> list[tuple[float, float]]:
|
||||||
"""DXF SOLID draws vtx0→vtx1→vtx3→vtx2, so a quad given in polygon order
|
"""DXF SOLID draws vtx0→vtx1→vtx3→vtx2, so a quad given in polygon order
|
||||||
(a, b, c, d) is stored as (a, b, d, c). Triangles are stored as given."""
|
(a, b, c, d) is stored as (a, b, d, c). Triangles are stored as given."""
|
||||||
|
|||||||
+13
-5
@@ -3315,6 +3315,10 @@ class ComBackend(AutoCADBackend):
|
|||||||
attributes=None,
|
attributes=None,
|
||||||
layer=None,
|
layer=None,
|
||||||
) -> EntityInfo:
|
) -> EntityInfo:
|
||||||
|
from backends.block_specs import validate_attribute_values
|
||||||
|
|
||||||
|
values = validate_attribute_values(attributes)
|
||||||
|
|
||||||
def _sync():
|
def _sync():
|
||||||
_require_block_defined(_acad_doc(), name)
|
_require_block_defined(_acad_doc(), name)
|
||||||
mspace = _msp()
|
mspace = _msp()
|
||||||
@@ -3328,13 +3332,13 @@ class ComBackend(AutoCADBackend):
|
|||||||
)
|
)
|
||||||
if layer:
|
if layer:
|
||||||
ref.Layer = layer
|
ref.Layer = layer
|
||||||
if attributes:
|
if values:
|
||||||
try:
|
try:
|
||||||
attrs = ref.GetAttributes()
|
attrs = ref.GetAttributes()
|
||||||
for attr in attrs:
|
for attr in attrs:
|
||||||
tag = attr.TagString
|
tag = attr.TagString
|
||||||
if tag in attributes:
|
if tag in values:
|
||||||
attr.TextString = str(attributes[tag])
|
attr.TextString = values[tag]
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
log.debug("Block insert: GetAttributes or attribute setting failed: %s", exc)
|
log.debug("Block insert: GetAttributes or attribute setting failed: %s", exc)
|
||||||
return _entity_info(ref)
|
return _entity_info(ref)
|
||||||
@@ -3730,6 +3734,10 @@ class ComBackend(AutoCADBackend):
|
|||||||
return await self._run(_sync)
|
return await self._run(_sync)
|
||||||
|
|
||||||
async def block_set_attributes(self, handle, attributes) -> dict:
|
async def block_set_attributes(self, handle, attributes) -> dict:
|
||||||
|
from backends.block_specs import validate_attribute_values
|
||||||
|
|
||||||
|
values = validate_attribute_values(attributes)
|
||||||
|
|
||||||
def _sync():
|
def _sync():
|
||||||
doc = _acad_doc()
|
doc = _acad_doc()
|
||||||
ref = doc.HandleToObject(handle)
|
ref = doc.HandleToObject(handle)
|
||||||
@@ -3737,8 +3745,8 @@ class ComBackend(AutoCADBackend):
|
|||||||
updated = []
|
updated = []
|
||||||
for attr in attrs:
|
for attr in attrs:
|
||||||
tag = attr.TagString
|
tag = attr.TagString
|
||||||
if tag in attributes:
|
if tag in values:
|
||||||
attr.TextString = str(attributes[tag])
|
attr.TextString = values[tag]
|
||||||
updated.append(tag)
|
updated.append(tag)
|
||||||
return {"ok": True, "updated_tags": updated}
|
return {"ok": True, "updated_tags": updated}
|
||||||
|
|
||||||
|
|||||||
@@ -4520,6 +4520,10 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
attributes=None,
|
attributes=None,
|
||||||
layer=None,
|
layer=None,
|
||||||
) -> EntityInfo:
|
) -> EntityInfo:
|
||||||
|
from backends.block_specs import validate_attribute_values
|
||||||
|
|
||||||
|
values = validate_attribute_values(attributes)
|
||||||
|
|
||||||
def _sync():
|
def _sync():
|
||||||
msp = self._msp()
|
msp = self._msp()
|
||||||
attribs: dict = {
|
attribs: dict = {
|
||||||
@@ -4531,12 +4535,12 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
attribs["layer"] = layer
|
attribs["layer"] = layer
|
||||||
self._require_block(name)
|
self._require_block(name)
|
||||||
ref = msp.add_blockref(name, (float(x), float(y)), dxfattribs=attribs)
|
ref = msp.add_blockref(name, (float(x), float(y)), dxfattribs=attribs)
|
||||||
if attributes:
|
if values:
|
||||||
# `add_auto_blockref` wraps the INSERT in an anonymous *U block,
|
# `add_auto_blockref` wraps the INSERT in an anonymous *U block,
|
||||||
# so the returned handle names `*U1` and carries no ATTRIBs.
|
# so the returned handle names `*U1` and carries no ATTRIBs.
|
||||||
# `add_auto_attribs` attaches ATTRIBs to this INSERT for every
|
# `add_auto_attribs` attaches ATTRIBs to this INSERT for every
|
||||||
# ATTDEF the definition has; values for tags it lacks are ignored.
|
# ATTDEF the definition has; values for tags it lacks are ignored.
|
||||||
ref.add_auto_attribs({str(k): str(v) for k, v in attributes.items()})
|
ref.add_auto_attribs(values)
|
||||||
self._mark_dirty()
|
self._mark_dirty()
|
||||||
return _entity_info_dxf(ref)
|
return _entity_info_dxf(ref)
|
||||||
|
|
||||||
@@ -4828,6 +4832,10 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
return await self._async(_sync)
|
return await self._async(_sync)
|
||||||
|
|
||||||
async def block_set_attributes(self, handle, attributes) -> dict:
|
async def block_set_attributes(self, handle, attributes) -> dict:
|
||||||
|
from backends.block_specs import validate_attribute_values
|
||||||
|
|
||||||
|
values = validate_attribute_values(attributes)
|
||||||
|
|
||||||
def _sync():
|
def _sync():
|
||||||
ent = self._get_entity(handle)
|
ent = self._get_entity(handle)
|
||||||
if ent.dxftype() != "INSERT":
|
if ent.dxftype() != "INSERT":
|
||||||
@@ -4835,8 +4843,8 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
updated = []
|
updated = []
|
||||||
for attrib in ent.attribs:
|
for attrib in ent.attribs:
|
||||||
tag = attrib.dxf.tag
|
tag = attrib.dxf.tag
|
||||||
if tag in attributes:
|
if tag in values:
|
||||||
self._write_attrib_value(attrib, str(attributes[tag]))
|
self._write_attrib_value(attrib, values[tag])
|
||||||
updated.append(tag)
|
updated.append(tag)
|
||||||
self._mark_dirty()
|
self._mark_dirty()
|
||||||
return {"ok": True, "updated_tags": updated}
|
return {"ok": True, "updated_tags": updated}
|
||||||
|
|||||||
@@ -3011,7 +3011,15 @@ async def block_insert(
|
|||||||
layer: Annotated[str | None, "Layer name"] = None,
|
layer: Annotated[str | None, "Layer name"] = None,
|
||||||
ctx: Context = None,
|
ctx: Context = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Insert a block and optionally set attribute values."""
|
"""Insert a block and optionally set attribute values.
|
||||||
|
|
||||||
|
Refused before any write, on both engines: an undefined or layout block
|
||||||
|
name, and an attribute value that is not text — a string is written as
|
||||||
|
is (one line), an int or finite float as its plain digits (`101` →
|
||||||
|
`"101"`), while `null`, booleans, lists and objects are refused naming
|
||||||
|
the tag (they used to be written as their Python repr). Values for tags
|
||||||
|
the block does not define are ignored.
|
||||||
|
"""
|
||||||
await ctx.info(f"Inserting block '{name}' at ({x},{y})")
|
await ctx.info(f"Inserting block '{name}' at ({x},{y})")
|
||||||
result = await _backend(ctx).block_insert(
|
result = await _backend(ctx).block_insert(
|
||||||
name, x, y, scale_x, scale_y, rotation, attributes, layer
|
name, x, y, scale_x, scale_y, rotation, attributes, layer
|
||||||
@@ -3085,6 +3093,11 @@ async def block_set_attributes(
|
|||||||
) -> dict:
|
) -> dict:
|
||||||
"""Update attribute values in a block reference.
|
"""Update attribute values in a block reference.
|
||||||
|
|
||||||
|
Same value rule as `block_insert`: strings and numbers are written,
|
||||||
|
`null` / booleans / lists / objects are refused naming the tag before
|
||||||
|
anything changes. `updated_tags` lists the tags that exist on the
|
||||||
|
reference and were written; unknown tags are ignored.
|
||||||
|
|
||||||
A multi-line attribute takes `Line1\\PLine2` and keeps every line; the
|
A multi-line attribute takes `Line1\\PLine2` and keeps every line; the
|
||||||
value written is the one `block_get_attributes` reads back and
|
value written is the one `block_get_attributes` reads back and
|
||||||
`block_explode` bursts, on both engines.
|
`block_explode` bursts, on both engines.
|
||||||
|
|||||||
@@ -1504,3 +1504,88 @@ async def test_com_explode_undoes_a_burst_that_fails_after_explode(com):
|
|||||||
assert placeholder.deleted is True
|
assert placeholder.deleted is True
|
||||||
assert [t.deleted for t in sheet.texts] == [True], "the TEXT written before the failure"
|
assert [t.deleted for t in sheet.texts] == [True], "the TEXT written before the failure"
|
||||||
assert space.calls == []
|
assert space.calls == []
|
||||||
|
|
||||||
|
|
||||||
|
# ── Task 3: attribute values are validated before any write ──────────────────
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"value, fragment",
|
||||||
|
[
|
||||||
|
(None, "NoneType"),
|
||||||
|
(True, "bool"),
|
||||||
|
({"a": 1}, "dict"),
|
||||||
|
([1, 2], "list"),
|
||||||
|
(float("nan"), "finite"),
|
||||||
|
("two\nlines", "single line"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
async def test_insert_refuses_a_non_text_attribute_value_by_tag(backend, value, fragment):
|
||||||
|
_define_tagged_block(backend)
|
||||||
|
before = len(list(backend._msp()))
|
||||||
|
with pytest.raises(TypeError, match=r"attributes\['TAG'\]") as exc:
|
||||||
|
await backend.block_insert("TB", 0, 0, attributes={"TAG": value})
|
||||||
|
assert fragment in str(exc.value)
|
||||||
|
assert len(list(backend._msp())) == before, "a refused insert must not write"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_insert_refuses_a_non_mapping_before_writing(backend):
|
||||||
|
_define_tagged_block(backend)
|
||||||
|
before = len(list(backend._msp()))
|
||||||
|
with pytest.raises(TypeError, match="attributes must be an object"):
|
||||||
|
await backend.block_insert("TB", 0, 0, attributes=[("TAG", "x")])
|
||||||
|
assert len(list(backend._msp())) == before
|
||||||
|
|
||||||
|
|
||||||
|
async def test_insert_writes_numbers_as_plain_text(backend):
|
||||||
|
_define_tagged_block(backend)
|
||||||
|
ref = await backend.block_insert("TB", 0, 0, attributes={"TAG": 101})
|
||||||
|
assert await backend.block_get_attributes(ref.handle) == {"TAG": "101"}
|
||||||
|
ref = await backend.block_insert("TB", 0, 0, attributes={"TAG": 2.5})
|
||||||
|
assert await backend.block_get_attributes(ref.handle) == {"TAG": "2.5"}
|
||||||
|
|
||||||
|
|
||||||
|
async def test_set_attributes_refuses_by_tag_and_leaves_the_value_alone(backend):
|
||||||
|
_define_tagged_block(backend)
|
||||||
|
ref = await backend.block_insert("TB", 0, 0, attributes={"TAG": "P-101"})
|
||||||
|
with pytest.raises(TypeError, match=r"attributes\['TAG'\] must be a string or a number"):
|
||||||
|
await backend.block_set_attributes(ref.handle, {"TAG": None})
|
||||||
|
assert await backend.block_get_attributes(ref.handle) == {"TAG": "P-101"}
|
||||||
|
result = await backend.block_set_attributes(ref.handle, {"TAG": 7})
|
||||||
|
assert result == {"ok": True, "updated_tags": ["TAG"]}
|
||||||
|
assert await backend.block_get_attributes(ref.handle) == {"TAG": "7"}
|
||||||
|
|
||||||
|
|
||||||
|
async def test_com_insert_refuses_before_insert_block_and_writes_numbers_as_text(com):
|
||||||
|
backend, document, space = com
|
||||||
|
space.attributes = {"TAG": ""}
|
||||||
|
with pytest.raises(TypeError, match=r"attributes\['TAG'\]"):
|
||||||
|
await backend.block_insert("TB", 0, 0, attributes={"TAG": None})
|
||||||
|
assert space.calls == [], "the refusal fires before InsertBlock"
|
||||||
|
|
||||||
|
await backend.block_insert("TB", 0, 0, attributes={"TAG": 101})
|
||||||
|
assert [name for name, _ in space.calls] == ["InsertBlock"]
|
||||||
|
attr = space.inserted[0].GetAttributes()[0]
|
||||||
|
assert attr.TextString == "101" and ("TextString", "101") in attr.writes
|
||||||
|
|
||||||
|
|
||||||
|
async def test_com_set_attributes_refuses_before_touching_the_reference(com):
|
||||||
|
backend, document, space = com
|
||||||
|
attr = _FakeObject("AcDbAttribute", "A1", TagString="TAG", TextString="P-101")
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def get_attributes():
|
||||||
|
calls.append("GetAttributes")
|
||||||
|
return (attr,)
|
||||||
|
|
||||||
|
document.objects["2F"] = _FakeObject(
|
||||||
|
"AcDbBlockReference", "2F", Name="TB", GetAttributes=get_attributes
|
||||||
|
)
|
||||||
|
with pytest.raises(TypeError, match=r"attributes\['TAG'\]"):
|
||||||
|
await backend.block_set_attributes("2F", {"TAG": True})
|
||||||
|
assert calls == [] and attr.TextString == "P-101"
|
||||||
|
assert await backend.block_set_attributes("2F", {"TAG": 2.5}) == {
|
||||||
|
"ok": True,
|
||||||
|
"updated_tags": ["TAG"],
|
||||||
|
}
|
||||||
|
assert attr.TextString == "2.5"
|
||||||
|
|||||||
Reference in New Issue
Block a user