Skip to content

Fix Packet.fields_desc type annotation to match runtime behavior - #5187

Open
Ts-Boom wants to merge 1 commit into
secdev:masterfrom
Ts-Boom:fix-fields-desc-type
Open

Ts-Boom wants to merge 1 commit into
secdev:masterfrom
Ts-Boom:fix-fields-desc-type

Conversation

@Ts-Boom

@Ts-Boom Ts-Boom commented Sep 19, 2026

Copy link
Copy Markdown

Description

Packet.fields_desc was typed as ClassVar[List[AnyField]], but at
runtime Packet_metaclass.__new__ explicitly handles the case where
elements can be Packet_metaclass instances (references to another
packet's fields_desc):

# scapy/base_classes.py ~line 372
current_fld = dct["fields_desc"]
# type: List[Union[scapy.fields.Field[Any, Any], Packet_metaclass]]

This means the annotation was incorrect for the pre-resolution state of the list, causing type checkers to flag valid usage like:

fields_desc = [AnotherPacket, ByteField('x', 0)]

Fix

Changed the annotation from List[AnyField] to List[Union[AnyField, Packet_metaclass]].
Packet_metaclass is already imported in packet.py (line 44) and Union is already in the typing imports — no new dependencies added.

References

AI Disclosure

No AI tools were used.

…dev#5018)

At runtime, Packet_metaclass.__new__ explicitly handles the case where
elements in fields_desc are Packet_metaclass instances (references to
another Packet's fields_desc), not just Field instances:

    if isinstance(fld_or_pkt, Packet_metaclass):
        for pkt_fld in fld_or_pkt.fields_desc:
            ...  # base_classes.py, line ~372

The class-level annotation declared fields_desc as List[AnyField],
which does not include Packet_metaclass. This caused type checkers to
flag valid usage such as:

    fields_desc = [MyOtherPacket, ByteField('x', 0)]

Updated the annotation to List[Union[AnyField, Packet_metaclass]] to
accurately reflect what the list can contain before Packet_metaclass
resolution flattens the references into plain Field instances.

Closes: secdev#5018
AI-Assisted: no
@Ts-Boom
Ts-Boom force-pushed the fix-fields-desc-type branch from 64c0471 to ac66137 Compare September 19, 2026 21:02
@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.36%. Comparing base (94a853b) to head (ac66137).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5187      +/-   ##
==========================================
- Coverage   80.37%   80.36%   -0.01%     
==========================================
  Files         375      375              
  Lines       97675    97675              
==========================================
- Hits        78506    78501       -5     
- Misses      19169    19174       +5     
Files with missing lines Coverage Δ
scapy/packet.py 84.79% <100.00%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Packet.fields_desc type annotation is inconsistent with runtime behavior

1 participant