Skip to content

Commit 8480567

Browse files
authored
Merge pull request #6 from eduralph/fix/form-gramps-id-11707
Form: fix crash and surface clear errors for malformed XML (bug 11707)
2 parents 4f0d7ba + 49c8afb commit 8480567

6 files changed

Lines changed: 950 additions & 20 deletions

File tree

Form/editform.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@
2828
# Python modules
2929
# -------------------------------------------------------------------------
3030
from gi.repository import Gdk
31+
import logging
3132
import pickle
3233

34+
LOG = logging.getLogger(".FormGramplet")
35+
3336
# ------------------------------------------------------------------------
3437
#
3538
# GTK modules
@@ -77,6 +80,7 @@
7780
get_section_columns,
7881
get_form_citation,
7982
)
83+
from form_validator import split_family_title
8084
from entrygrid import EntryGrid
8185

8286
# ------------------------------------------------------------------------
@@ -114,6 +118,12 @@ def __init__(self, dbstate, uistate, track, event, citation, callback):
114118
self.citation = citation
115119
self.callback = callback
116120

121+
LOG.debug(
122+
"Opening EditForm for event %s, citation %s",
123+
event.get_handle() or "<new>",
124+
citation.get_handle() or "<new>",
125+
)
126+
117127
ManagedWindow.__init__(self, uistate, track, citation)
118128

119129
self.widgets = {}
@@ -1102,7 +1112,15 @@ def __init__(self, dbstate, uistate, track, event, citation, form_id, section):
11021112
hbox = Gtk.Box()
11031113

11041114
title = get_section_title(form_id, section)
1105-
title1, title2 = title.split("/")
1115+
title1, title2 = split_family_title(title)
1116+
if not title2:
1117+
LOG.warning(
1118+
"FamilySection for form '%s' section '%s' has title '%s' "
1119+
"without the expected 'X/Y' separator; second label will be empty",
1120+
form_id,
1121+
section,
1122+
title,
1123+
)
11061124

11071125
label = Gtk.Label(label="<b>%s</b>" % title1)
11081126
label.set_use_markup(True)

Form/form.py

Lines changed: 96 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
# ---------------------------------------------------------------
3030
import os
3131
import xml.dom.minidom
32+
import xml.parsers.expat
33+
import logging
3234

3335
# ---------------------------------------------------------------
3436
#
@@ -37,8 +39,14 @@
3739
# ---------------------------------------------------------------
3840
from gramps.gen.datehandler import parser
3941
from gramps.gen.config import config
40-
from gramps.gui.dialog import ErrorDialog, WarningDialog
41-
import logging
42+
from gramps.gui.dialog import ErrorDialog
43+
44+
# ---------------------------------------------------------------
45+
#
46+
# Gramps specific
47+
#
48+
# ---------------------------------------------------------------
49+
from form_validator import validate_form_dom, validate_form_element
4250

4351
LOG = logging.getLogger(".FormGramplet")
4452

@@ -111,7 +119,14 @@ class Form:
111119
A class to read form definitions from an XML file.
112120
"""
113121

114-
def __init__(self):
122+
def __init__(self, definition_dir=None):
123+
"""
124+
:param definition_dir: optional override for the directory the
125+
loader scans for ``form_*.xml`` / ``custom.xml`` files.
126+
Defaults to the directory containing this module. Exposed
127+
primarily so tests can point the loader at an isolated
128+
temporary directory.
129+
"""
115130
self.__references = {}
116131
self.__dates = {}
117132
self.__headings = {}
@@ -122,27 +137,90 @@ def __init__(self):
122137
self.__names = {}
123138
self.__section_types = {}
124139

140+
base_dir = definition_dir or os.path.dirname(__file__)
141+
LOG.debug("Loading form definitions from %s", base_dir)
125142
for file_name in definition_files:
126-
full_path = os.path.join(os.path.dirname(__file__), file_name)
143+
full_path = os.path.join(base_dir, file_name)
127144
if os.path.exists(full_path):
128-
try:
129-
self.__load_definitions(full_path)
130-
except Exception as e:
131-
WarningDialog(
132-
_("Failed to load Form definition file:\n%s\n") % full_path,
133-
)
134-
LOG.warning(
135-
"\nERROR: failed to load Form definition file.\n%s\nException:\n%s",
136-
full_path,
137-
str(e),
138-
)
139-
140-
def __load_definitions(self, definition_file):
141-
dom = xml.dom.minidom.parse(definition_file)
145+
self.__load_file(full_path)
146+
else:
147+
LOG.debug("Form definition file not present: %s", full_path)
148+
LOG.info(
149+
"Loaded %d form definition(s) from %s",
150+
len(self.__names),
151+
base_dir,
152+
)
153+
154+
def __load_file(self, full_path):
155+
"""
156+
Parse and validate a single form definition file, then load any
157+
well-formed ``<form>`` elements it contains.
158+
159+
Parse errors and structural validation errors are reported to the
160+
user through an :class:`ErrorDialog` and logged; malformed forms
161+
are skipped while valid forms from the same file are still loaded.
162+
"""
163+
LOG.debug("Parsing form definition file %s", full_path)
164+
try:
165+
dom = xml.dom.minidom.parse(full_path)
166+
except xml.parsers.expat.ExpatError as exc:
167+
ErrorDialog(
168+
_("XML syntax error in Form definition file"),
169+
"%s\n\n%s" % (full_path, exc),
170+
)
171+
LOG.warning(
172+
"XML syntax error in Form definition file %s: %s",
173+
full_path,
174+
exc,
175+
)
176+
return
177+
except Exception as exc:
178+
ErrorDialog(
179+
_("Failed to read Form definition file"),
180+
"%s\n\n%s" % (full_path, exc),
181+
)
182+
LOG.warning(
183+
"Failed to read Form definition file %s: %s", full_path, exc
184+
)
185+
return
186+
187+
errors = validate_form_dom(dom)
188+
if errors:
189+
ErrorDialog(
190+
_("Invalid Form definition file"),
191+
"%s\n\n%s" % (full_path, "\n".join(errors)),
192+
)
193+
LOG.warning(
194+
"Invalid Form definition file %s:\n%s",
195+
full_path,
196+
"\n".join(errors),
197+
)
198+
199+
try:
200+
self.__load_definitions(dom)
201+
finally:
202+
dom.unlink()
203+
204+
def __load_definitions(self, dom):
142205
top = dom.getElementsByTagName("forms")
206+
if not top:
207+
return
143208

144209
for form in top[0].getElementsByTagName("form"):
210+
if validate_form_element(form):
211+
# Errors for this form were already surfaced by the
212+
# file-level validator — skip it so __load_definitions
213+
# never touches a malformed element.
214+
skipped_id = (
215+
form.attributes["id"].value
216+
if "id" in form.attributes
217+
else "<missing id>"
218+
)
219+
LOG.debug("Skipping invalid <form> '%s'", skipped_id)
220+
continue
221+
145222
id = form.attributes["id"].value
223+
LOG.debug("Loading form '%s'", id)
146224
self.__names[id] = form.attributes["title"].value
147225
self.__types[id] = form.attributes["type"].value
148226
if "reference" in form.attributes:
@@ -194,7 +272,6 @@ def __load_definitions(self, definition_file):
194272
self.__columns[id][role].append(
195273
(attr_text, long_text, int(size_text))
196274
)
197-
dom.unlink()
198275

199276
def get_form_ids(self):
200277
"""Return a list of ids for all form definitions."""

Form/form_validator.py

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
#
2+
# Gramps - a GTK+/GNOME based genealogy program
3+
#
4+
# Copyright (C) 2026 Eduard Ralph
5+
#
6+
# This program is free software; you can redistribute it and/or modify
7+
# it under the terms of the GNU General Public License as published by
8+
# the Free Software Foundation; either version 2 of the License, or
9+
# (at your option) any later version.
10+
#
11+
# This program is distributed in the hope that it will be useful,
12+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
# GNU General Public License for more details.
15+
#
16+
# You should have received a copy of the GNU General Public License
17+
# along with this program; if not, write to the Free Software
18+
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
19+
#
20+
21+
"""
22+
Pure-Python validation for Form addon XML definition files.
23+
24+
Kept free of GTK/Gramps imports so it can be unit-tested without a GUI
25+
environment.
26+
"""
27+
28+
# ------------------------
29+
# Python modules
30+
# ------------------------
31+
import xml.dom.minidom
32+
import xml.parsers.expat
33+
34+
35+
VALID_SECTION_TYPES = frozenset({"person", "family", "multi"})
36+
REQUIRED_FORM_ATTRS = ("id", "title", "type")
37+
38+
39+
def split_family_title(title: str) -> tuple[str, str]:
40+
"""
41+
Split a family-section title of the form ``'X/Y'`` into ``(X, Y)``.
42+
43+
Falls back gracefully when the separator is absent or the input is
44+
empty, so callers never raise ``ValueError`` on malformed XML.
45+
46+
:param title: the raw title string from the XML (may be empty)
47+
:returns: a two-tuple of title parts; the second element is empty
48+
when the title contains no ``'/'``
49+
"""
50+
if not title:
51+
return "", ""
52+
parts = title.split("/", 1)
53+
if len(parts) == 2:
54+
return parts[0], parts[1]
55+
return parts[0], ""
56+
57+
58+
def validate_form_element(form) -> list[str]:
59+
"""
60+
Validate a single ``<form>`` DOM element against the form schema.
61+
62+
:param form: a DOM element for a single form definition
63+
:returns: a list of human-readable error messages scoped to this
64+
form; empty when the form is valid
65+
"""
66+
errors: list[str] = []
67+
68+
form_id = (
69+
form.attributes["id"].value
70+
if "id" in form.attributes
71+
else "<missing id>"
72+
)
73+
for required in REQUIRED_FORM_ATTRS:
74+
if required not in form.attributes:
75+
errors.append(
76+
"Form '%s': missing required attribute '%s'"
77+
% (form_id, required)
78+
)
79+
80+
for section in form.getElementsByTagName("section"):
81+
role = (
82+
section.attributes["role"].value
83+
if "role" in section.attributes
84+
else ""
85+
)
86+
if not role:
87+
errors.append(
88+
"Form '%s': <section> is missing required attribute 'role'"
89+
% form_id
90+
)
91+
continue
92+
93+
if "type" not in section.attributes:
94+
errors.append(
95+
"Form '%s': section '%s' is missing required attribute 'type'"
96+
% (form_id, role)
97+
)
98+
continue
99+
100+
section_type = section.attributes["type"].value
101+
if not section_type:
102+
errors.append(
103+
"Form '%s': section '%s' has an empty 'type' attribute"
104+
% (form_id, role)
105+
)
106+
continue
107+
108+
if section_type not in VALID_SECTION_TYPES:
109+
errors.append(
110+
"Form '%s': section '%s' has invalid type '%s' "
111+
"(expected one of: %s)"
112+
% (
113+
form_id,
114+
role,
115+
section_type,
116+
", ".join(sorted(VALID_SECTION_TYPES)),
117+
)
118+
)
119+
continue
120+
121+
title = (
122+
section.attributes["title"].value
123+
if "title" in section.attributes
124+
else ""
125+
)
126+
if section_type == "family":
127+
parts = title.split("/")
128+
if len(parts) != 2 or not parts[0].strip() or not parts[1].strip():
129+
errors.append(
130+
"Form '%s': family section '%s' requires a title "
131+
"of the form 'Name1/Name2' (got '%s')"
132+
% (form_id, role, title)
133+
)
134+
135+
return errors
136+
137+
138+
def validate_form_dom(dom: xml.dom.minidom.Document) -> list[str]:
139+
"""
140+
Validate the structure of a parsed form definitions DOM.
141+
142+
Checks that:
143+
144+
* a ``<forms>`` root element exists;
145+
* each ``<form>`` element has ``id``, ``title`` and ``type`` attributes;
146+
* each ``<section>`` element has non-empty ``role`` and ``type``
147+
attributes;
148+
* each section's ``type`` is one of ``person``, ``family`` or ``multi``;
149+
* ``family``-type sections declare a title of the form ``'X/Y'`` with
150+
two non-empty parts.
151+
152+
:param dom: a parsed ``xml.dom.minidom.Document``
153+
:returns: a list of human-readable error messages; empty when the
154+
document is valid
155+
"""
156+
top = dom.getElementsByTagName("forms")
157+
if not top:
158+
return ["Missing <forms> root element"]
159+
160+
errors: list[str] = []
161+
for form in top[0].getElementsByTagName("form"):
162+
errors.extend(validate_form_element(form))
163+
return errors
164+
165+
166+
def parse_and_validate(path: str) -> tuple[xml.dom.minidom.Document | None, list[str]]:
167+
"""
168+
Parse ``path`` as XML and validate it against the form schema.
169+
170+
:param path: filesystem path to a form definitions XML file
171+
:returns: a ``(dom, errors)`` tuple. When parsing fails, ``dom`` is
172+
``None`` and ``errors`` contains a single description of
173+
the syntax error. When parsing succeeds, ``dom`` is the
174+
parsed document and ``errors`` lists any structural
175+
problems (empty when the file is valid).
176+
"""
177+
try:
178+
dom = xml.dom.minidom.parse(path)
179+
except xml.parsers.expat.ExpatError as exc:
180+
return None, ["XML syntax error: %s" % exc]
181+
except (OSError, ValueError) as exc:
182+
return None, ["Failed to read file: %s" % exc]
183+
return dom, validate_form_dom(dom)

0 commit comments

Comments
 (0)