From 4931c05feb105a06cc2af22bad8f24326c873819 Mon Sep 17 00:00:00 2001 From: Akiva Brookler Date: Mon, 22 Jun 2026 18:48:03 -0400 Subject: [PATCH] Fix AOI export crashes: FILETIME overflow and unsafe attribute escaping Two real-world .ACD projects fail to export to L5X: 1. _parse_aoi_nameless converts an AOI's Created/Edited Windows FILETIME to a datetime. On some AOI nameless records the variable-length field walk drifts and reads 8 garbage bytes, producing a FILETIME whose year exceeds datetime's 9999 ceiling and raising "OverflowError: date value out of range", aborting the whole export. 2. L5xElement.to_xml serializes attribute values with html.escape, which only handles & < > " '. When a binary field decodes to garbage (e.g. an AOI Vendor whose length/offset drifts, decoded with errors="replace"), the attribute ends up containing raw control characters and even raw newlines, producing non-well-formed L5X that downstream XML parsers reject ("open quote" / unterminated attribute). Fixes: - Add _filetime_to_iso(), which guards OverflowError/OSError and returns "" for empty/out-of-range FILETIMEs; use it for both AOI dates. - Add _escape_xml_attr(), which strips XML-1.0-illegal control characters and encodes TAB/CR/LF as numeric character references in addition to html.escape; use it for all attribute-value serialization. Adds unit tests for both helpers. Co-authored-by: Cursor --- acd/l5x/elements.py | 58 ++++++++++++++++++++++++++--------- test/test_elements_helpers.py | 42 +++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 15 deletions(-) create mode 100644 test/test_elements_helpers.py diff --git a/acd/l5x/elements.py b/acd/l5x/elements.py index d9ab51458..3aad052a3 100644 --- a/acd/l5x/elements.py +++ b/acd/l5x/elements.py @@ -16,6 +16,27 @@ from acd.l5x.port_structures import PORT_STRUCTURES +# Characters that are illegal in XML 1.0: everything outside +# #x9 | #xA | #xD | #x20-#xD7FF | #xE000-#xFFFD | #x10000-#x10FFFF. +_XML_ILLEGAL_RE = re.compile("[\x00-\x08\x0b\x0c\x0e-\x1f\ufffe\uffff]") + + +def _escape_xml_attr(value: object) -> str: + """Escape a value for use inside a double-quoted XML attribute. + + Beyond ``html.escape``, this strips characters that are illegal in XML 1.0 + and encodes the legal-but-delimiting whitespace (TAB/CR/LF) as numeric + character references. Some binary records decode to garbage when an offset + drifts (e.g. an AOI ``Vendor`` field read with ``errors="replace"``); without + this, those raw control characters and newlines land verbatim in an + attribute value, producing non-well-formed L5X that breaks downstream XML + parsers. + """ + text = _XML_ILLEGAL_RE.sub("", str(value)) + text = html.escape(text, quote=True) + return text.replace("\t", " ").replace("\r", " ").replace("\n", " ") + + @dataclass class L5xElementBuilder: _cur: Cursor @@ -78,7 +99,7 @@ def to_xml(self) -> str: _overrides = getattr(self, "_xml_attr_overrides", {}) xml_attr_name = _overrides.get(attribute, attribute.title().replace("_", "")) attribute_list.append( - f'{xml_attr_name}="{html.escape(str(attribute_value), quote=True)}"' + f'{xml_attr_name}="{_escape_xml_attr(attribute_value)}"' ) _export_name = ( @@ -611,7 +632,7 @@ def to_xml(self) -> str: if self._comm_method is not None: conn_parts: List[str] = [] for (conn_name, rpi_str, conn_type) in self._connections: - safe_name = html.escape(conn_name, quote=True) + safe_name = _escape_xml_attr(conn_name) # Derive InputTag / OutputTag stubs based on connection type. if conn_type == "Output": tag_stubs = ( @@ -773,7 +794,7 @@ def to_xml(self) -> str: ) if rung_xmls: rll_content = f'{"".join(rung_xmls)}' - return f'{rll_content}' + return f'{rll_content}' @dataclass @@ -1884,6 +1905,23 @@ def _parse_fffeff(data: bytes, offset: int): return s, offset + 4 + length * 2 +def _filetime_to_iso(ft: int) -> str: + """Convert a Windows FILETIME (100-ns units since 1601-01-01) to ISO8601. + + Returns "" for an empty or out-of-range value. Some AOI nameless records + carry a corrupt FILETIME (the variable-length field walk can drift and read + 8 garbage bytes), which would otherwise overflow ``datetime`` for years + beyond 9999 and abort the whole export with ``OverflowError``. + """ + if not ft: + return "" + try: + dt = datetime(1601, 1, 1) + timedelta(microseconds=ft // 10) + except (OverflowError, OSError): + return "" + return dt.strftime("%Y-%m-%dT%H:%M:%S.") + f"{dt.microsecond // 1000:03d}Z" + + def _parse_aoi_nameless(data: bytes) -> dict: """Extract AOI metadata from its large nameless record.""" result: dict = {} @@ -1912,12 +1950,7 @@ def _parse_aoi_nameless(data: bytes) -> dict: _, offset = _parse_fffeff(data, offset) # CreatedDate FILETIME (8 bytes, Windows FILETIME in 100-ns units) - ft = struct.unpack_from(" dict: result["revision_extension"] = rev_ext or None # EditedDate FILETIME (always last 8 bytes) - ft = struct.unpack_from("d"e') == "a&b<c>d"e" + + +def test_escape_xml_attr_strips_illegal_control_chars(): + assert _escape_xml_attr("a\x00\x15\x1fb") == "ab" + + +def test_escape_xml_attr_encodes_whitespace_delimiters(): + assert _escape_xml_attr("a\tb\nc\rd") == "a b c d" + + +def test_escape_xml_attr_keeps_attribute_well_formed(): + # A mis-parsed binary field (e.g. an AOI Vendor) with a raw newline and + # control bytes must not produce non-well-formed XML. + garbage = "Acme\x15Corp\nv\t1.0\ufffd" + xml = f'' + parsed = minidom.parseString(xml) # raises on malformed XML + assert parsed.documentElement.tagName == "AOI"