From da170622e6020b657e438ffb2c619e164c2c57b0 Mon Sep 17 00:00:00 2001 From: "Teppei.F" <37261985+T3pp31@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:34:28 +0900 Subject: [PATCH 1/3] fields: preserve containers when copying fields AI-Assisted: yes (Codex) --- scapy/fields.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ test/fields.uts | 28 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/scapy/fields.py b/scapy/fields.py index 23c8fa774c3..e7610f7fe6d 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -312,6 +312,54 @@ class _FieldContainer(object): """ __slots__ = ["fld"] + def copy(self): + # type: () -> Any + def _copy_attr(attr): + # type: (Any) -> Any + if hasattr(attr, "copy"): + return attr.copy() + return attr + + cls = self.__class__ + other = cls.__new__(cls) + + if hasattr(self, "__dict__"): + other.__dict__ = { + key: _copy_attr(value) + for key, value in self.__dict__.items() + } + + copied_slots = set() + for base in cls.__mro__: + slots = base.__dict__.get("__slots__", ()) + if isinstance(slots, str): + slots = (slots,) + for slot in slots: + if slot in ("__dict__", "__weakref__") or slot in copied_slots: + continue + slot_descriptor = base.__dict__.get(slot) + if getattr(cls, slot, None) is not slot_descriptor: + continue + copied_slots.add(slot) + try: + value = getattr(self, slot) + except AttributeError: + continue + setattr(other, slot, _copy_attr(value)) + + return other + + def __setattr__(self, attr, value): + # type: (str, Any) -> None + try: + object.__setattr__(self, attr, value) + except AttributeError as ex: + try: + fld = object.__getattribute__(self, "fld") + except AttributeError: + raise ex + setattr(fld, attr, value) + def __getattr__(self, attr): # type: (str) -> Any return getattr(self.fld, attr) diff --git a/test/fields.uts b/test/fields.uts index 38263f2a49f..a127056a1aa 100644 --- a/test/fields.uts +++ b/test/fields.uts @@ -15,6 +15,34 @@ #Field("foo", None, fmt="I").getfield(None, b"\x12\x34\x56\x78ABCD") #assert _ == ("ABCD",0x12345678) += FieldContainer copy +~ core field + +field_container = Emph(ByteField("foo", 0)) +field_container_copy = field_container.copy() + +assert type(field_container_copy) is Emph +assert type(field_container_copy.fld) is ByteField +assert field_container_copy.fld is not field_container.fld +assert field_container_copy.name == "foo" +assert field_container_copy.default == 0 + +class TEST_FIELD_CONTAINER_COPY_SOURCE(Packet): + fields_desc = [Emph(ByteField("foo", 0))] + +class TEST_FIELD_CONTAINER_COPY_TARGET(Packet): + fields_desc = [TEST_FIELD_CONTAINER_COPY_SOURCE, ByteField("bar", 0)] + foo = 7 + +source_field = TEST_FIELD_CONTAINER_COPY_SOURCE.fields_desc[0] +target_field = TEST_FIELD_CONTAINER_COPY_TARGET.fields_desc[0] + +assert type(target_field) is Emph +assert type(target_field.fld) is ByteField +assert target_field.fld is not source_field.fld +assert target_field.default == 7 +assert source_field.default == 0 + = ConditionnalField class ~ core field From 5443e61c648e09857c3c5092dc86cc29d2d12b40 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 26 Jun 2026 16:28:52 +0000 Subject: [PATCH 2/3] fields: simplify _FieldContainer.copy() and fix __setattr__ delegation Address review feedback on PR #5009: - Replace the MRO-based copy implementation with a simpler approach that creates a new container and copies each __slots__ entry, calling copy() on contained fields where available. This preserves the wrapper type (the bug was __getattr__ delegating copy to the wrapped field). - Guard __setattr__ delegation when the attribute is defined on the container class (e.g. MultipleTypeField's read-only fld property). Co-authored-by: Teppei.F --- scapy/fields.py | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/scapy/fields.py b/scapy/fields.py index e7610f7fe6d..b5fde672e8a 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -314,39 +314,12 @@ class _FieldContainer(object): def copy(self): # type: () -> Any - def _copy_attr(attr): - # type: (Any) -> Any - if hasattr(attr, "copy"): - return attr.copy() - return attr - - cls = self.__class__ - other = cls.__new__(cls) - - if hasattr(self, "__dict__"): - other.__dict__ = { - key: _copy_attr(value) - for key, value in self.__dict__.items() - } - - copied_slots = set() - for base in cls.__mro__: - slots = base.__dict__.get("__slots__", ()) - if isinstance(slots, str): - slots = (slots,) - for slot in slots: - if slot in ("__dict__", "__weakref__") or slot in copied_slots: - continue - slot_descriptor = base.__dict__.get(slot) - if getattr(cls, slot, None) is not slot_descriptor: - continue - copied_slots.add(slot) - try: - value = getattr(self, slot) - except AttributeError: - continue - setattr(other, slot, _copy_attr(value)) - + other = self.__class__.__new__(self.__class__) + for slot in self.__slots__: + val = object.__getattribute__(self, slot) + if hasattr(val, "copy"): + val = val.copy() + object.__setattr__(other, slot, val) return other def __setattr__(self, attr, value): @@ -354,6 +327,9 @@ def __setattr__(self, attr, value): try: object.__setattr__(self, attr, value) except AttributeError as ex: + for cls in type(self).__mro__: + if attr in cls.__dict__: + raise ex try: fld = object.__getattribute__(self, "fld") except AttributeError: From 46699fa5e3ca8503f1b2007d61d9737abc70a3da Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 26 Jun 2026 16:35:36 +0000 Subject: [PATCH 3/3] Reply to PR #5009 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Conversation comment] Thanks for the review! I've pushed an update addressing both comments: - copy(): Simplified the implementation. The issue wasn't that we shouldn't call the sub-field's copy() at all — it's that without an explicit _FieldContainer.copy(), container.copy() was resolved via __getattr__ and returned only container.fld.copy() (a plain ByteField instead of Emph). The new version creates a new container instance and copies each __slots__ entry, calling .copy() on values that support it (e.g. the wrapped field). - __setattr__: Added a guard so delegation only happens for attributes that aren't defined on the container class. Assignments like mtf.fld = ... on MultipleTypeField now correctly raise AttributeError instead of silently mutating the selected underlying field. Rebased onto current master. All test/fields.uts tests pass, including the new regression test. [Reply to gpotter2 on copy()] Good point — I've simplified it significantly. We can't just return self.fld.copy() though: that was the original bug. Without an explicit copy() on _FieldContainer, the call goes through __getattr__ and ends up as self.fld.copy(), which returns a bare ByteField instead of an Emph wrapper (see #4657). The new implementation is much shorter: create a new container via __new__, iterate over __slots__, and call .copy() on slot values where available. This keeps the wrapper type while still deep-copying the contained field. [Reply to Copilot on __setattr__] Fixed — __setattr__ now checks whether attr is defined on the container class (via the MRO __dict__) before delegating to the wrapped field. Read-only descriptors like MultipleTypeField.fld will raise the original AttributeError instead of being forwarded to the selected underlying field. Co-authored-by: Teppei.F