From d0a7d7497f817039133b09560f49c414ad367270 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 6 Oct 2026 14:53:04 -0400 Subject: [PATCH] fix(corekit): ProtoChain.__add__ returns stale protocols/aliases __add__ built its result with copy.copy(self), which carried the left operand's cached protocols/aliases into the merged chain. Build a fresh instance through __new__ instead, as from_list does, and drop the now unused copy import. __add__ is the only method that copies the chain; a += b falls back to it and rebinds. Closes #1099 --- pcapkit/corekit/protochain.py | 7 ++- ...st_protochain_add_stale_cache_1099_unit.py | 61 +++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 tests/corekit/test_protochain_add_stale_cache_1099_unit.py diff --git a/pcapkit/corekit/protochain.py b/pcapkit/corekit/protochain.py index 3a761f8205..6d0f8410f1 100644 --- a/pcapkit/corekit/protochain.py +++ b/pcapkit/corekit/protochain.py @@ -9,7 +9,6 @@ """ import collections.abc -import copy from typing import TYPE_CHECKING, overload from pcapkit.utilities.compat import cached_property @@ -260,6 +259,8 @@ def __add__(self, other: 'ProtoChain') -> 'ProtoChain': Merged protocol chain. """ - new = copy.copy(self) - new.__data__ += other.__data__ + # build a fresh instance, so the cached ``protocols`` and + # ``aliases`` of ``self`` are not carried over + new = self.__class__.__new__(self.__class__) + new.__data__ = self.__data__ + other.__data__ return new diff --git a/tests/corekit/test_protochain_add_stale_cache_1099_unit.py b/tests/corekit/test_protochain_add_stale_cache_1099_unit.py new file mode 100644 index 0000000000..278ef7b770 --- /dev/null +++ b/tests/corekit/test_protochain_add_stale_cache_1099_unit.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #1099: :meth:`ProtoChain.__add__ ` +must not carry the left operand's cached ``protocols``/``aliases`` into the result. + +Both are :func:`~functools.cached_property` values stored on the instance, so a +result built by shallow-copying the left operand kept them from before the merge. +""" +from __future__ import annotations + +import unittest + +from pcapkit.corekit.protochain import ProtoChain +from pcapkit.protocols.internet.ipv4 import IPv4 +from pcapkit.protocols.link.ethernet import Ethernet +from pcapkit.protocols.transport.tcp import TCP + + +class ProtoChainAddCacheTests(unittest.TestCase): + def _check(self, *, warm: bool) -> None: + left = ProtoChain(Ethernet) + right = ProtoChain(TCP, basis=ProtoChain(IPv4)) # TCP:IPv4 + if warm: + self.assertEqual(left.protocols, (Ethernet,)) + self.assertEqual(left.aliases, ('Ethernet',)) + self.assertEqual(right.protocols, (TCP, IPv4)) + + merged = left + right + + self.assertEqual(merged.protocols, (Ethernet, TCP, IPv4)) + self.assertEqual(merged.aliases, ('Ethernet', 'TCP', 'IPv4')) + self.assertEqual(merged.chain, 'Ethernet:TCP:IPv4') + self.assertEqual(len(merged), 3) + + # neither operand is mutated, whether or not its cache was warm + self.assertEqual(left.protocols, (Ethernet,)) + self.assertEqual(left.aliases, ('Ethernet',)) + self.assertEqual(left.chain, 'Ethernet') + self.assertEqual(right.chain, 'TCP:IPv4') + + def test_add_after_reading_cached_properties(self) -> None: + self._check(warm=True) + + def test_add_without_prior_read(self) -> None: + self._check(warm=False) + + def test_inplace_add_rebinds_without_mutating_the_original(self) -> None: + original = ProtoChain(Ethernet) + self.assertEqual(original.protocols, (Ethernet,)) + + chain = original + chain += ProtoChain(IPv4) + + self.assertIsNot(chain, original) + self.assertEqual(chain.protocols, (Ethernet, IPv4)) + self.assertEqual(chain.aliases, ('Ethernet', 'IPv4')) + self.assertEqual(original.protocols, (Ethernet,)) + self.assertEqual(original.chain, 'Ethernet') + + +if __name__ == '__main__': + unittest.main()