From ae4d69cfd9c60df22c885318ee64b6a08817e887 Mon Sep 17 00:00:00 2001 From: Angie Xiang Date: Mon, 9 Oct 2023 12:11:23 +0000 Subject: [PATCH] fix(core): prevent perpetual re-election in case of sequence node overflow Once sequence node is incremented beyond 2147483647, the sequence overflows into the negative space. When getting predecessors, cast the node sequence from String to int before comparing. Closes #730 --- kazoo/recipe/lock.py | 4 ++-- kazoo/tests/test_lock.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/kazoo/recipe/lock.py b/kazoo/recipe/lock.py index 59c6d8f3..b0f38f79 100644 --- a/kazoo/recipe/lock.py +++ b/kazoo/recipe/lock.py @@ -276,7 +276,7 @@ def _get_predecessor(self, node): (e.g. rlock), this and also edge cases where the lock's ephemeral node is gone. """ - node_sequence = node[len(self.prefix) :] + node_sequence = int(node[len(self.prefix) :]) children = self.client.get_children(self.path) found_self = False # Filter out the contenders using the computed regex @@ -284,7 +284,7 @@ def _get_predecessor(self, node): for child in children: match = self._contenders_re.search(child) if match is not None: - contender_sequence = match.group(1) + contender_sequence = int(match.group(1)) # Only consider contenders with a smaller sequence number. # A contender with a smaller sequence number has a higher # priority. diff --git a/kazoo/tests/test_lock.py b/kazoo/tests/test_lock.py index 397e971f..0d1ecbbe 100644 --- a/kazoo/tests/test_lock.py +++ b/kazoo/tests/test_lock.py @@ -798,7 +798,36 @@ def _thread(sem, event, timeout): class TestSequence(unittest.TestCase): + def test_get_predecessor(self): + """Validate selection of predecessors.""" + pyLock_prefix = "514e5a831836450cb1a56c741e990fd8__lock__" + pyLock_predecessor = f"{pyLock_prefix}0000000030" + pyLock = f"{pyLock_prefix}0000000031" + pyLock_successor = f"{pyLock_prefix}0000000032" + children = ["hello", pyLock_predecessor, "world", pyLock, pyLock_successor] + client = MagicMock() + client.get_children.return_value = children + lock = Lock(client, "test") + assert lock._get_predecessor(pyLock) == pyLock_predecessor + + def test_get_predecessor_with_overflowed_sequence(self): + """Validate selection of predecessors with negative sequence. + + This can occur in case of an integer overflow, if the sequence counter is + incremented beyond 2147483647. + """ + pyLock_prefix = "514e5a831836450cb1a56c741e990fd8__lock__" + pyLock_predecessor = f"{pyLock_prefix}-0000000032" + pyLock = f"{pyLock_prefix}-0000000031" + pyLock_successor = f"{pyLock_prefix}-0000000030" + children = ["hello", pyLock_predecessor, "world", pyLock, pyLock_successor] + client = MagicMock() + client.get_children.return_value = children + lock = Lock(client, "test") + assert lock._get_predecessor(pyLock) == pyLock_predecessor + + def test_get_predecessor_no_predecessor(self): """Validate selection of predecessors.""" goLock = "_c_8eb60557ba51e0da67eefc47467d3f34-lock-0000000031" pyLock = "514e5a831836450cb1a56c741e990fd8__lock__0000000032"