Skip to content

Commit 1f032f8

Browse files
committed
fix(httpv2)!: default RST_STREAM and GOAWAY to NO_ERROR (#2311)
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.
1 parent bfe184b commit 1f032f8

4 files changed

Lines changed: 93 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ Layer-limited extraction: `layer='link'` no longer sets the termination flag on
225225
- **a breaking change to** `NTPTimestamp`: it and `PMIPv6Timestamp` are `Info` classes in `pcapkit.protocols.data.internet.mh` rather than tuples, and `NTPTimestamp` keeps no alias at its 1.4.x home in `pcapkit.protocols.internet.mh`. As Mappings they no longer index, unpack, compare or hash as a tuple does; `.seconds` and `.fraction` still work, and the dumpers write both as keys ([#1886](https://github.com/JarryShaw/PyPCAPKit/issues/1886), [#1887](https://github.com/JarryShaw/PyPCAPKit/pull/1887)).
226226
- **a breaking change to** rebuilding from `to_dict()`: the payload is dispatched only from the `__next_name__` and `__next_type__` keys, and the IPv6 extension-header chain only from `__exthdr__`, never from key order, which on PyPy follows an earlier instance's attribute layout and so picked the wrong layer. `to_dict()`, iteration and `len()` carry the dunder keys a new `Info.__included__` names, and `Info.__excluded__` only keeps a key out of `repr`, `str` and the dumps. A hand-written dict needs those keys, `__short_read__` among them, to rebuild, and a custom `Info` lists its own dunder keys in `__included__` ([#2003](https://github.com/JarryShaw/PyPCAPKit/issues/2003), [#2014](https://github.com/JarryShaw/PyPCAPKit/pull/2014)).
227227
- **a breaking change to** `PCAPNG.context`: the block's section `Context`, which it returned from 1.0.0 through 1.4.x, is `PCAPNG.section_context`, with no alias, and `context` is the caller-supplied `ContextRegistry` every protocol inherits from `ProtocolBase`, on a Section Header Block too. `PCAPNG.section_context` raises `UnsupportedCall` where `context` did: on a block that holds no section, such as a Section Header Block the `PCAPNG` engine has not assigned the section it opens. Code reading `block.context` for the section reads `block.section_context` ([#2172](https://github.com/JarryShaw/PyPCAPKit/issues/2172), [#2205](https://github.com/JarryShaw/PyPCAPKit/pull/2205)).
228+
- **a breaking change to** the HTTP/2 `RST_STREAM` and `GOAWAY` builders: a frame constructed without `error` carries `NO_ERROR` (`0x0`), the code [RFC 9113 Section 7](https://datatracker.ietf.org/doc/html/rfc9113#section-7) gives a condition that is not the result of an error, rather than `HTTP_1_1_REQUIRED` (`0xd`), which asks the peer to use HTTP/1.1 instead. A `make` that omits `error` emits different octets; one that passes it is unchanged ([#2311](https://github.com/JarryShaw/PyPCAPKit/issues/2311)).
228229

229230
#### Fixed
230231

‎docs/source/changelog/1.5.0.rst‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,6 +1489,12 @@ Changed
14891489
:class:`~pcapkit.foundation.engines.pcapng.PCAPNG` engine has not assigned the section
14901490
it opens. Code reading ``block.context`` for the section reads
14911491
``block.section_context`` (:issue:`2172`, :pr:`2205`).
1492+
* **a breaking change to** the HTTP/2 ``RST_STREAM`` and ``GOAWAY`` builders: a frame
1493+
constructed without ``error`` carries ``NO_ERROR`` (``0x0``), the code
1494+
:rfc:`9113#section-7` gives a condition that is not the result of an error, rather
1495+
than ``HTTP_1_1_REQUIRED`` (``0xd``), which asks the peer to use HTTP/1.1 instead.
1496+
A ``make`` that omits ``error`` emits different octets; one that passes it is
1497+
unchanged (:issue:`2311`).
14921498

14931499
Fixed
14941500
~~~~~

‎pcapkit/protocols/application/httpv2.py‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,7 +1251,7 @@ def _make_http_priority(self, frame: 'Optional[Data_PriorityFrame]' = None, *,
12511251
), cast('Flags', 0)
12521252

12531253
def _make_http_rst_stream(self, frame: 'Optional[Data_RSTStreamFrame]' = None, *,
1254-
error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.HTTP_1_1_REQUIRED,
1254+
error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.NO_ERROR,
12551255
error_default: 'Optional[int]' = None,
12561256
error_namespace: 'Optional[dict[str, int] | dict[int, str] | Type[StdlibEnum] | Type[AenumEnum]]' = None, # pylint: disable=line-too-long
12571257
error_reversed: 'bool' = False,
@@ -1260,7 +1260,7 @@ def _make_http_rst_stream(self, frame: 'Optional[Data_RSTStreamFrame]' = None, *
12601260
12611261
Args:
12621262
frame: Frame data model.
1263-
error: Error code.
1263+
error: Error code, ``NO_ERROR`` unless given.
12641264
error_default: Default value of error code.
12651265
error_namespace: Namespace of error code.
12661266
error_reversed: Reversed namespace of error code.
@@ -1269,6 +1269,13 @@ def _make_http_rst_stream(self, frame: 'Optional[Data_RSTStreamFrame]' = None, *
12691269
Returns:
12701270
Constructed frame schema and updated flags.
12711271
1272+
.. versionchanged:: 1.5.0
1273+
1274+
``error`` defaults to ``NO_ERROR``, the code :rfc:`9113#section-7`
1275+
gives a condition that is not the result of an error, rather than
1276+
``HTTP_1_1_REQUIRED``, which asks the peer to use HTTP/1.1 instead
1277+
(:issue:`2311`).
1278+
12721279
"""
12731280
if frame is not None:
12741281
error_val = frame.error
@@ -1430,7 +1437,7 @@ def _make_http_ping(self, frame: 'Optional[Data_PingFrame]' = None, *,
14301437
def _make_http_goaway(self, frame: 'Optional[Data_GoawayFrame]' = None, *,
14311438
last_reserved: 'int' = 0,
14321439
last_sid: 'int' = 0,
1433-
error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.HTTP_1_1_REQUIRED,
1440+
error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.NO_ERROR,
14341441
error_default: 'Optional[int]' = None,
14351442
error_namespace: 'Optional[dict[str, int] | dict[int, str] | Type[StdlibEnum] | Type[AenumEnum]]' = None, # pylint: disable=line-too-long
14361443
error_reversed: 'bool' = False,
@@ -1442,7 +1449,7 @@ def _make_http_goaway(self, frame: 'Optional[Data_GoawayFrame]' = None, *,
14421449
frame: Frame data model.
14431450
last_reserved: Reserved bit of the last stream identifier.
14441451
last_sid: Last stream identifier.
1445-
error: Error code.
1452+
error: Error code, ``NO_ERROR`` unless given.
14461453
error_default: Default value of error code.
14471454
error_namespace: Namespace of error code.
14481455
error_reversed: Reversed namespace of error code.
@@ -1452,6 +1459,13 @@ def _make_http_goaway(self, frame: 'Optional[Data_GoawayFrame]' = None, *,
14521459
Returns:
14531460
Constructed frame schema and updated flags.
14541461
1462+
.. versionchanged:: 1.5.0
1463+
1464+
``error`` defaults to ``NO_ERROR``, the code :rfc:`9113#section-7`
1465+
gives a condition that is not the result of an error, rather than
1466+
``HTTP_1_1_REQUIRED``, which asks the peer to use HTTP/1.1 instead
1467+
(:issue:`2311`).
1468+
14551469
"""
14561470
if frame is not None:
14571471
last_reserved = frame.last_reserved
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# -*- coding: utf-8 -*-
2+
"""HTTP/2 ``RST_STREAM`` and ``GOAWAY`` frames default to ``NO_ERROR``.
3+
4+
GitHub issue #2311: ``HTTP._make_http_rst_stream`` and
5+
``HTTP._make_http_goaway`` defaulted ``error`` to ``HTTP_1_1_REQUIRED``
6+
(``0xd``), so a frame built without an error code told the peer to use
7+
HTTP/1.1 instead. Both default to ``NO_ERROR`` (``0x0``), the code
8+
:rfc:`9113#section-7` gives a condition that is not the result of an error.
9+
An explicit ``error`` is still honoured.
10+
11+
Each frame is built with ``frame={}``, because the default ``frame=b''`` does
12+
not build these fixed-payload frames at all (#2274). Every case builds its own
13+
octets in memory and reads no capture. Classes are imported inside each test,
14+
after :func:`~tests._support.reimport_once_per_class`.
15+
16+
"""
17+
18+
import unittest
19+
from typing import Any
20+
21+
from tests._support import reimport_once_per_class
22+
23+
#: ``RST_STREAM`` on stream 1 carrying ``NO_ERROR``.
24+
RST_STREAM_NO_ERROR = bytes.fromhex('000004030000000001' '00000000')
25+
26+
#: ``GOAWAY`` on stream 0, last stream 0, carrying ``NO_ERROR``.
27+
GOAWAY_NO_ERROR = bytes.fromhex('000008070000000000' '00000000' '00000000')
28+
29+
30+
class TestHTTPv2ErrorCodeDefault(unittest.TestCase):
31+
"""Pin the error code a frame built without ``error`` carries."""
32+
33+
def setUp(self) -> None:
34+
reimport_once_per_class(self)
35+
36+
def build(self, name: 'str', sid: 'int', frame: 'dict[str, object]') -> 'Any':
37+
"""Build the HTTP/2 frame ``name`` from the ``frame`` mapping."""
38+
from pcapkit.const.http.frame import Frame
39+
from pcapkit.protocols.application.httpv2 import HTTP
40+
41+
return HTTP(type=Frame[name], sid=sid, frame=frame)
42+
43+
def test_rst_stream_defaults_to_no_error(self) -> None:
44+
from pcapkit.const.http.error_code import ErrorCode
45+
46+
http = self.build('RST_STREAM', 1, {})
47+
self.assertEqual(bytes(http), RST_STREAM_NO_ERROR)
48+
self.assertEqual(http.info.error, ErrorCode.NO_ERROR)
49+
50+
def test_goaway_defaults_to_no_error(self) -> None:
51+
from pcapkit.const.http.error_code import ErrorCode
52+
53+
http = self.build('GOAWAY', 0, {})
54+
self.assertEqual(bytes(http), GOAWAY_NO_ERROR)
55+
self.assertEqual(http.info.error, ErrorCode.NO_ERROR)
56+
57+
def test_explicit_error_is_still_emitted(self) -> None:
58+
from pcapkit.const.http.error_code import ErrorCode
59+
60+
for name, sid, offset in (('RST_STREAM', 1, 9), ('GOAWAY', 0, 13)):
61+
with self.subTest(frame=name):
62+
http = self.build(name, sid, {'error': ErrorCode.HTTP_1_1_REQUIRED})
63+
self.assertEqual(bytes(http)[offset:offset + 4], bytes.fromhex('0000000d'))
64+
self.assertEqual(http.info.error, ErrorCode.HTTP_1_1_REQUIRED)
65+
66+
67+
if __name__ == '__main__':
68+
unittest.main()

0 commit comments

Comments
 (0)