Skip to content

Commit 8412d3f

Browse files
committed
fix: raise instead of silently corrupting output on an unresolvable delimiter sentinel
nickname_delimiters/maiden_delimiters string values that don't resolve to a real Constants.regexes key used to fall back to RegexTupleManager's EMPTY_REGEX default. That's not a harmless no-op: EMPTY_REGEX matches at every character position, so handle_match() fired repeatedly and appended '' into the bucket's list each time, producing a truthy whitespace-only nickname/maiden while leaving the intended delimiter's content (e.g. literal parentheses) unstripped elsewhere in the name. Found independently by two review agents with working reproductions while auditing PR #199. Now raises ValueError naming the bad key instead. To keep Constants(regexes=<minimal custom set>) working (confirmed via the existing test_override_regex), nickname_delimiters is only seeded with a built-in name if it's actually present in the regexes passed to the constructor -- so a caller who deliberately drops e.g. "parenthesis" from a custom regexes set doesn't end up with a dangling sentinel that looks like a mistake.
1 parent cddb09f commit 8412d3f

3 files changed

Lines changed: 53 additions & 5 deletions

File tree

nameparser/config/__init__.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,14 @@ def __init__(self,
485485
# routes to without losing that live link. maiden_delimiters starts
486486
# empty -- maiden is off until a caller routes a delimiter to it.
487487
# See issue #22.
488+
# Only seed a built-in name if it's actually present in self.regexes --
489+
# a caller who overrides regexes with a minimal custom set (dropping
490+
# e.g. "parenthesis" entirely) shouldn't end up with a dangling
491+
# string sentinel that parse_nicknames() would treat as a mistake.
492+
# See parse_nicknames()'s fail-loud check on an unresolvable sentinel.
488493
self.nickname_delimiters = TupleManager[re.Pattern[str] | str]({
489-
'quoted_word': 'quoted_word',
490-
'double_quotes': 'double_quotes',
491-
'parenthesis': 'parenthesis',
494+
name: name for name in ('quoted_word', 'double_quotes', 'parenthesis')
495+
if name in self.regexes
492496
})
493497
self.maiden_delimiters = TupleManager[re.Pattern[str] | str]()
494498
self.patronymic_name_order = patronymic_name_order

nameparser/parser.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -922,15 +922,33 @@ def _handle(m: 're.Match[str]') -> str:
922922
# delimiter value names a Constants.regexes entry, resolved via
923923
# getattr() so overriding e.g. self.C.regexes.parenthesis keeps
924924
# working; anything else is already a compiled pattern, added
925-
# directly by a caller.
925+
# directly by a caller. Unlike a caller directly querying
926+
# self.C.regexes (where RegexTupleManager's EMPTY_REGEX default for
927+
# an unknown attribute is harmless -- the caller sees the pattern and
928+
# can react to it), a bad string here is an internal cross-reference
929+
# the delimiter dict itself is responsible for keeping valid.
930+
# EMPTY_REGEX matches the empty string at every position, so
931+
# silently falling back to it would not just skip the delimiter --
932+
# handle_match() would fire on every zero-width match and append ''
933+
# into the bucket's list repeatedly, producing a truthy
934+
# whitespace-only nickname/maiden while leaving the real delimited
935+
# content (e.g. literal parentheses) unstripped. Fail loudly instead.
926936
for bucket, delimiters in (
927937
('nickname', self.C.nickname_delimiters),
928938
('maiden', self.C.maiden_delimiters),
929939
):
930940
target_list = getattr(self, bucket + '_list')
931941
_handle_match = handle_match(target_list)
932942
for raw_pattern in delimiters.values():
933-
_re = raw_pattern if isinstance(raw_pattern, re.Pattern) else getattr(self.C.regexes, raw_pattern)
943+
if isinstance(raw_pattern, re.Pattern):
944+
_re = raw_pattern
945+
elif raw_pattern in self.C.regexes:
946+
_re = getattr(self.C.regexes, raw_pattern)
947+
else:
948+
raise ValueError(
949+
f"{bucket}_delimiters references unknown regexes key {raw_pattern!r}. "
950+
f"Known regexes keys: {sorted(self.C.regexes)}"
951+
)
934952
self._full_name = _re.sub(_handle_match, self._full_name)
935953

936954
def squash_emoji(self) -> None:

tests/test_nicknames.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,29 @@ def test_maiden_appears_in_as_dict_via_routing(self) -> None:
282282
C.maiden_delimiters['parenthesis'] = C.nickname_delimiters.pop('parenthesis')
283283
hn = HumanName("Baker (Johnson), Jenny", constants=C)
284284
self.assertEqual(hn.as_dict()['maiden'], "Johnson")
285+
286+
def test_unresolvable_string_sentinel_raises(self) -> None:
287+
# A string value in nickname_delimiters/maiden_delimiters that
288+
# doesn't name a real regexes key used to silently fall back to
289+
# EMPTY_REGEX, which matches at every position and corrupts parsing
290+
# (appends '' into the bucket repeatedly, and leaves the intended
291+
# delimiter's content unstripped elsewhere in the name). It must
292+
# raise instead.
293+
C = Constants()
294+
C.nickname_delimiters['typo'] = 'parenthesus'
295+
with pytest.raises(ValueError):
296+
HumanName("Jenny (Johnson) Baker", constants=C)
297+
298+
def test_routing_same_delimiter_to_both_buckets_nickname_wins(self) -> None:
299+
# Misuse case: assigning the same key into both dicts instead of
300+
# moving it with pop() (as the docs instruct). nickname_delimiters is
301+
# processed first in parse_nicknames()'s bucket loop, so it consumes
302+
# the match via re.sub() before maiden_delimiters ever sees it --
303+
# maiden stays empty. Pinning this precedence so it doesn't silently
304+
# change if the bucket processing order is ever reordered.
305+
C = Constants()
306+
C.maiden_delimiters['parenthesis'] = C.regexes['parenthesis']
307+
hn = HumanName("Baker (Johnson), Jenny", constants=C)
308+
self.m(hn.nickname, "Johnson", hn)
309+
self.m(hn.maiden, "", hn)
310+

0 commit comments

Comments
 (0)