diff --git a/CHANGELOG.md b/CHANGELOG.md index 3456de175f..6c25af1a0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -226,6 +226,7 @@ Layer-limited extraction: `layer='link'` no longer sets the termination flag on - **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)). - **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)). - **a breaking change to** HIP's `REG_FAILED` parameter: its first octet is the Failure Type, and it carries no lifetime ([RFC 8003 Section 4.5](https://datatracker.ietf.org/doc/html/rfc8003#section-4.5)), so `pcapkit.protocols.data.internet.hip.RegFailedParameter` replaces `lifetime` with `failure_type`, a `RegistrationFailure`, with no alias, and its `reg_type` holds `Registration` values rather than failure types. `HIP._make_param_reg_failed()` takes `failure_type` in place of `lifetime`, and `reg_failed` takes registration types. The octets are unchanged ([#2309](https://github.com/JarryShaw/PyPCAPKit/issues/2309)). +- **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)). #### Fixed diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 302df3ff15..3d7c944506 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -1498,6 +1498,12 @@ Changed types. :meth:`.HIP._make_param_reg_failed` takes ``failure_type`` in place of ``lifetime``, and ``reg_failed`` takes registration types. The octets are unchanged (:issue:`2309`). +* **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` 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 (:issue:`2311`). Fixed ~~~~~ diff --git a/pcapkit/protocols/application/httpv2.py b/pcapkit/protocols/application/httpv2.py index ed64f7f158..4e70f7f687 100644 --- a/pcapkit/protocols/application/httpv2.py +++ b/pcapkit/protocols/application/httpv2.py @@ -1251,7 +1251,7 @@ def _make_http_priority(self, frame: 'Optional[Data_PriorityFrame]' = None, *, ), cast('Flags', 0) def _make_http_rst_stream(self, frame: 'Optional[Data_RSTStreamFrame]' = None, *, - error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.HTTP_1_1_REQUIRED, + error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.NO_ERROR, error_default: 'Optional[int]' = None, error_namespace: 'Optional[dict[str, int] | dict[int, str] | Type[StdlibEnum] | Type[AenumEnum]]' = None, # pylint: disable=line-too-long error_reversed: 'bool' = False, @@ -1260,7 +1260,7 @@ def _make_http_rst_stream(self, frame: 'Optional[Data_RSTStreamFrame]' = None, * Args: frame: Frame data model. - error: Error code. + error: Error code, ``NO_ERROR`` unless given. error_default: Default value of error code. error_namespace: Namespace of error code. error_reversed: Reversed namespace of error code. @@ -1269,6 +1269,13 @@ def _make_http_rst_stream(self, frame: 'Optional[Data_RSTStreamFrame]' = None, * Returns: Constructed frame schema and updated flags. + .. versionchanged:: 1.5.0 + + ``error`` defaults to ``NO_ERROR``, the code :rfc:`9113#section-7` + gives a condition that is not the result of an error, rather than + ``HTTP_1_1_REQUIRED``, which asks the peer to use HTTP/1.1 instead + (:issue:`2311`). + """ if frame is not None: error_val = frame.error @@ -1430,7 +1437,7 @@ def _make_http_ping(self, frame: 'Optional[Data_PingFrame]' = None, *, def _make_http_goaway(self, frame: 'Optional[Data_GoawayFrame]' = None, *, last_reserved: 'int' = 0, last_sid: 'int' = 0, - error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.HTTP_1_1_REQUIRED, + error: 'Enum_ErrorCode | str | int | StdlibEnum | AenumEnum' = Enum_ErrorCode.NO_ERROR, error_default: 'Optional[int]' = None, error_namespace: 'Optional[dict[str, int] | dict[int, str] | Type[StdlibEnum] | Type[AenumEnum]]' = None, # pylint: disable=line-too-long error_reversed: 'bool' = False, @@ -1442,7 +1449,7 @@ def _make_http_goaway(self, frame: 'Optional[Data_GoawayFrame]' = None, *, frame: Frame data model. last_reserved: Reserved bit of the last stream identifier. last_sid: Last stream identifier. - error: Error code. + error: Error code, ``NO_ERROR`` unless given. error_default: Default value of error code. error_namespace: Namespace of error code. error_reversed: Reversed namespace of error code. @@ -1452,6 +1459,13 @@ def _make_http_goaway(self, frame: 'Optional[Data_GoawayFrame]' = None, *, Returns: Constructed frame schema and updated flags. + .. versionchanged:: 1.5.0 + + ``error`` defaults to ``NO_ERROR``, the code :rfc:`9113#section-7` + gives a condition that is not the result of an error, rather than + ``HTTP_1_1_REQUIRED``, which asks the peer to use HTTP/1.1 instead + (:issue:`2311`). + """ if frame is not None: last_reserved = frame.last_reserved diff --git a/tests/protocols/application/test_httpv2_error_default_2311_unit.py b/tests/protocols/application/test_httpv2_error_default_2311_unit.py new file mode 100644 index 0000000000..2240e942b3 --- /dev/null +++ b/tests/protocols/application/test_httpv2_error_default_2311_unit.py @@ -0,0 +1,68 @@ +# -*- coding: utf-8 -*- +"""HTTP/2 ``RST_STREAM`` and ``GOAWAY`` frames default to ``NO_ERROR``. + +GitHub issue #2311: ``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 told the peer to use +HTTP/1.1 instead. Both default to ``NO_ERROR`` (``0x0``), the code +:rfc:`9113#section-7` gives a condition that is not the result of an error. +An explicit ``error`` is still honoured. + +Each frame is built with ``frame={}``, because the default ``frame=b''`` does +not build these fixed-payload frames at all (#2274). Every case builds its own +octets in memory and reads no capture. Classes are imported inside each test, +after :func:`~tests._support.reimport_once_per_class`. + +""" + +import unittest +from typing import Any + +from tests._support import reimport_once_per_class + +#: ``RST_STREAM`` on stream 1 carrying ``NO_ERROR``. +RST_STREAM_NO_ERROR = bytes.fromhex('000004030000000001' '00000000') + +#: ``GOAWAY`` on stream 0, last stream 0, carrying ``NO_ERROR``. +GOAWAY_NO_ERROR = bytes.fromhex('000008070000000000' '00000000' '00000000') + + +class TestHTTPv2ErrorCodeDefault(unittest.TestCase): + """Pin the error code a frame built without ``error`` carries.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def build(self, name: 'str', sid: 'int', frame: 'dict[str, object]') -> 'Any': + """Build the HTTP/2 frame ``name`` from the ``frame`` mapping.""" + from pcapkit.const.http.frame import Frame + from pcapkit.protocols.application.httpv2 import HTTP + + return HTTP(type=Frame[name], sid=sid, frame=frame) + + def test_rst_stream_defaults_to_no_error(self) -> None: + from pcapkit.const.http.error_code import ErrorCode + + http = self.build('RST_STREAM', 1, {}) + self.assertEqual(bytes(http), RST_STREAM_NO_ERROR) + self.assertEqual(http.info.error, ErrorCode.NO_ERROR) + + def test_goaway_defaults_to_no_error(self) -> None: + from pcapkit.const.http.error_code import ErrorCode + + http = self.build('GOAWAY', 0, {}) + self.assertEqual(bytes(http), GOAWAY_NO_ERROR) + self.assertEqual(http.info.error, ErrorCode.NO_ERROR) + + def test_explicit_error_is_still_emitted(self) -> None: + from pcapkit.const.http.error_code import ErrorCode + + for name, sid, offset in (('RST_STREAM', 1, 9), ('GOAWAY', 0, 13)): + with self.subTest(frame=name): + http = self.build(name, sid, {'error': ErrorCode.HTTP_1_1_REQUIRED}) + self.assertEqual(bytes(http)[offset:offset + 4], bytes.fromhex('0000000d')) + self.assertEqual(http.info.error, ErrorCode.HTTP_1_1_REQUIRED) + + +if __name__ == '__main__': + unittest.main()