From 01cd2d223045b1f4a028e8001b0b2cd35e8fb6b8 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 20 Aug 2026 10:01:46 -0400 Subject: [PATCH 1/3] Cover non-BMP characters and lone-surrogate sources with tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-BMP codepoint is one UTF-16 surrogate pair in a UTF-16 source and one 4-byte UTF-8 sequence on disk — exactly the input a byte-region scanner that guessed at character boundaries would break on. Nothing pinned that down. tests/test_unicode.py adds the deterministic cases: byte-identical round trips with non-BMP text in an attribute value, in element text, and inside a ; exact scanner regions across a 4-byte sequence; an untouched non-BMP entry emitted verbatim while its neighbour is edited; streaming read and write; clean validation. On the refusal side, a numeric character reference to a lone surrogate and CESU-8/WTF-8 surrogate halves both raise LiftParseError through the full and the streaming reader, so no lone surrogate reaches the model from a file. A UTF-16 source carrying non-BMP content loads, re-serializes canonically as UTF-8 (the documented byte-identity exception for a non-ASCII-compatible encoding), and is byte-stable from there on. The property suite drew non-BMP codepoints from st.characters() too rarely to count as coverage, so text and attribute-value alphabets now sample them explicitly. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_property_roundtrip.py | 10 +- tests/test_unicode.py | 238 +++++++++++++++++++++++++++++++ 2 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 tests/test_unicode.py diff --git a/tests/test_property_roundtrip.py b/tests/test_property_roundtrip.py index 54979f2..5a77d29 100644 --- a/tests/test_property_roundtrip.py +++ b/tests/test_property_roundtrip.py @@ -26,10 +26,16 @@ # non-characters; \r is excluded (parsers normalize it), \t and \n are added # back via explicit alternatives where allowed. _CHARS = st.characters(min_codepoint=0x20, codec="utf-8", exclude_characters="￾￿") -_TEXT = st.text(alphabet=st.one_of(_CHARS, st.sampled_from("\t\n")), max_size=30) +# Non-BMP codepoints — 4-byte UTF-8, one surrogate pair each in a UTF-16 source +# — drawn explicitly: they are what a byte scanner guessing at character +# boundaries would break on, and _CHARS alone reaches them too rarely to count +# as coverage. See tests/test_unicode.py for the deterministic cases. +_NON_BMP = st.sampled_from("\U0001f389\U00020000\U0001e900\U0001d11e\U000e0021") +_CHARS_INCL_NON_BMP = st.one_of(_CHARS, _NON_BMP) +_TEXT = st.text(alphabet=st.one_of(_CHARS_INCL_NON_BMP, st.sampled_from("\t\n")), max_size=30) # Attribute values: XML parsers normalize \t\n in attributes to spaces, so keep # tokens to characters that round-trip verbatim. -_TOKEN = st.text(alphabet=_CHARS, min_size=1, max_size=15) +_TOKEN = st.text(alphabet=_CHARS_INCL_NON_BMP, min_size=1, max_size=15) _LANG = st.sampled_from(["en", "fr", "th", "sg", "es", "qaa-x-test"]) _WHEN = st.one_of( diff --git a/tests/test_unicode.py b/tests/test_unicode.py new file mode 100644 index 0000000..6182cd2 --- /dev/null +++ b/tests/test_unicode.py @@ -0,0 +1,238 @@ +"""Non-BMP characters and surrogate encodings, end to end. + +Python strings are sequences of codepoints, not UTF-16 code units, so a +"surrogate pair" in a UTF-16 source is just one non-BMP codepoint by the time +the model sees it. Nothing in the reader, the byte-region scanner, or the writer +pairs code units — and these tests pin that down, because the fidelity contract +(``docs/en/fidelity.md``) is a byte-level promise and 4-byte UTF-8 sequences are +exactly the input that would break a scanner that guessed at character +boundaries. + +Two halves: + +- **Non-BMP content is ordinary content**: byte-identical round trips, exact + scanner regions, verbatim untouched entries under edit, streaming reads and + writes, and clean validation — in element text, in a span, and in an + attribute value. +- **A lone surrogate never reaches the model from a file**: neither as a + numeric character reference (invalid XML) nor as CESU-8/WTF-8 bytes, which + tools that mishandle UTF-16 internally emit (each surrogate half individually + UTF-8-encoded instead of paired first). Both are rejected at parse time as + ``LiftParseError``, not silently mangled. + +UTF-16 sources are covered here too: they load, and — being non-ASCII-compatible +— re-serialize canonically as UTF-8 rather than byte-identically, which is the +documented exception, not a defect. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +import sil_lift +from sil_lift import LiftParseError, Span +from sil_lift._scan import scan + +PARTY = "\U0001f389" # PARTY POPPER, plane 1 (emoji) +CJK_B = "\U00020000" # CJK Ext. B ideograph, plane 2 +ADLAM = "\U0001e900" # ADLAM CAPITAL LETTER ALIF, plane 1 (a real orthography) +NON_BMP = PARTY + CJK_B + ADLAM + +# Non-BMP codepoints in every place a LIFT document can hold text: an attribute +# value, element text, and a nested run. +NON_BMP_LIFT = f""" + + +
{ADLAM}{ADLAM}
 + +party {PARTY} {CJK_B} + + + +
ascii
 + + +""".encode() + +UTF16_NON_BMP_LIFT = f""" + +
{NON_BMP}
\ + + +""".encode("utf-16") + +# A numeric character reference to a lone surrogate: not a valid XML character, +# whatever the encoding. Written by tools that emit UTF-16 code units as if they +# were codepoints. +LONE_SURROGATE_REF_LIFT = b""" + +
�
 + +""" + +# CESU-8/WTF-8: U+1F389 as its two UTF-16 halves (U+D83C, U+DF89), each +# UTF-8-encoded on its own instead of paired into one 4-byte sequence. Valid +# UTF-8 would be f0 9f 8e 89. +CESU8_LIFT = ( + b'\n\n' + b'
' + b"\xed\xa0\xbc\xed\xbe\x89" + b"
\n\n" +) + + +def _write(tmp_path: Path, name: str, data: bytes) -> Path: + path = tmp_path / name + path.write_bytes(data) + return path + + +# --- non-BMP content is ordinary content --------------------------------------- + + +def test_non_bmp_round_trips_byte_identically(tmp_path: Path) -> None: + source = _write(tmp_path, "non-bmp.lift", NON_BMP_LIFT) + lexicon = sil_lift.load(source) + + entry = lexicon.entries[0] + assert entry.id == f"nb-{NON_BMP}" # attribute value + assert str(entry.lexical_unit["qaa-x-nb"]) == ADLAM * 2 # element text + gloss = entry.senses[0].gloss("en") + assert gloss is not None + (span,) = [fragment for fragment in gloss.fragments if isinstance(fragment, Span)] + assert str(span) == PARTY # span content + assert str(gloss) == f"party {PARTY} {CJK_B}" + + out = tmp_path / "out.lift" + lexicon.save(out) + assert out.read_bytes() == NON_BMP_LIFT + + +def test_scanner_regions_land_on_four_byte_utf8_boundaries() -> None: + """The scanner only ever matches ASCII structural bytes, which no UTF-8 + continuation byte can impersonate — so a 4-byte sequence in an attribute + value cannot shift a region's start or end.""" + result = scan(NON_BMP_LIFT) + assert result is not None + assert [region.tag for region in result.children] == ["entry", "entry"] + + non_bmp_region, ascii_region = result.children + reused = NON_BMP_LIFT[non_bmp_region.start : non_bmp_region.end] + assert reused.startswith(f'") + assert reused.decode() # a region boundary mid-codepoint would not decode + assert NON_BMP_LIFT[ascii_region.start : ascii_region.end].startswith(b' None: + source = _write(tmp_path, "non-bmp.lift", NON_BMP_LIFT) + lexicon = sil_lift.load(source) + lexicon.entries[1].lexical_unit["qaa-x-nb"] = CJK_B # touch the *other* entry + + out = tmp_path / "out.lift" + lexicon.save(out) + written = out.read_bytes() + + result = scan(NON_BMP_LIFT) + assert result is not None + region = result.children[0] + assert NON_BMP_LIFT[region.start : region.end] in written + # The re-serialized entry carries raw UTF-8, not a numeric character + # reference — the canonical path escapes markup, never non-ASCII text. + assert CJK_B.encode() in written + assert b"&#x" not in written + + reloaded = sil_lift.load(out) + assert reloaded.entries[0] == lexicon.entries[0] + assert str(reloaded.entries[1].lexical_unit["qaa-x-nb"]) == CJK_B + + +def test_non_bmp_survives_streaming_read_and_write(tmp_path: Path) -> None: + source = _write(tmp_path, "non-bmp.lift", NON_BMP_LIFT) + with sil_lift.open_reader(source) as reader: + entries = list(reader) + assert [entry.id for entry in entries] == [f"nb-{NON_BMP}", "ascii"] + + out = tmp_path / "streamed.lift" + with sil_lift.open_writer(out, producer="test_unicode") as writer: + for entry in entries: + writer.write(entry) + + reloaded = sil_lift.load(out) + assert str(reloaded.entries[0].lexical_unit["qaa-x-nb"]) == ADLAM * 2 + assert reloaded.entries == entries + + +def test_non_bmp_document_validates_clean(tmp_path: Path) -> None: + """Non-BMP text is not a validation finding: the model, the RELAX NG layer, + and the semantic checks all treat it as ordinary character data.""" + source = _write(tmp_path, "non-bmp.lift", NON_BMP_LIFT) + assert list(sil_lift.iter_problems(source)) == [] + + +# --- lone surrogates never reach the model from a file -------------------------- + + +@pytest.mark.parametrize( + ("data", "name"), + [ + (LONE_SURROGATE_REF_LIFT, "lone-surrogate-ref.lift"), + (CESU8_LIFT, "cesu8.lift"), + ], + ids=["character-reference", "cesu-8-bytes"], +) +def test_lone_surrogate_sources_are_rejected(data: bytes, name: str, tmp_path: Path) -> None: + source = _write(tmp_path, name, data) + with pytest.raises(LiftParseError, match="not well-formed"): + sil_lift.load(source) + + +@pytest.mark.parametrize( + ("data", "name"), + [ + (LONE_SURROGATE_REF_LIFT, "lone-surrogate-ref.lift"), + (CESU8_LIFT, "cesu8.lift"), + ], + ids=["character-reference", "cesu-8-bytes"], +) +def test_lone_surrogate_sources_are_rejected_when_streaming( + data: bytes, name: str, tmp_path: Path +) -> None: + """Same refusal on the streaming path, which pumps events past the first + entry at open time and so fails there rather than mid-iteration.""" + source = _write(tmp_path, name, data) + with ( + pytest.raises(LiftParseError, match="not well-formed"), + sil_lift.open_reader(source) as reader, + ): + list(reader) + + +# --- UTF-16 sources ------------------------------------------------------------- + + +def test_utf16_source_with_non_bmp_loads_and_saves_as_utf8(tmp_path: Path) -> None: + """A UTF-16 source loads with its non-BMP content intact; saving falls back + to canonical UTF-8, the documented byte-identity exception for a + non-ASCII-compatible encoding.""" + source = _write(tmp_path, "utf16.lift", UTF16_NON_BMP_LIFT) + lexicon = sil_lift.load(source) + (entry,) = lexicon.entries + assert entry.id == f"nb-{ADLAM}" + assert str(entry.lexical_unit["qaa-x-nb"]) == NON_BMP + + out = tmp_path / "out.lift" + lexicon.save(out) + written = out.read_bytes() + assert written != UTF16_NON_BMP_LIFT, "expected the canonical fallback, not passthrough" + assert written.startswith(b'') + assert NON_BMP.encode() in written # one 4-byte sequence per codepoint + + reloaded = sil_lift.load(out) + assert reloaded.entries == lexicon.entries + # Byte identity resumes once the document is UTF-8: re-saving changes nothing. + again = tmp_path / "again.lift" + reloaded.save(again) + assert again.read_bytes() == written From b13553d0ccd3d5759278a076818e3794ca6def06 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 20 Aug 2026 13:58:51 -0400 Subject: [PATCH 2/3] Refuse unrepresentable content with LiftWriteError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Python string can hold a lone surrogate (U+D800-U+DFFF); an XML document cannot, in any encoding. No LIFT file can introduce one — the parser rejects both a � character reference and CESU-8/WTF-8 bytes — so it only ever arrives as a string assigned through the API, and then lxml raised a bare UnicodeEncodeError from wherever the text or attribute was set, naming no node, out of four different public entry points: save(), changes(), changed_entries(), and validation. Every writer path that builds and serializes a node now goes through _guarded(), which reports it as LiftWriteError naming the node and the codepoint. Anything else a str cannot encode as UTF-8 — nothing, today — is re-raised untouched rather than mislabelled. The digest functions delegate to the canonical byte functions they duplicated, so the guard covers snapshots too, and canonical_range_bytes / _ranges_root_open_bytes replace the copies of those two bodies. Validation renders the document before checking it, so it cannot check an unrenderable one: it now reports the refusal as a single lone-surrogate error (addressed to the companion, and without stopping the rest, when a .lift-ranges is the unwritable part) instead of propagating the encode error. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 +- docs/en/fidelity.md | 6 ++ docs/en/guides/validate.md | 2 + src/sil_lift/__init__.py | 3 +- src/sil_lift/_errors.py | 15 ++++- src/sil_lift/_model.py | 4 +- src/sil_lift/_validate.py | 28 +++++++-- src/sil_lift/_writer.py | 92 ++++++++++++++++++++------- tests/test_unicode.py | 124 ++++++++++++++++++++++++++++++++++++- 9 files changed, 244 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c896b13..ec04d9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,10 @@ releases may contain breaking changes. documents and untouched entries are written byte-identically; touched entries re-serialize canonically with all out-of-schema content preserved. Fidelity contract documented in `docs/en/fidelity.md` and enforced by corpus - byte-identity tests plus Hypothesis round-trip properties. + byte-identity tests plus Hypothesis round-trip properties. Content XML cannot + represent — a lone surrogate, which only an API assignment can introduce — is + refused with `LiftWriteError` naming the node, and reported by validation as + `lone-surrogate`. - Change detection against the loaded document, reading the same parse-time digests. `Lexicon.changed_entries()` reports entries whose content differs (an entry's digest covers its whole subtree, so an edit at any depth reports diff --git a/docs/en/fidelity.md b/docs/en/fidelity.md index b160e14..6b9c2df 100644 --- a/docs/en/fidelity.md +++ b/docs/en/fidelity.md @@ -26,6 +26,12 @@ Exceptions (the writer falls back to full canonical serialization, which is sema !!! note ""Canonical" here is not related to any other Canonical XML" Canonical form on this page means `sil-lift`'s own documented shape, described in a bullet above. It is unrelated to W3C's Canonical XML (C14N) process. It is unrelated to `SIL.Core`'s `CanonicalXmlSettings` class. +## Content XML cannot represent + +Non-BMP characters — emoji, CJK Extension B, Adlam, anything above U+FFFF — are ordinary content and round-trip byte-identically. A "surrogate pair" is a UTF-16 encoding detail: Python strings are sequences of codepoints, so nothing in the reader, the byte scanner, or the writer ever sees one. + +A _lone_ surrogate (U+D800–U+DFFF) is different: a Python string may hold one, an XML document may not, in any encoding. It can never arrive from a file — the parser rejects both spellings, a `�` character reference and CESU-8/WTF-8 bytes — only from a string assigned through the API. Saving such a model raises `LiftWriteError` naming the node and the codepoint and writes nothing; validation reports it as a single `lone-surrogate` error, since the document cannot be serialized for the schema layers to check. + ## Known approximations (touched nodes only) - Comments _inside_ a `` run are preserved but moved next to the run, not kept at their exact character offset. diff --git a/docs/en/guides/validate.md b/docs/en/guides/validate.md index f73b98b..b6fb006 100644 --- a/docs/en/guides/validate.md +++ b/docs/en/guides/validate.md @@ -44,6 +44,8 @@ Every finding carries one of these, whichever layer produced it — `schema` and | `undefined-range-value` | warning | a grammatical-info or range-keyed trait value the range does not list | | `uri-not-rfc` | warning | an href that is not a valid URI — FLEx's `file://C:/...` | +All three layers work from what `save()` would write, so a document that cannot be serialized at all is reported as a single `lone-surrogate` error instead — see [Fidelity guarantees](../fidelity.md#content-xml-cannot-represent). + ## Real-world FieldWorks (FLEx) output FieldWorks systematically writes some content that strict tooling rejects. Here is sil-lift's policy, so that real lexicons validate usefully: diff --git a/src/sil_lift/__init__.py b/src/sil_lift/__init__.py index e3da0d8..e8aaa4f 100644 --- a/src/sil_lift/__init__.py +++ b/src/sil_lift/__init__.py @@ -9,7 +9,7 @@ from typing import TYPE_CHECKING from ._canonical import canonicalize -from ._errors import LiftError, LiftParseError, LiftValidationError +from ._errors import LiftError, LiftParseError, LiftValidationError, LiftWriteError from ._extras import Extras from ._header import FieldDefinition, Header, Range, RangeElement from ._model import ( @@ -59,6 +59,7 @@ "LiftParseError", "LiftReader", "LiftValidationError", + "LiftWriteError", "LiftWriter", "MediaRef", "Multitext", diff --git a/src/sil_lift/_errors.py b/src/sil_lift/_errors.py index 43571f0..5a5f070 100644 --- a/src/sil_lift/_errors.py +++ b/src/sil_lift/_errors.py @@ -7,7 +7,7 @@ if TYPE_CHECKING: from ._validate import Problem -__all__ = ["LiftError", "LiftParseError", "LiftValidationError"] +__all__ = ["LiftError", "LiftParseError", "LiftValidationError", "LiftWriteError"] class LiftError(Exception): @@ -23,6 +23,19 @@ class LiftParseError(LiftError): """ +class LiftWriteError(LiftError): + """A model holds content that XML cannot represent, so it cannot be written. + + The only such content is a lone surrogate (U+D800-U+DFFF): a Python string + may hold one, an XML document may not, in any encoding. It cannot come from + a LIFT file — the parser rejects both spellings, a numeric character + reference and CESU-8/WTF-8 bytes — so it is always a string assigned + through the API. Raised by :meth:`Lexicon.save` and by everything else that + serializes (:meth:`Lexicon.changes`, validation, the streaming writer); + nothing is written, and the model is left untouched. + """ + + class LiftValidationError(LiftError): """Raised by the fail-fast validation wrappers on the first error-level :class:`~sil_lift.Problem` (warnings never raise).""" diff --git a/src/sil_lift/_model.py b/src/sil_lift/_model.py index e130f9f..8597750 100644 --- a/src/sil_lift/_model.py +++ b/src/sil_lift/_model.py @@ -565,7 +565,9 @@ def save(self, path: str | os.PathLike[str] | None = None) -> None: paths (they are shared with the original document, not copied). Raises :class:`ValueError` if no target path is available (none was - passed and the lexicon was not loaded from a file). + passed and the lexicon was not loaded from a file), and + :class:`~sil_lift.LiftWriteError` if the model holds content XML cannot + represent (a lone surrogate) — nothing is written in that case. """ from ._writer import render_document diff --git a/src/sil_lift/_validate.py b/src/sil_lift/_validate.py index a607445..5961640 100644 --- a/src/sil_lift/_validate.py +++ b/src/sil_lift/_validate.py @@ -24,6 +24,10 @@ (``missing-id`` opt-in via ``require_ids``). The codes are named on ``Problem.code`` below; ``docs/en/guides/validate.md`` tabulates each one's level and what it flags. + +A document that cannot be serialized at all — a lone surrogate assigned through +the API — is reported as a single ``lone-surrogate`` error instead of the layers +above, all of which need the rendered bytes. """ from __future__ import annotations @@ -35,7 +39,7 @@ from lxml import etree -from ._errors import LiftValidationError +from ._errors import LiftValidationError, LiftWriteError from ._model import GrammaticalInfo, Lexicon, _normalize_href from ._text import Multitext, Trait @@ -57,9 +61,7 @@ class Problem: """One validation finding, addressable to a file/entry/line.""" level: Literal["error", "warning"] - code: str # "schema", "duplicate-guid", "dangling-ref", "range-parent", - # "undefined-range-value", "normalization-mismatch", "duplicate-form-lang", - # "missing-media", "uri-not-rfc", "dangling-ranges-href", "missing-id" + code: str # e.g. "schema", "duplicate-guid", "dangling-ref", ... message: str file: Path | None = None entry_id: str | None = None @@ -101,11 +103,25 @@ def iter_lexicon_problems(lexicon: Lexicon, *, require_ids: bool = False) -> Ite # source, so line numbers keep matching the file on disk — and rendered # entry order always matches lexicon.entries, keeping the entry_lines # table aligned for semantic addressing even after edits or sort(). - data = render_document(lexicon) + # + # A lone surrogate makes the document unrenderable, so it is reported as + # the one finding and nothing else runs: every layer below needs the + # rendered bytes (the schema layers parse them, and the semantic layer + # addresses findings by their line numbers). Reporting it here is what + # makes it diagnosable at all — save() would raise the same refusal. + try: + data = render_document(lexicon) + except LiftWriteError as exc: + yield Problem("error", "lone-surrogate", str(exc), file=lexicon.path) + return entry_lines, problems = _schema_problems(data, lift_schema, lexicon.path) yield from problems for ranges_file in lexicon.ranges_files.values(): - rdata = render_ranges_document(ranges_file) + try: + rdata = render_ranges_document(ranges_file) + except LiftWriteError as exc: + yield Problem("error", "lone-surrogate", str(exc), file=ranges_file.path) + continue _, range_problems = _schema_problems(rdata, ranges_schema, ranges_file.path) yield from range_problems yield from _semantic_problems(lexicon, entry_lines, require_ids=require_ids) diff --git a/src/sil_lift/_writer.py b/src/sil_lift/_writer.py index 10fe2ee..fcb38d9 100644 --- a/src/sil_lift/_writer.py +++ b/src/sil_lift/_writer.py @@ -15,6 +15,8 @@ reassembles byte-identically. Snapshots are sha256 digests of canonical bytes, taken at parse time. + +Both paths refuse content XML cannot represent: see :func:`_guarded`. """ from __future__ import annotations @@ -26,6 +28,7 @@ from lxml import etree +from ._errors import LiftWriteError from ._extras import Extras from ._header import FieldDefinition, Header, Range, RangeElement from ._model import ( @@ -69,6 +72,34 @@ _FRAGMENT_PARSER = etree.XMLParser(resolve_entities=False, no_network=True) +def _guarded(what: str, build: Callable[[], bytes]) -> bytes: + """Serialize one node, turning unrepresentable content into a LIFT error. + + A Python string may hold a lone surrogate; XML may not, in any encoding. + lxml reports it as a bare ``UnicodeEncodeError`` from wherever the text or + attribute was set — deep inside the builders below, naming no node. Every + entry point that builds and serializes a node passes through here so the + failure arrives as a :class:`~sil_lift.LiftWriteError` that says which node + and which codepoint. Nothing else in a ``str`` is unencodable as UTF-8, so + anything else is re-raised untouched rather than mislabelled. + """ + try: + return build() + except UnicodeEncodeError as exc: + char = exc.object[exc.start] + if not 0xD800 <= ord(char) <= 0xDFFF: + raise + raise LiftWriteError( + f"{what}: U+{ord(char):04X} is a lone surrogate, which XML cannot " + f"represent in any encoding (in {exc.object!r})" + ) from exc + + +def _entry_label(entry: Entry) -> str: + name = entry.id or entry.guid + return f"entry {name!r}" if name else "entry (no id or guid)" + + # --- byte-reuse state (created by the reader, consumed here) --------------------- @@ -111,15 +142,15 @@ class _RangesSourceInfo: def entry_digest(entry: Entry) -> bytes: - return hashlib.sha256(_node_bytes(_entry_el(entry))).digest() + return hashlib.sha256(canonical_entry_bytes(entry)).digest() def header_digest(header: Header) -> bytes: - return hashlib.sha256(_node_bytes(_header_el(header))).digest() + return hashlib.sha256(canonical_header_bytes(header)).digest() def range_digest(range_: Range) -> bytes: - return hashlib.sha256(_node_bytes(_range_el(range_))).digest() + return hashlib.sha256(canonical_range_bytes(range_)).digest() # --- canonical building blocks --------------------------------------------------- @@ -624,24 +655,31 @@ def _node_bytes(el: etree._Element) -> bytes: def canonical_entry_bytes(entry: Entry) -> bytes: - return _node_bytes(_entry_el(entry)) + return _guarded(_entry_label(entry), lambda: _node_bytes(_entry_el(entry))) def canonical_header_bytes(header: Header) -> bytes: - return _node_bytes(_header_el(header)) + return _guarded("header", lambda: _node_bytes(_header_el(header))) + + +def canonical_range_bytes(range_: Range) -> bytes: + return _guarded(f"range {range_.id!r}", lambda: _node_bytes(_range_el(range_))) # --- document rendering ------------------------------------------------------------ def _root_open_bytes(lexicon: Lexicon) -> bytes: - el = _element( - "lift", - [("version", "0.13"), ("producer", lexicon.producer)], - lexicon.extra, - ) - serialized = etree.tostring(el, encoding="unicode").encode("utf-8") - return serialized[:-2] + b">" # "" -> "" + def build() -> bytes: + el = _element( + "lift", + [("version", "0.13"), ("producer", lexicon.producer)], + lexicon.extra, + ) + serialized = etree.tostring(el, encoding="unicode").encode("utf-8") + return serialized[:-2] + b">" # "" -> "" + + return _guarded(" root", build) def canonical_document( @@ -659,7 +697,7 @@ def canonical_document( if node.kind == "text": continue # character data at root level is not representable position = min(node.index, len(chunks)) - chunks.insert(position, node.xml.encode("utf-8") + b"\n") + chunks.insert(position, _guarded("root-level residue", node.xml.encode) + b"\n") parts = [b'\n', _root_open_bytes(lexicon), b"\n"] # Each chunk's trailing newline is the inter-chunk separator. Byte-reused # (untouched) regions end at ">", so append the newline they lack — without @@ -818,20 +856,32 @@ def render_document(lexicon: Lexicon) -> bytes: # --- .lift-ranges documents ---------------------------------------------------------- +def _ranges_root_open_bytes(ranges_file: RangesFile) -> bytes: + def build() -> bytes: + root = _element("lift-ranges", [], ranges_file.extra) + serialized = etree.tostring(root, encoding="unicode").encode("utf-8") + return serialized[:-2] + b">" # "" -> "" + + return _guarded(" root", build) + + def canonical_ranges_document( ranges_file: RangesFile, range_bytes: Callable[[Range], bytes] | None = None, ) -> bytes: if range_bytes is None: - range_bytes = lambda r: _node_bytes(_range_el(r)) # noqa: E731 + range_bytes = canonical_range_bytes chunks = [range_bytes(range_) for range_ in ranges_file.ranges] for node in sorted(ranges_file.extra._nodes, key=lambda n: n.index): if node.kind == "text": continue - chunks.insert(min(node.index, len(chunks)), node.xml.encode("utf-8") + b"\n") - root = _element("lift-ranges", [], ranges_file.extra) - serialized = etree.tostring(root, encoding="unicode").encode("utf-8") - parts = [b'\n', serialized[:-2] + b">", b"\n"] + fragment = _guarded("root-level residue", node.xml.encode) + chunks.insert(min(node.index, len(chunks)), fragment + b"\n") + parts = [ + b'\n', + _ranges_root_open_bytes(ranges_file), + b"\n", + ] # See canonical_document: reused regions need the newline they lack, or the # chunk runs into its neighbor. parts.extend(chunk if chunk.endswith(b"\n") else chunk + b"\n" for chunk in chunks) @@ -852,7 +902,7 @@ def fn(range_: Range) -> bytes: record, region = found if range_digest(range_) == record.digest: return source.data[region.start : region.end] - return _node_bytes(_range_el(range_)) + return canonical_range_bytes(range_) return fn @@ -883,9 +933,7 @@ def render_ranges_document(ranges_file: RangesFile) -> bytes: if root_unchanged: parts.append(data[source.root_open_start : source.root_open_end]) else: - root = _element("lift-ranges", [], ranges_file.extra) - serialized = etree.tostring(root, encoding="unicode").encode("utf-8") - parts.append(serialized[:-2] + b">") + parts.append(_ranges_root_open_bytes(ranges_file)) position = source.root_open_end range_index = 0 for region in source.children: diff --git a/tests/test_unicode.py b/tests/test_unicode.py index 6182cd2..ad84f8f 100644 --- a/tests/test_unicode.py +++ b/tests/test_unicode.py @@ -8,7 +8,7 @@ exactly the input that would break a scanner that guessed at character boundaries. -Two halves: +Three parts: - **Non-BMP content is ordinary content**: byte-identical round trips, exact scanner regions, verbatim untouched entries under edit, streaming reads and @@ -19,6 +19,10 @@ tools that mishandle UTF-16 internally emit (each surrogate half individually UTF-8-encoded instead of paired first). Both are rejected at parse time as ``LiftParseError``, not silently mangled. +- **One assigned through the API is refused on the way out**: XML cannot + represent it in any encoding, so every entry point that serializes raises + ``LiftWriteError`` naming the node and the codepoint, and validation reports + it as a ``lone-surrogate`` error rather than crashing. UTF-16 sources are covered here too: they load, and — being non-ASCII-compatible — re-serialize canonically as UTF-8 rather than byte-identically, which is the @@ -27,13 +31,18 @@ from __future__ import annotations -from pathlib import Path +from typing import TYPE_CHECKING import pytest import sil_lift -from sil_lift import LiftParseError, Span +from sil_lift import LiftError, LiftParseError, LiftWriteError, Span from sil_lift._scan import scan +from sil_lift._writer import _guarded + +if TYPE_CHECKING: + from collections.abc import Callable + from pathlib import Path PARTY = "\U0001f389" # PARTY POPPER, plane 1 (emoji) CJK_B = "\U00020000" # CJK Ext. B ideograph, plane 2 @@ -236,3 +245,112 @@ def test_utf16_source_with_non_bmp_loads_and_saves_as_utf8(tmp_path: Path) -> No again = tmp_path / "again.lift" reloaded.save(again) assert again.read_bytes() == written + + +# --- lone surrogates assigned through the API ----------------------------------- + +LONE = "a\ud800b" # the only way in: no LIFT file can carry U+D800 + + +def _lexicon_with_lone_surrogate() -> sil_lift.Lexicon: + lexicon = sil_lift.Lexicon() + entry = sil_lift.Entry(id="abat", guid="6b1b7ce6-4b3a-4d16-9a1f-8f1e3b0f2a03") + entry.lexical_unit["en"] = LONE + lexicon.entries.append(entry) + return lexicon + + +def test_save_refuses_a_lone_surrogate_and_writes_nothing(tmp_path: Path) -> None: + lexicon = _lexicon_with_lone_surrogate() + out = tmp_path / "lone.lift" + with pytest.raises(LiftWriteError) as caught: + lexicon.save(out) + message = str(caught.value) + assert "entry 'abat'" in message # which node + assert "U+D800" in message # which codepoint + assert not out.exists() + assert isinstance(caught.value, LiftError) # catchable with the base class + assert str(lexicon.entries[0].lexical_unit["en"]) == LONE # model untouched + + +def _stream_out(lexicon: sil_lift.Lexicon, path: Path) -> None: + with sil_lift.open_writer(path) as writer: + writer.write(lexicon.entries[0]) + + +@pytest.mark.parametrize( + "call", + [ + lambda lexicon, path: lexicon.save(path), + lambda lexicon, path: lexicon.changes(), + lambda lexicon, path: lexicon.changed_entries(), + _stream_out, + ], + ids=["save", "changes", "changed-entries", "streaming-write"], +) +def test_every_serializing_entry_point_refuses( + call: Callable[[sil_lift.Lexicon, Path], object], tmp_path: Path +) -> None: + """Not just save(): the change guard serializes to compare digests, and the + streaming writer builds the same entry elements. All three used to surface a + bare UnicodeEncodeError from inside lxml.""" + source = _write(tmp_path, "loaded.lift", NON_BMP_LIFT) + lexicon = sil_lift.load(source) # loaded, so the change guard has a baseline + lexicon.entries[0].lexical_unit["en"] = LONE + + with pytest.raises(LiftWriteError, match=r"U\+D800"): + call(lexicon, tmp_path / "out.lift") + + +def test_lone_surrogate_in_the_root_or_a_range_names_that_node(tmp_path: Path) -> None: + """The refusal covers every node the writer builds, not only entries.""" + lexicon = sil_lift.Lexicon(producer=LONE) + with pytest.raises(LiftWriteError, match=" root"): + lexicon.save(tmp_path / "root.lift") + + ranges_file = sil_lift.RangesFile() + ranges_file.ranges.append(sil_lift.Range(id=f"etymology{LONE}")) + with pytest.raises(LiftWriteError, match="range 'etymology"): + ranges_file.save(tmp_path / "companion.lift-ranges") + + +def test_validation_reports_a_lone_surrogate_instead_of_crashing() -> None: + """Validation renders the document first, so it cannot check an unrenderable + one — it reports the reason as its single finding.""" + (problem,) = _lexicon_with_lone_surrogate().iter_problems() + assert problem.level == "error" + assert problem.code == "lone-surrogate" + assert "U+D800" in problem.message + + +def test_validation_reports_a_lone_surrogate_in_a_companion(tmp_path: Path) -> None: + """An unwritable companion is addressed to the companion, and the .lift's own + layers keep running around it.""" + source = _write(tmp_path, "with-companion.lift", NON_BMP_LIFT) + lexicon = sil_lift.load(source) + ranges_file = lexicon.add_ranges_file(href="with-companion.lift-ranges") + ranges_file.add_range("etymology") + lexicon.save() # the companion exists on disk, so findings can address it + ranges_file.add_range(f"borrowed{LONE}") + # A finding on the .lift side, to show it is still reached. + lexicon.entries[1].relations.append(sil_lift.Relation(type="synonym", ref="nope")) + + problems = list(lexicon.iter_problems()) + (surrogate,) = [problem for problem in problems if problem.code == "lone-surrogate"] + assert surrogate.message.startswith("range 'borrowed") + assert "U+D800" in surrogate.message + assert surrogate.file is not None + assert surrogate.file.name == "with-companion.lift-ranges" + assert "dangling-ref" in {problem.code for problem in problems} + + +def test_guard_re_raises_anything_that_is_not_a_surrogate() -> None: + """Nothing else in a str is unencodable as UTF-8, so the guard must not + relabel a hypothetical other UnicodeEncodeError as a lone surrogate.""" + other = UnicodeEncodeError("ascii", "é", 0, 1, "ordinal not in range(128)") + + def build() -> bytes: + raise other + + with pytest.raises(UnicodeEncodeError): + _guarded("entry 'x'", build) From 3af77453e4ca7065c8d8b29c5fc4c75d8502eed9 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 21 Aug 2026 14:41:47 -0400 Subject: [PATCH 3/3] Clean up class doc --- src/sil_lift/_errors.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/sil_lift/_errors.py b/src/sil_lift/_errors.py index 5a5f070..fb52841 100644 --- a/src/sil_lift/_errors.py +++ b/src/sil_lift/_errors.py @@ -24,16 +24,7 @@ class LiftParseError(LiftError): class LiftWriteError(LiftError): - """A model holds content that XML cannot represent, so it cannot be written. - - The only such content is a lone surrogate (U+D800-U+DFFF): a Python string - may hold one, an XML document may not, in any encoding. It cannot come from - a LIFT file — the parser rejects both spellings, a numeric character - reference and CESU-8/WTF-8 bytes — so it is always a string assigned - through the API. Raised by :meth:`Lexicon.save` and by everything else that - serializes (:meth:`Lexicon.changes`, validation, the streaming writer); - nothing is written, and the model is left untouched. - """ + """An in-memory document holds content that XML cannot represent, so it cannot be written.""" class LiftValidationError(LiftError):