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

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
 

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
 ~~~~~

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

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()