From a8f804787e3f0579d2a65227d59d061cfc640a3a Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 25 Sep 2026 18:11:13 -0400 Subject: [PATCH] fix(http): identify the HTTP version before parsing, not by trial and error (#800) `HTTP._guess_version` decided the version by trial-parsing -- try `httpv1`, and if it declines, try `httpv2` -- which answers "did a parser accept this?" where the question is "what is this?", and got both directions wrong. The HTTP/2 connection preface came back `version='2'` only because `httpv2` read its `b'PRI'` as a declared frame length of 5,265,993, and garbage text came back `version='2'` the same way. #802 made that inconsistency a refusal, which left a real HTTP/2 connection opening reported as not-HTTP at all. * positively identify HTTP/2 by prefix-comparing the first 24 octets against `b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'` (RFC 9113 section 3.4), then parse the frame that follows the preface rather than the preface itself; a preface counts as header, so `length` includes it and `info.packet` no longer reports the tail of the preface as payload * positively identify HTTP/1 with `_test_start_line`, which applies the parser's own anchored patterns and unpackings, and commit to that version instead of re-offering a malformed HTTP/1 message to the HTTP/2 arm * keep the trial parse as a last resort only, for a mid-stream segment that carries neither preface nor start line; no frame-header heuristic is added * leave `Upgrade: h2c` out of scope -- it is stateful and correctly HTTP/1.1 on the wire -- and document why, with a test pinning it `pytest tests/protocols/application/` 118 passed, 420 subtests; `unittest` 59 tests OK; protochain over all 23 sample captures (1604 frames, 231 HTTP-bearing) byte-identical to `4530424df`; coverage 100% on both changed modules. --- .../pcapkit/protocols/application/http.rst | 4 + .../pcapkit/protocols/application/httpv1.rst | 2 + pcapkit/protocols/application/http.py | 157 +++++++- pcapkit/protocols/application/httpv1.py | 65 ++++ tests/protocols/application/test_http_unit.py | 354 +++++++++++++++++- 5 files changed, 568 insertions(+), 14 deletions(-) diff --git a/docs/source/pcapkit/protocols/application/http.rst b/docs/source/pcapkit/protocols/application/http.rst index 05d6a25d67..4096b65386 100644 --- a/docs/source/pcapkit/protocols/application/http.rst +++ b/docs/source/pcapkit/protocols/application/http.rst @@ -28,6 +28,10 @@ and :class:`HTTP/2 `. .. automethod:: _guess_version + .. autoattribute:: _preface_length + +.. autodata:: pcapkit.protocols.application.http._HTTP2_PREFACE + .. rubric:: Footnotes .. [*] https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol diff --git a/docs/source/pcapkit/protocols/application/httpv1.rst b/docs/source/pcapkit/protocols/application/httpv1.rst index b97f1c3193..899ad947da 100644 --- a/docs/source/pcapkit/protocols/application/httpv1.rst +++ b/docs/source/pcapkit/protocols/application/httpv1.rst @@ -40,6 +40,8 @@ as below: .. automethod:: _read_http_header .. automethod:: _read_http_body +.. autofunction:: pcapkit.protocols.application.httpv1._test_start_line + Auxiliary Data -------------- diff --git a/pcapkit/protocols/application/http.py b/pcapkit/protocols/application/http.py index df89530e96..fc7f4aa710 100644 --- a/pcapkit/protocols/application/http.py +++ b/pcapkit/protocols/application/http.py @@ -29,6 +29,17 @@ __all__ = ['HTTP'] +#: The HTTP/2 connection preface (:rfc:`9113#section-3.4`). A client opens every +#: HTTP/2 connection -- with prior knowledge, over TLS, or after an upgrade -- +#: by sending exactly these 24 octets, and the sequence is *designed* to be +#: identifiable without parsing: it is a well-formed HTTP/1.1 request line whose +#: method ``PRI`` is reserved and permanently unregistered, so no valid HTTP/1 +#: message can begin with it and a prefix compare cannot false-positive on one. +#: That is what makes it a positive identification rather than a heuristic, and +#: it is why :meth:`HTTP._guess_version` tests it before attempting any parse +#: (#800). +_HTTP2_PREFACE = b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n' + class HTTP(Application[_PT, _ST], Generic[_PT, _ST]): """This class implements all protocols in HTTP family. @@ -42,6 +53,16 @@ class HTTP(Application[_PT, _ST], Generic[_PT, _ST]): #: Saved subclass protocol data (only for HTTP base class). _http: 'HTTP[_PT, _ST]' + #: Octets consumed ahead of the identified version's own header -- in practice + #: the 24-octet HTTP/2 connection preface, when :meth:`_guess_version` + #: identified one and parsed the frame that follows it. They belong to this + #: packet's header rather than to its payload, so :meth:`read` adds them to + #: :attr:`length`; without that, ``ProtocolBase.__init__``'s + #: ``self._info.__update__(packet=self.packet.payload)`` slices the payload + #: from octet 9 of a buffer whose frame starts at octet 24 and reports the + #: tail of the preface as packet payload. See #800. + _preface_length = 0 + #: This class is a version dispatcher rather than a protocol with a header of #: its own, so its construction keywords cannot be enumerated: :meth:`make` #: declares only ``version`` and forwards everything else to @@ -129,7 +150,7 @@ def read(self, length: 'Optional[int]' = None, *, raise ProtocolError(f'HTTP/{version}: invalid format') from error self._version = http.version - self._length = http.length + self._length = http.length + self._preface_length self._http = http return http.info @@ -195,7 +216,20 @@ def _make_data(cls, data: '_PT') -> 'dict[str, Any]': # type: ignore[override] return protocol._make_data(data) # type: ignore[arg-type] def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': - """Guess HTTP version. + """Identify the HTTP version of the payload, and parse it with that version. + + The payload is *identified* first and trial-parsed only as a last resort. + Until #800 there was no identification step at all: both versions were + tried in turn and whichever parser did not object was taken as the + answer, which answers "did a parser accept this?" where the question is + "what is this?" -- and got both directions wrong. The HTTP/2 connection + preface came back ``version='2'`` only because ``httpv2.HTTP`` read its + leading ``b'PRI'`` as a 24-bit declared frame length of 5,265,993, and + ``b'foo bar baz\\r\\nX: y\\r\\n\\r\\n'`` -- not HTTP at all -- came back + ``version='2'`` the same way. #799/#802 closed the second of those by + requiring a frame's declared length to be backed by its buffer, but that + left the preface *unidentifiable*: a real HTTP/2 connection opening is + refused by both arms and reported as not-HTTP. Args: length: Length of packet data. @@ -207,13 +241,121 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': Parsed packet data. Raises: - ProtocolError: If no version in the family accepts the payload. This - is the *only* exception this method raises for an unparseable - payload -- a candidate failing in any other way is a fall-through - and not an answer. + ProtocolError: If no version in the family accepts the payload, or if + an identified version's own parser refuses it. This is the + *only* exception this method raises for an unparseable payload -- + a candidate failing in any other way, on a version that was not + positively identified, is a fall-through and not an answer. + + Note: + Two things this deliberately does **not** decide, because one payload + with no flow context cannot: + + * **An ``Upgrade: h2c`` exchange** (:rfc:`7540#section-3.2`, + deprecated but not removed by :rfc:`9113#section-3.1`) stays + HTTP/1.1 here, and is *correctly* HTTP/1.1: on the wire the upgrade + request and its ``101 Switching Protocols`` response are HTTP/1.1 + messages and parse as such. The switch takes effect only *after* + the ``101``, so deciding that later segments of the same connection + are HTTP/2 needs per-connection state keyed on the 4-tuple, and + this method is handed a single payload with no such context. + Recognising the ``Upgrade: h2c`` field is possible; acting on it is + not, so it is left alone rather than half-implemented. + * **A mid-stream segment** -- a bare HTTP/2 frame header with no + preface ahead of it, or an opaque HTTP/1 message body -- is + genuinely undecidable from one payload, and the honest answer is + the ``Raw`` that an escaping ``ProtocolError`` becomes under + :func:`~pcapkit.protocols.misc.raw.beholder`. In particular there + is deliberately *no* heuristic on the nine-octet frame header + ("type at most 9, reserved bit clear"): that misfires on binary + HTTP/1 bodies, which is precisely how garbage text was classified + HTTP/2 to begin with. A self-consistent bare frame is still parsed + as HTTP/2, but by the fall-through below -- on the parser's own + length/type consistency rules -- rather than by a guess dressed up + as identification. """ - # NOTE: The two arms suppress different sets, and the asymmetry is + # NOTE: Positive identification, before any parse attempt. The preface is + # a fixed 24-octet sequence that only an HTTP/2 client sends and that no + # valid HTTP/1 message can begin with (see ``_HTTP2_PREFACE``), so a + # prefix compare is an *answer* rather than evidence: it cannot + # false-positive on HTTP/1, and it needs no parse to reach. + # + # ``length`` bounds the compare as well as ``self._data``, because a + # caller may hand this method fewer octets than the buffer holds, and + # claiming a preface out of octets that were not part of this payload + # would be the same kind of accident this change removes. + preface_len = len(_HTTP2_PREFACE) + if length >= preface_len and self._data[:preface_len] == _HTTP2_PREFACE: + from pcapkit.protocols.application.httpv2 import HTTP as HTTPv2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + + # The preface is not a frame -- the frames begin after it + # (:rfc:`9113#section-3.4` requires a ``SETTINGS`` frame immediately + # following), so it is skipped rather than fed to ``httpv2.HTTP``, + # which is how it used to be misread as framing. + if length == preface_len: + # A preface with nothing after it *is* HTTP/2, but this library's + # HTTP/2 data model is one frame per packet and has no + # representation for a frameless segment, so there is nothing to + # return. Refused with a message that says which of the two it + # was -- an HTTP/2 connection opening truncated at the preface, + # not an unrecognised payload -- because that distinction is the + # whole point of identifying before parsing. + raise ProtocolError('HTTP/2: connection preface with no frame') + + try: + http = HTTPv2(self._data[preface_len:length], length - preface_len, **kwargs) + except ProtocolError: + raise + # NOTE: Converted, unlike the HTTP/1 commit below, because this route + # is new and has no escaping-error contract to keep: the old arm 2 + # suppressed :exc:`struct.error` and fell through to ``unknown HTTP + # version``, so a preface followed by a frame that trips the #805 + # residual (an inner field shortfall, e.g. a 16-octet ``GOAWAY``) + # must still reach the caller as something it can catch, not as a + # bare stdlib error. Same normalisation, and the same reasoning, as + # ``read``'s explicit ``version=`` path above. + except (ValueError, struct.error) as error: + raise ProtocolError('HTTP/2: invalid format') from error + + # The preface is header, not payload -- see ``_preface_length``. + self._preface_length = preface_len + return http + + from pcapkit.protocols.application.httpv1 import HTTP as HTTPv1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + from pcapkit.protocols.application.httpv1 import \ + _test_start_line # isort: skip # pylint: disable=import-outside-toplevel + + # NOTE: The second identification: an HTTP/1 ``request-line`` or + # ``status-line``, tested with the very patterns ``httpv1.HTTP`` parses + # with (``httpv1.py``'s ``_RE_METHOD``, ``_RE_VERSION`` and + # ``_RE_STATUS``, all anchored), so the predicate and the parser cannot + # disagree about what HTTP/1 looks like. + # + # Identified means *committed*: a malformed HTTP/1 message is reported as + # the malformed HTTP/1 message it is, instead of being handed to the + # HTTP/2 arm, which accepts any self-consistent nine-octet-or-longer + # buffer. Re-trying an identified HTTP/1 payload as HTTP/2 is exactly how + # HTTP/1 traffic acquires a confident HTTP/2 mislabel -- the failure #787 + # exists to stop -- and #682 is about to route 231 real HTTP/1 frames + # through here. + # + # Nothing is suppressed on this arm, deliberately: it is not a candidate + # to be declined, so there is nothing to decline *to*, and + # ``test_guess_version_does_not_suppress_struct_error_on_the_http1_arm`` + # pins that a :exc:`struct.error` -- or pcapkit's own ``StructError``, + # whose ``eof`` flag ``NoPayload`` handling reads -- reaches the caller + # from this route rather than being converted or swallowed. + if _test_start_line(self._data[:length]): + return HTTPv1(self._data, length, **kwargs) + + # NOTE: Neither identification matched, so this is the fall-through: a + # trial parse, kept because a *mid-stream* payload carries no start line + # and no preface, and a self-consistent HTTP/2 frame is still the best + # answer available for one. It is the last resort rather than the whole + # method, which is the #800 change. + # + # The two arms suppress different sets, and the asymmetry is # deliberate. Only the *last* arm additionally suppresses # :exc:`struct.error`, because a payload too short to hold HTTP/2's # nine-octet frame header, or one whose frame-specific fields exceed @@ -267,7 +409,6 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': # it into ``Raw``; it would also erase ``StructError.eof``, which # ``NoPayload`` handling reads. ``unknown HTTP version`` is only the best # case, needing arm 2 to decline as well. - from pcapkit.protocols.application.httpv1 import HTTP as HTTPv1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel with contextlib.suppress(ProtocolError): return HTTPv1(self._data, length, **kwargs) diff --git a/pcapkit/protocols/application/httpv1.py b/pcapkit/protocols/application/httpv1.py index ef396b8f5d..9472776c14 100644 --- a/pcapkit/protocols/application/httpv1.py +++ b/pcapkit/protocols/application/httpv1.py @@ -72,6 +72,71 @@ _RE_STATUS = re.compile(rb'\d{3}\Z') +def _test_start_line(data: 'bytes') -> 'bool': + """Whether ``data`` opens with an HTTP/1.* start line. + + This is a *classification* predicate and parses nothing: it answers "is this + HTTP/1?" for :meth:`HTTP._guess_version + `, which until #800 + answered that question by trial-parsing every version in the family and + keeping whichever one did not object -- so a payload that is not HTTP at all + was classified by which parser happened to fail less loudly. + + Args: + data: Payload to classify. + + Returns: + Whether the payload's first line is a ``request-line`` or a + ``status-line`` (:rfc:`9112#section-2.1`). + + Note: + The acceptance rule is deliberately the *same* one + :meth:`HTTP._read_http_header + ` applies + further down this module -- ``_RE_METHOD`` with ``_RE_VERSION`` for a + request, ``_RE_VERSION`` with ``_RE_STATUS`` for a response -- which is + why this lives beside those three patterns rather than in the dispatcher + that calls it. The two must accept the same start lines: a predicate + looser than the parser classifies payloads the parser then refuses, and + one tighter than the parser hands real HTTP/1 to a later arm. + ``test_start_line_predicate_agrees_with_the_httpv1_parser`` pins that + agreement. + + Both of the unpackings the parser performs *before* those patterns are + mirrored too, and this is not pedantry -- the second of them is the whole + reason the HTTP/2 connection preface is not claimed here. ``PRI * + HTTP/2.0\\r\\n\\r\\nSM\\r\\n\\r\\n`` is deliberately a well-formed + HTTP/1.1 *request line* (:rfc:`9113#section-3.4`), so a predicate that + tested only the first line would answer :data:`True` for it. Split at the + header/body separator first, as :meth:`HTTP.read + ` does, and the preface's + header is ``PRI * HTTP/2.0`` with no CRLF left in it -- which is exactly + why the parser refuses it, and now why this does. Measured: without the + separator split this returned :data:`True` for the preface. + + An HTTP/0.9 request line carries only two tokens and so is not recognised + here either, matching the parser, which raises on fewer than three. + + """ + header = data.split(b'\r\n\r\n', maxsplit=1)[0] + if header == data: # no header/body separator -- ``read`` raises + return False + + startline = header.split(b'\r\n', maxsplit=1)[0] + if startline == header: # header holds no CRLF -- ``_read_http_header`` raises + return False + + try: + para1, para2, para3 = re.split(rb'\s+', startline, maxsplit=2) + except ValueError: + return False + + return bool( + (re.match(_RE_METHOD, para1) and re.match(_RE_VERSION, para3)) # request-line + or (re.match(_RE_VERSION, para1) and re.match(_RE_STATUS, para2)) # status-line + ) + + class Type(StrEnum): """HTTP packet type.""" diff --git a/tests/protocols/application/test_http_unit.py b/tests/protocols/application/test_http_unit.py index 1af1164a3c..0b5c4a5a68 100644 --- a/tests/protocols/application/test_http_unit.py +++ b/tests/protocols/application/test_http_unit.py @@ -491,6 +491,13 @@ def test_guess_version_reaches_http2_on_the_connection_preface(self) -> None: is a real gap (tracked as #800), but reading it as if it were binary framing was never a fix for that gap, only an accident #799 closes. + #800 has since closed that gap, so the one-buffer construction is + asserted here on the *explicit* path only: ``read(version=2)`` still has + no notion of the preface and must still refuse to read it as framing, + while the guess path now identifies the preface by prefix compare and + parses the frame after it. That half moved to + ``test_guess_version_identifies_the_http2_connection_preface`` below. + """ import io import warnings @@ -517,14 +524,349 @@ def test_guess_version_reaches_http2_on_the_connection_preface(self) -> None: self.assertEqual(clean.version, '2') self.assertEqual([w for w in caught if issubclass(w.category, ProtocolWarning)], []) - # The preface prepended to the frame, read as one buffer, must now be - # refused on *both* paths -- see the docstring above for why that is - # the fix rather than a regression. + # The preface prepended to the frame, read as one buffer, is still + # refused on the *explicit* path: ``read(version=2)`` hands the whole + # buffer to ``httpv2.HTTP``, which has no notion of the preface and must + # not read its ASCII as framing. The guess path identifies it instead -- + # see ``test_guess_version_identifies_the_http2_connection_preface``. raw = preface + settings - for label, kwargs in (('guessed', {}), ('explicit', {'version': 2})): - with self.subTest(path=label): + with self.assertRaises(ProtocolError): + HTTP(io.BytesIO(raw), len(raw), version=2) + + def test_guess_version_identifies_the_http2_connection_preface(self) -> None: + """The preface is identified by prefix compare, not by a parse (#800). + + ``b'PRI * HTTP/2.0\\r\\n\\r\\nSM\\r\\n\\r\\n'`` followed by a SETTINGS + frame is what every HTTP/2 connection opens with + (:rfc:`9113#section-3.4`), and before this change the dispatcher had no + notion of it: the HTTP/1 arm declined it and the HTTP/2 arm read the + preface's own ASCII as a frame header, ``b'PRI'`` becoming a declared + length of 5,265,993. #799/#802 then made that inconsistency a refusal, + which was right in itself but left a real HTTP/2 connection opening + reported as not-HTTP at all -- measured on ``f046b38f8``/``4530424df``, + both the bare preface and preface-plus-SETTINGS raised ``unknown HTTP + version``. + + Three things are asserted, because "it answers 2" alone would also be + true of the accident this replaces: + + * the answer is ``version='2'``; + * the parsed frame is *the SETTINGS frame*, byte-identical to reading + that frame on its own -- which is what proves the preface was skipped + rather than consumed as framing; + * the reported declared length is the frame's real 9, not a number the + preface's ASCII happens to spell. + + """ + import io + import warnings + + from pcapkit.protocols.application.http import HTTP + from pcapkit.utilities.warnings import ProtocolWarning + + preface = b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n' + settings = http2_frame_bytes(0x04, 0x00, 0, b'') + raw = preface + settings + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + guessed = HTTP(io.BytesIO(raw), len(raw)) + frame_only = HTTP(io.BytesIO(settings), len(settings), version=2) + + self.assertEqual(guessed.version, '2') + self.assertEqual(guessed.alias, 'HTTP/2') + # The preface is header rather than payload, so it counts towards + # ``length`` -- without that, ``__init__``'s ``packet=self.packet.payload`` + # injection slices from octet 9 of a buffer whose frame starts at 24 and + # reports the tail of the preface as this packet's payload, which is what + # ``info`` equality below would otherwise catch. + self.assertEqual(guessed.length, len(preface) + frame_only.length) + self.assertEqual(guessed.info, frame_only.info) + self.assertEqual(guessed.info.packet, b'') + self.assertEqual(guessed.info.length, 9) + self.assertNotEqual(guessed.info.length, 5265993) + self.assertEqual([w for w in caught if issubclass(w.category, ProtocolWarning)], []) + + # The frame after the preface is parsed for real, not assumed: a padded + # SETTINGS payload comes back as its own settings, and a frame the + # parser refuses is refused rather than answered by the identification. + settings_2 = http2_frame_bytes(0x04, 0x00, 0, b'\x00\x03\x00\x00\x00d') + guessed_2 = HTTP(io.BytesIO(preface + settings_2), len(preface) + len(settings_2)) + self.assertEqual(guessed_2.version, '2') + self.assertEqual(guessed_2.info, HTTP(io.BytesIO(settings_2), len(settings_2), + version=2).info) + + def test_guess_version_reports_a_preface_with_no_frame_as_such(self) -> None: + """A preface with nothing after it is HTTP/2, but carries no frame. + + :rfc:`9113#section-3.4` requires the preface to be followed immediately + by a SETTINGS frame, so a payload that is *exactly* the 24 preface octets + is either truncated or cut at a segment boundary. It is still positively + identified as HTTP/2 -- but this library's HTTP/2 data model is one frame + per packet and has no representation for a frameless segment, so there is + nothing to return and the payload is refused. + + What the fix buys here is the *diagnosis*, which is why the message is + asserted rather than merely the exception type: on ``4530424df`` this + answered ``unknown HTTP version``, indistinguishable from genuine + garbage, and it now says which of the two it was. Reporting a version + with no data would be the dishonest alternative. + + """ + import io + + from pcapkit.protocols.application.http import HTTP + from pcapkit.utilities.exceptions import ProtocolError + + preface = b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n' + self.assertEqual(len(preface), 24) + + with self.assertRaises(ProtocolError) as ctx: + HTTP(io.BytesIO(preface), len(preface)) + self.assertEqual(str(ctx.exception), 'HTTP/2: connection preface with no frame') + + # A preface followed by something too short to be a frame is refused by + # the HTTP/2 parser itself -- identification commits to the version, it + # does not excuse the frame. + raw = preface + b'\x00\x00\x09\x04' + with self.assertRaises(ProtocolError) as ctx: + HTTP(io.BytesIO(raw), len(raw)) + self.assertIn('9-octet frame header', str(ctx.exception)) + + # And a preface followed by a frame that trips the #805 residual -- a + # sixteen-octet GOAWAY, whose fixed ``stream`` and ``error`` fields alone + # want eight octets after the header -- must come back catchable. The + # identified HTTP/2 route normalises that, because the trial-parse arm it + # replaces for this input used to suppress :exc:`struct.error` and answer + # ``unknown HTTP version``; without the conversion the bare stdlib error + # would now escape a dispatcher documented to raise ``ProtocolError``. + import struct + + from pcapkit.utilities.exceptions import BaseError + + goaway = b'\x00\x00\x15\x07\x00\x00\x00\x00\x00' + b'\xff' * 7 + self.assertEqual(len(goaway), 16) + raw = preface + goaway + with self.assertRaises(ProtocolError) as ctx: + HTTP(io.BytesIO(raw), len(raw)) + self.assertEqual(str(ctx.exception), 'HTTP/2: invalid format') + self.assertIsInstance(ctx.exception, BaseError) + self.assertNotIsInstance(ctx.exception, struct.error) + self.assertIsInstance(ctx.exception.__cause__, struct.error) + + def test_guess_version_does_not_classify_text_as_http2(self) -> None: + """Garbage text must never come back HTTP/2. + + ``b'foo bar baz\\r\\nX: y\\r\\n\\r\\n'`` answered ``version='2'`` on + ``main`` before #802 -- the headline wrong answer of #800 -- because the + HTTP/2 arm accepted any buffer of nine octets or more and ``b'foo'`` read + as a declared length of 6,712,175 that nothing checked. + + Honest about what closed it: #802's ``schema.length > length`` check + already refuses this, so this case passes on ``4530424df`` too and is a + *regression guard* rather than a fix demonstration. It is worth pinning + here all the same, because after #800 the answer no longer depends on + that guard at all: text does not match the preface and does not match an + HTTP/1 start line, so it is never positively identified as anything, and + the fall-through is the only route left to it. Both defences are asserted + so that loosening either one is noticed. + + """ + import io + + from pcapkit.protocols.application.http import HTTP + from pcapkit.protocols.application.httpv1 import _test_start_line + from pcapkit.utilities.exceptions import ProtocolError + + preface = b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n' + cases = ( + ('lowercase start line', b'foo bar baz\r\nX: y\r\n\r\n'), + ('no start line at all', b'not http at all'), + ('prose', b'the quick brown fox jumps over the lazy dog\r\n\r\n'), + ('almost a method', b'Get /index.html HTTP/1.1\r\nHost: example.com\r\n\r\n'), + ) + + for label, raw in cases: + with self.subTest(case=label): + # Neither identification claims it ... + self.assertFalse(raw.startswith(preface)) + self.assertFalse(_test_start_line(raw)) + # ... and the fall-through refuses it rather than guessing. with self.assertRaises(ProtocolError): - HTTP(io.BytesIO(raw), len(raw), **kwargs) + HTTP(io.BytesIO(raw), len(raw)) + + def test_guess_version_commits_to_http1_once_the_start_line_says_so(self) -> None: + """An identified HTTP/1 message is not re-tried as HTTP/2 (#800). + + This is the direction that matters for #682, which is about to route 231 + real HTTP/1 frames through this dispatcher. Before the fix, an HTTP/1 + message the HTTP/1 parser refused was handed to the HTTP/2 arm, which + accepts any self-consistent buffer of nine octets or more -- so the + answer for a malformed HTTP/1 message depended on what its first three + ASCII octets happened to spell as a 24-bit length. Now the start line + decides the version and the HTTP/1 parser's own verdict is the answer. + + Asserted on the message, because the exception *type* is + ``ProtocolError`` either way: on ``4530424df`` these payloads came back + ``unknown HTTP version`` (both arms having declined), and they now come + back ``HTTP: invalid format`` from the version that was actually + identified. + + """ + import io + + from pcapkit.protocols.application.http import HTTP + from pcapkit.protocols.application.httpv1 import _test_start_line + from pcapkit.utilities.exceptions import ProtocolError + + cases = ( + ('field line with no colon', b'GET / HTTP/1.1\r\nbadfield\r\n\r\n'), + ('first field line folded', b'GET / HTTP/1.1\r\n Host: example.com\r\n\r\n'), + ('status line, no colon', b'HTTP/1.1 200 OK\r\nbadfield\r\n\r\n'), + ) + + for label, raw in cases: + with self.subTest(case=label): + self.assertTrue(_test_start_line(raw)) + with self.assertRaises(ProtocolError) as ctx: + HTTP(io.BytesIO(raw), len(raw)) + self.assertEqual(str(ctx.exception), 'HTTP: invalid format') + + def test_start_line_predicate_agrees_with_the_httpv1_parser(self) -> None: + """``_test_start_line`` must accept exactly what ``httpv1.HTTP`` accepts. + + The predicate classifies and the parser parses, and they are two + statements of the same rule -- which is why the predicate lives beside + ``_RE_METHOD``/``_RE_VERSION``/``_RE_STATUS`` in ``httpv1.py`` rather + than in the dispatcher. Drift either way is a defect: a predicate looser + than the parser classifies payloads the parser then refuses, and a + tighter one hands real HTTP/1 to a later arm, which is the mislabel #787 + and #800 are both about. + + Pinned by construction rather than by inspection -- each payload is run + through ``httpv1.HTTP`` as well as through the predicate, so the two + cannot be edited apart without this failing. + + """ + import io + + from pcapkit.protocols.application.httpv1 import HTTP as HTTPv1 + from pcapkit.protocols.application.httpv1 import _test_start_line + from pcapkit.utilities.exceptions import ProtocolError + + accepted = ( + ('request', b'GET /index.html HTTP/1.1\r\nHost: example.com\r\n\r\n'), + ('request, extra spaces', b'GET / HTTP/1.0\r\nHost: example.com\r\n\r\n'), + ('response', b'HTTP/1.1 200 OK\r\nServer: example\r\n\r\nbody'), + ('response, no message', b'HTTP/1.0 404 -\r\nServer: example\r\n\r\n'), + ('hyphenated method', b'M-SEARCH * HTTP/1.1\r\nHost: example.com\r\n\r\n'), + ) + refused = ( + # Rejected by the anchored patterns themselves. + ('lowercase method', b'Get / HTTP/1.1\r\nHost: example.com\r\n\r\n'), + ('four-digit status', b'HTTP/1.1 2000 OK\r\nServer: example\r\n\r\n'), + ('no version token', b'GET / FTP/1.1\r\nHost: example.com\r\n\r\n'), + # Rejected by the unpackings ``_read_http_header`` performs first. + ('two-token start line', b'GET /\r\nHost: example.com\r\n\r\n'), + ('no CRLF at all', b'GET / HTTP/1.1'), + ('the HTTP/2 preface', b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'), + ('not http at all', b'not http at all'), + ) + + for label, raw in accepted: + with self.subTest(accepted=label): + self.assertTrue(_test_start_line(raw)) + # The parser agrees: it reads this without raising. + self.assertIn(HTTPv1(io.BytesIO(raw), len(raw)).version, ('1.0', '1.1')) + + for label, raw in refused: + with self.subTest(refused=label): + self.assertFalse(_test_start_line(raw)) + # The parser agrees: it refuses this at the start line. + with self.assertRaises(ProtocolError): + HTTPv1(io.BytesIO(raw), len(raw)) + + def test_guess_version_leaves_a_mid_stream_frame_to_the_fall_through(self) -> None: + """A bare frame carries no preface and no start line, and is undecidable. + + Identification cannot answer for a mid-stream segment, and #800 + deliberately adds no heuristic for one -- no "frame type at most 9, + reserved bit clear" test, because that is what misfires on binary HTTP/1 + bodies. So a bare frame falls through to the trial parse, where the + HTTP/2 parser's own length and type consistency rules decide: a + self-consistent frame is read as HTTP/2 (unchanged behaviour, pinned so + the new identification step is not mistaken for a replacement of the + fall-through), and one whose declared length its buffer does not back is + refused rather than answered. + + """ + import io + + from pcapkit.protocols.application.http import HTTP + from pcapkit.protocols.application.httpv1 import _test_start_line + from pcapkit.utilities.exceptions import ProtocolError + + preface = b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n' + settings = http2_frame_bytes(0x04, 0x00, 0, b'') + + # Neither identification fires, so this reaches the fall-through. + self.assertFalse(settings.startswith(preface)) + self.assertFalse(_test_start_line(settings)) + + guessed = HTTP(io.BytesIO(settings), len(settings)) + self.assertEqual(guessed.version, '2') + self.assertEqual(guessed.info, + HTTP(io.BytesIO(settings), len(settings), version=2).info) + + # A nine-octet header declaring 16777215 is not backed by its buffer. + inconsistent = b'\xff\xff\xff\x04\x00\x00\x00\x00\x00' + with self.assertRaises(ProtocolError): + HTTP(io.BytesIO(inconsistent), len(inconsistent)) + + def test_guess_version_keeps_an_upgrade_h2c_exchange_on_http1(self) -> None: + """An ``Upgrade: h2c`` exchange stays HTTP/1.1, and that is correct. + + :rfc:`7540#section-3.2`'s upgrade -- deprecated by + :rfc:`9113#section-3.1` but not removed -- is explicitly out of #800's + scope, and not because it was awkward: on the wire the upgrade request + and its ``101 Switching Protocols`` response *are* HTTP/1.1 messages, and + HTTP/1.1 is the right answer for both. The switch takes effect only after + the ``101``, so classifying later segments of the same connection as + HTTP/2 needs per-connection state keyed on the 4-tuple, and + ``_guess_version`` is handed one payload with no flow context. + Recognising the field is possible; acting on it is not, so it is left + alone rather than half-implemented. + + Pinned so that a later attempt to "support h2c" by sniffing the header + has to change a test that says why it must not. + + """ + import io + + from pcapkit.protocols.application.http import HTTP + + request = (b'GET / HTTP/1.1\r\n' + b'Host: example.com\r\n' + b'Connection: Upgrade, HTTP2-Settings\r\n' + b'Upgrade: h2c\r\n' + b'HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA\r\n\r\n') + response = (b'HTTP/1.1 101 Switching Protocols\r\n' + b'Connection: Upgrade\r\n' + b'Upgrade: h2c\r\n\r\n') + + for label, raw in (('upgrade request', request), ('101 response', response)): + with self.subTest(case=label): + guessed = HTTP(io.BytesIO(raw), len(raw)) + explicit = HTTP(io.BytesIO(raw), len(raw), version=1) + + self.assertEqual(guessed.version, '1.1') + self.assertEqual(guessed.alias, 'HTTP/1.1') + self.assertEqual(guessed.info, explicit.info) + + # The field is visible in the parsed message -- it is simply not acted + # on, which is the distinction this test exists to record. + upgraded = HTTP(io.BytesIO(request), len(request)) + self.assertEqual(upgraded.info.header['Upgrade'], 'h2c') def test_guess_version_still_prefers_http1_for_http1_bytes(self) -> None: """HTTP/1 is tried first and must still win, request and response alike.