Skip to content

fix(protocols)!: refuse Quick-Start values their fields cannot carry - #2630

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2617-quick-start
Nov 2, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2617-quick-start

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — a bug fix

Description of your pull request and other information

The Quick-Start rate codec, which had four makers and seven readers that disagreed, becomes one pair on ProtocolBase: _read_qs_rate before _make_qs_rate. IPv4, HOPOPT, IPv6_Opts and TCP all use it. Per the owner's ruling on #2617, a rate the field cannot carry raises ProtocolError everywhere, so there is no clamp. The valid range is 0, or 80 to 2,621,439 kbps. HOPOPT and IPv6_Opts gain the unassigned-function maker that IPv4 has, so every reader has a maker. Each pair round-trips both ways. TTL, nonce and reserved are range-checked through _make_uint as well.

Valid input builds the same bytes as before: every rate from 0 to 3,000,000, all 1,024 parse cases, and all 23 captures. Rates between two codes still round down, as on main; for example, 100 becomes 80. A few tests pin behaviour main already had, so they pass there by design: RoundTripTests, the past-four-bits refusal, the bounds builds, the Report TTL and TTL Diff.

Breaking: these inputs built packets in 1.5.x and now raise:

A Sonnet cross-review checked the codec against RFC 4782 and returned GOOD TO GO. Its findings are folded in.

Closes #2617

…2617)

- Move the RFC 4782 rate codec into a ProtocolBase reader/maker pair,
  _read_qs_rate and _make_qs_rate, called by IPv4, HOPOPT, IPv6-Opts and
  TCP in place of their own copies.
- Raise ProtocolError for a rate other than 0 or 80..2621439 kbps, and, via
  _make_uint, for a TTL, nonce or reserved value outside its bits, where
  the makers clamped or wrapped them, wrote reserved bits into the nonce,
  or raised ValueError, OverflowError or FieldValueError.
- HOPOPT and IPv6-Opts make an unassigned QS function as an unassigned
  option, the form their readers keep one in, as IPv4 does.
- Tests: the codec, every refusal, and make -> parse and parse -> rebuild
  for every rate code; TCP's clamp pin becomes a refusal.
@JarryShaw JarryShaw added this to the PyPCAPKit v1.6 milestone Nov 2, 2026
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) breaking Breaks public-facing behaviour or API (apply alongside the type label) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 48c3e9ea3. A Sonnet cross-review checked the codec against RFC 4782 §3.1, §3.2 and §4.2, and swept every rate from 0 to 2,621,444 against main's formulas. Valid rates encode identically, and everything outside 0 or 80..2,621,439 kbps raises. 816 parse wires rebuild byte-exactly, and all 16 mutations it tried were caught. Its findings are folded in: TTL, nonce and reserved now go through _make_uint, and the commit scope is protocols. TCP's TTL Diff already masks mod 256, as the RFC defines, so it needed no change.

@JarryShaw
JarryShaw merged commit 71d9c1b into main Nov 2, 2026
14 checks passed
@JarryShaw
JarryShaw deleted the fix/2617-quick-start branch November 2, 2026 04:53
@github-project-automation github-project-automation Bot moved this to Done in PyPCAPKit Nov 2, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 2, 2026
JarryShaw added a commit that referenced this pull request Nov 2, 2026
…in the constants sweep (#2617)

#2630 added ProtocolBase._read_qs_rate and _make_qs_rate. Their literals are
RFC 4782 section 3.1's rate unit, K = 40 kbps, and the 2 ** 16 bound the
four-bit rate code cannot reach, which the sweep read as copies of
IPV6_HEADER_LEN and RECORD_HEADER_LEN. Four NOT_A_COPY entries say why.

test_rfc_constants_one_home_1739_unit.py and test_conventions_doc_claims.py pass.
JarryShaw added a commit that referenced this pull request Nov 2, 2026
…types and the registry split into 1.6.0 (#2449)

- Breaking: Quick-Start values their fields cannot carry raise, reversing
  1.5.0's round-down of a TCP rate under 80 kbps (#2630).
- Folded into unreleased entries: tls12_cid records and DTLSContext
  (#2631), LinkType 105 and 127 in Frame and PCAP-NG (#2632), and
  DTLS.analyze_handshake behind dtls_handshake and packet (#2634).
- Changed: foundation.registry.protocols split into per-layer submodules
  (#2635); 1.6.0's register_* roles and 1.5.0's three register_protocol*
  roles point at their submodules, 1.5.0's text unchanged.
- The conventions entry records the per-layer wrapper placement (#2635)
  and parse ownership (#2634). No entry for #2629, which changes no
  released behaviour.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(internet): the Quick-Start rate codec raises in IPv4 and clamps in TCP

1 participant