Skip to content

fix(httpv2)!: default RST_STREAM and GOAWAY to NO_ERROR (#2311) - #2337

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2311-httpv2-default-no-error
Oct 27, 2026
Merged

fix(httpv2)!: default RST_STREAM and GOAWAY to NO_ERROR (#2311)#2337
JarryShaw merged 1 commit into
mainfrom
fix/2311-httpv2-default-no-error

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 26, 2026

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style
  • make test passes, and a test case covers the change
  • Changelog entry: 1.5.0.rst (breaking change)

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2311, per the ruling (take the recommendation). Breaking: the default error code changes.

  • Fix: HTTP._make_http_rst_stream and HTTP._make_http_goaway defaulted error to HTTP_1_1_REQUIRED (0xd). A frame built without an error code therefore asked the peer to fall back to HTTP/1.1. Both now default to NO_ERROR (0x0), per RFC 9113 §7. The docstrings carry a versionchanged note.
  • Changelog: 1.5.0.rst records the breaking change, and CHANGELOG.md is regenerated with util/changelog_md.py.
  • Test: test_httpv2_error_default_2311_unit.py builds both frames with frame={}, pins the error-code octets as zero, and checks that an explicit error is still honoured.
  • Fail-without-fix: on origin/main, 2 failed (the last octet is \r, not \x00). On the head, 3 passed.

@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
@JarryShaw JarryShaw added breaking Breaks public-facing behaviour or API (apply alongside the type label) bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 26, 2026
@JarryShaw

JarryShaw commented Oct 26, 2026

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at a90b72a43 (Sonnet).

  • RFC 9113 §7, checked against the RFC text: NO_ERROR means the condition is not an error. That makes it the neutral default for both frames.
  • Defaults: RST_STREAM and GOAWAY now default to NO_ERROR, and an explicit HTTP_1_1_REQUIRED still packs 0000000d. No other HTTP/2 maker keeps the old default.
  • Changelog: the entry closes the "Changed" block in the file's breaking-change form, and util/changelog_md.py --check passes.
  • Fail-without-fix: on origin/main, 2 failed (trailing \r). On the head, 144 passed, and the neighbouring HTTP/2 tests gave 118 passed.
  • Non-blocking: the default frame=b'' still cannot build these frames. That is fix(httpv2): make's default frame=b'' cannot build frame types with a fixed-size payload #2274, which is blocked until this PR merges.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 26, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 26, 2026
@JarryShaw
JarryShaw force-pushed the fix/2311-httpv2-default-no-error branch from a90b72a to 1f032f8 Compare October 26, 2026 22:32
@JarryShaw

JarryShaw commented Oct 26, 2026

Copy link
Copy Markdown
Owner Author

Rebased a90b72a43 → 1f032f8ff onto main bfe184bd6. The patch-id is identical, so the GOOD TO GO verdict carries over to the new head.

The old head's Project tests failed on test_no_test_calls_unparse_outside_a_version_gate. That was not this PR. It came from main's #2306 scan test, which called ast.unparse outside a version gate; bfe184bd6 fixes it on main, and CI is re-running on the new head.

@github-actions

github-actions Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Coverage: 91.56% (unit tier, Python 3.15, 1f032f8ff, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 881 2618 867 92.11%
pcapkit/corekit 2589 74 816 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3023 23 1044 25 98.82%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17437 218 4586 185 98.15%
pcapkit/toolkit 869 36 298 10 95.20%
pcapkit/utilities 504 7 146 5 98.15%
pcapkit/vendor 4432 1988 1018 167 51.87%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw force-pushed the fix/2311-httpv2-default-no-error branch from 1f032f8 to eb18a59 Compare October 26, 2026 23:46
@JarryShaw

JarryShaw commented Oct 26, 2026

Copy link
Copy Markdown
Owner Author

Rebased 1f032f8ff → eb18a5907 onto main b7b02c0fa. The patch-id is identical to the old head — git's rebase applied every hunk, including the CHANGELOG.md/1.5.0.rst additions, with no textual conflict against #2312's changelog entry, so nothing in the diff changed.

The old head's CI was green (Required checks passed SUCCESS, 0 failures, KTLO clean). On the new head: tests/protocols/application (httpv2 and http), tests/test_docstring_contract.py and tests/test_tier_guard.py give 211 passed, and util/changelog_md.py --check passes.

Since the patch is unchanged and the old head was already green, the owner can bypass checks and merge once the new head's CI completes — this only re-establishes a path through the required contexts on top of main.

@JarryShaw
fix(httpv2)!: default RST_STREAM and GOAWAY to NO_ERROR (#2311)
325659a 
Problem: HTTP._make_http_rst_stream and HTTP._make_http_goaway defaulted
error to HTTP_1_1_REQUIRED (0xd), so a frame built without an error code
asked the peer to use HTTP/1.1 instead.

Fix: both default to NO_ERROR (0x0), per RFC 9113 section 7; docstrings
carry a versionchanged note, and the 1.5.0 changelog records the breaking
change (CHANGELOG.md regenerated).

Test: test_httpv2_error_default_2311_unit.py builds both frames with
frame={} and pins the emitted error-code octets as zero, and an explicit
error as still honoured.
@JarryShaw
JarryShaw force-pushed the fix/2311-httpv2-default-no-error branch from eb18a59 to 325659a Compare October 26, 2026 23:55
@JarryShaw

JarryShaw commented Oct 26, 2026

Copy link
Copy Markdown
Owner Author

Rebased again eb18a5907 → 325659a6a onto main 9dd46d8d4. This time it was a real conflict: #2309 (HIP REG_FAILED) landed on main between my last rebase and now, and both it and this PR append a "breaking change" bullet to the same spot in CHANGELOG.md and 1.5.0.rst. Resolved by keeping both bullets, in the order they already appear in 1.5.0.rst, then regenerating CHANGELOG.md from it with util/changelog_md.py rather than hand-editing the generated copy. git patch-id on pcapkit/protocols/application/httpv2.py and the test file is identical before and after — only the changelog resolution changed.

On the new head: tests/protocols/application (httpv2 and http), tests/test_docstring_contract.py and tests/test_tier_guard.py give 211 passed, and util/changelog_md.py --check passes.

My earlier note on this PR described a clean rebase against an older main; that rebase is superseded by this one. Not safe to bypass checks and merge until the new head's CI completes — flagging that explicitly since I said otherwise before #2309 landed.

@JarryShaw
JarryShaw merged commit 0f27e62 into main Oct 27, 2026
59 checks passed
@JarryShaw
JarryShaw deleted the fix/2311-httpv2-default-no-error branch October 27, 2026 00:05
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 27, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 27, 2026
This was referenced Oct 27, 2026
Merged
Closed
Merged
JarryShaw added a commit that referenced this pull request Oct 28, 2026
@JarryShaw
docs(changelog): backfill entries for the #2312-#2407 range (#2409)
7f59b3e 
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Two pairs share a
bullet, since each is the direct follow-up of the other: #2393 with
#2401, and #2405 with #2407. The diff on the entry file is purely
additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on the seven classes owning
a __proto__ registry (#2392), __proto_dest__ on the four enum registries
and Vendor.PROTO_DEST to generate it (#2403). Both PRs added the autodoc
entries, so both are rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
JarryShaw added a commit that referenced this pull request Oct 28, 2026
@JarryShaw
docs(changelog): backfill entries for the #2312-#2407 range (#2409)
813e9f8 
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Three pairs share a
bullet, since each is the direct follow-up or counterpart of the other:
#2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the
entry file is purely additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on each class owning a
__proto__ registry (#2392), __proto_dest__ on the enum registries and
Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes
and registries rather than counting them, per the no-counted-numbers rule
for the unreleased entry. Both PRs added the autodoc entries, so both are
rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
JarryShaw added a commit that referenced this pull request Oct 28, 2026
@JarryShaw
docs: backfill changelog entries for the #2312-#2407 range (#2412)
27c797d 
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Three pairs share a
bullet, since each is the direct follow-up or counterpart of the other:
#2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the
entry file is purely additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on each class owning a
__proto__ registry (#2392), __proto_dest__ on the enum registries and
Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes
and registries rather than counting them, per the no-counted-numbers rule
for the unreleased entry. Both PRs added the autodoc entries, so both are
rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix)

Projects

PyPCAPKit
Status: Done

Milestone

PyPCAPKit v1.5

Development

Successfully merging this pull request may close these issues.

fix(httpv2): RST_STREAM and GOAWAY default their error code to HTTP_1_1_REQUIRED

1 participant

@JarryShaw