Skip to content

fix(tcp): NS flag is written by make() but never parsed #1098

Closed

Description

@JarryShaw

Describe the bug
make() packs the NS bit, but the reader has #ns=bool(schema.offset['ns']) commented out (tcp.py:474). data/transport/tcp.py:52 also has #ns commented out, so Flags has no ns field.

Reproduction
TCP(srcport=1, dstport=2, ns=True) sets byte 12 to 0x51, so NS is on the wire. The parsed Flags has no ns field, and a from_data rebuild clears NS with no warning.

Expected behavior
NS round-trips. If it was dropped on purpose (RFC 8311 made NS historic), make() should reject ns instead.

System information
main at b10e7951b, CPython 3.14.7, Linux.

Additional context
Found during the #719 second-round sweep, then reproduced on main.

Activity

  1. added
    fixPull requests that fix a defect (fix: subject prefix)
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 6, 2026
  2. JarryShaw commented on Oct 6, 2026

    @JarryShaw
    OwnerAuthor

    Decision needed: should NS be exposed in Flags?

    #1114 restores it so the flag round-trips. But docs/source/pcapkit/protocols/transport/tcp.rst:538-541, written in #413 on 2026-09-16, says NS is deliberately left out because RFC 3540 was reclassified as Historic. The code change that commented it out (2e39aeb99, "minor revision for TCP on typings") gives no reason. So the footnote may be a reason written after the fact, not an original design decision.

    1. Expose it. Keep fix(tcp): NS flag is written by make() but never parsed #1114 and delete the footnote. NS then round-trips like every other flag.
    2. Keep it hidden. Drop fix(tcp): NS flag is written by make() but never parsed #1114's model change, and have make() reject ns, or ignore it and say so. Today make() writes a bit that parsing never reports.

    Either way the footnote changes. My recommendation is (1), because the bit is on the wire and make() already accepts it.

  3. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 6, 2026
  4. JarryShaw commented on Oct 6, 2026

    @JarryShaw
    OwnerAuthor

    take (1)

  5. JarryShaw commented on Oct 6, 2026

    @JarryShaw
    OwnerAuthor

    Ruling from the maintainer: option (1), expose NS. #1114 keeps the change to the Flags model, and the tcp.rst footnote saying NS is deliberately hidden is being removed.

  6. removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 6, 2026
  7. added 2 commits that reference this issue on Oct 6, 2026
    45e84f0
    eeed87f
  8. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    fixPull requests that fix a defect (fix: subject prefix)

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions