diff --git a/docs/source/pep.rst b/docs/source/pep.rst index b6d3455f45..118caf1bd9 100644 --- a/docs/source/pep.rst +++ b/docs/source/pep.rst @@ -357,17 +357,21 @@ Three follow-ups the above deliberately left alone: public import paths, so the misclassification is documented rather than fixed. It is inert for layer-limited extraction, since IPv4 and IPv6 terminate an ``internet`` extraction before either is reached. -* :attr:`UDP.__proto__ ` points - its HTTP ports at the version-guessing - :class:`pcapkit.protocols.application.http.HTTP`, while - :attr:`TCP.__proto__ ` points - the same ports at :class:`pcapkit.protocols.application.httpv1.HTTP`. The - asymmetry predates the 8080 entries. Reconciling it changes what existing - captures parse to, so it wants its own change and its own decision, which is - why it is asked for here. The separate defect that ``http.HTTP``'s explicit - ``version=`` path was unusable -- - `#447 `__ -- has since been - fixed, and is no longer part of this request. +* **Done.** :attr:`UDP.__proto__ + ` pointed its HTTP ports at the + version-identifying :class:`pcapkit.protocols.application.http.HTTP` while + :attr:`TCP.__proto__ ` pointed + the same ports at :class:`pcapkit.protocols.application.httpv1.HTTP`, an + asymmetry that predated the 8080 entries. Both now bind the proxy, so a TCP + segment's HTTP version is decided by its payload rather than asserted by its + port number. It waited on + `#800 `__, which replaced + the proxy's trial-and-error guess with a positive identification: repointing + ahead of that would have routed 231 real HTTP/1.1 fixture frames through a + guess path that was known-wrong on non-HTTP input. The separate defect that + ``http.HTTP``'s explicit ``version=`` path was unusable -- + `#447 `__ -- had already + been fixed and was never part of this. Beyond those, the gaps most likely to be met in a real capture are ICMP (1), ICMPv6 (58) and IGMP (2) on the internet layer, all three of which have stubs; diff --git a/examples/generators/dispatch.py b/examples/generators/dispatch.py index f1b60666e2..9044e22be3 100644 --- a/examples/generators/dispatch.py +++ b/examples/generators/dispatch.py @@ -625,8 +625,10 @@ def _internet_enum() -> 'Any': # -- TCP.__proto__ (port) -------------------------------------------------- 'tcp/20': ('pcapkit.protocols.application.ftp', 'FTP_DATA'), 'tcp/21': ('pcapkit.protocols.application.ftp', 'FTP'), - 'tcp/80': ('pcapkit.protocols.application.httpv1', 'HTTP'), - 'tcp/8080': ('pcapkit.protocols.application.httpv1', 'HTTP'), + # Repointed from ``httpv1`` to the version-identifying proxy by #682, which + # is what makes these two agree with ``udp/80`` and ``udp/8080`` below. + 'tcp/80': ('pcapkit.protocols.application.http', 'HTTP'), + 'tcp/8080': ('pcapkit.protocols.application.http', 'HTTP'), # -- UDP.__proto__ (port) -------------------------------------------------- 'udp/80': ('pcapkit.protocols.application.http', 'HTTP'), diff --git a/pcapkit/protocols/application/http.py b/pcapkit/protocols/application/http.py index fc7f4aa710..bf23e68c37 100644 --- a/pcapkit/protocols/application/http.py +++ b/pcapkit/protocols/application/http.py @@ -337,8 +337,9 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': # 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. + # exists to stop -- and #682 now routes 231 real HTTP/1 frames from the + # fixture corpus through here, TCP:80/8080 having been repointed at this + # class. # # Nothing is suppressed on this arm, deliberately: it is not a candidate # to be declined, so there is nothing to decline *to*, and diff --git a/pcapkit/protocols/transport/tcp.py b/pcapkit/protocols/transport/tcp.py index fcc61de9eb..42f95f55d8 100644 --- a/pcapkit/protocols/transport/tcp.py +++ b/pcapkit/protocols/transport/tcp.py @@ -185,9 +185,9 @@ class TCP(Transport[Data_TCP, Schema_TCP], * - 21 - :class:`pcapkit.protocols.application.ftp.FTP` * - 80 - - :class:`pcapkit.protocols.application.httpv1.HTTP` + - :class:`pcapkit.protocols.application.http.HTTP` * - 8080 - - :class:`pcapkit.protocols.application.httpv1.HTTP` + - :class:`pcapkit.protocols.application.http.HTTP` This class currently supports parsing of the following TCP options, which are directly mapped to the :class:`pcapkit.const.tcp.option.Option` @@ -325,10 +325,21 @@ class TCP(Transport[Data_TCP, Schema_TCP], # 8443 is deliberately absent: IANA registers it as ``pcsync-https`` # rather than as an HTTP alternate, and traffic there is TLS-wrapped, # which pcapkit does not parse. + # + # Both HTTP ports bind the version-dispatching + # :class:`pcapkit.protocols.application.http.HTTP` rather than + # ``httpv1.HTTP``, because HTTP/1 and HTTP/2 share these ports on the + # wire -- RFC 9113 calls the cleartext form ``http`` and keeps it on + # 80 -- so the port cannot decide the version and the payload has to. + # That dispatch is a positive identification (the RFC 9113 ยง3.4 + # connection preface, then an HTTP/1 start line) rather than a trial + # parse; see #682 for the repoint and #800 for the identification it + # waited on. UDP's table already bound the proxy for the same ports, + # so this is also what removes the asymmetry between the two. 20: ModuleDescriptor('pcapkit.protocols.application.ftp', 'FTP_DATA'), 21: ModuleDescriptor('pcapkit.protocols.application.ftp', 'FTP'), - 80: ModuleDescriptor('pcapkit.protocols.application.httpv1', 'HTTP'), - 8080: ModuleDescriptor('pcapkit.protocols.application.httpv1', 'HTTP'), + 80: ModuleDescriptor('pcapkit.protocols.application.http', 'HTTP'), + 8080: ModuleDescriptor('pcapkit.protocols.application.http', 'HTTP'), }, ) diff --git a/pcapkit/protocols/transport/udp.py b/pcapkit/protocols/transport/udp.py index 772876bb7b..8c13a96f14 100644 --- a/pcapkit/protocols/transport/udp.py +++ b/pcapkit/protocols/transport/udp.py @@ -66,14 +66,13 @@ class UDP(Transport[Data_UDP, Schema_UDP], Note: Both HTTP ports here resolve to - :class:`pcapkit.protocols.application.http.HTTP`, which sniffs HTTP/1 - against HTTP/2 and delegates, whereas + :class:`pcapkit.protocols.application.http.HTTP`, which identifies the + version from the payload and delegates. :attr:`TCP.__proto__ ` - binds :class:`pcapkit.protocols.application.httpv1.HTTP` directly for - the same ports. The asymmetry predates the 8080 entries -- port 80 was - already split this way -- and each table is left internally consistent - rather than repointing port 80 and changing what existing captures - parse to. Reconciling the two is left as its own change. + bound :class:`pcapkit.protocols.application.httpv1.HTTP` directly for the + same ports until #682, which repointed it here and so removed an + asymmetry that had predated the 8080 entries -- port 80 was already split + that way. Both tables now agree. """ @@ -95,10 +94,11 @@ class UDP(Transport[Data_UDP, Schema_UDP], # 1701 l2tp l2tp # 8080 http-alt HTTP Alternate (see port 80) # - # Both HTTP entries keep pointing at the version-guessing + # Both HTTP entries keep pointing at the version-dispatching # :class:`pcapkit.protocols.application.http.HTTP`, which is what - # port 80 already used here -- unlike TCP, which binds HTTP/1 - # directly. c.f. the note in the class docstring. + # port 80 already used here. TCP bound HTTP/1 directly for the same + # ports until #682 repointed it at the proxy too, so the two tables + # no longer disagree. c.f. the note in the class docstring. 80: ModuleDescriptor('pcapkit.protocols.application.http', 'HTTP'), 8080: ModuleDescriptor('pcapkit.protocols.application.http', 'HTTP'), diff --git a/tests/protocols/transport/test_tcp_http_dispatch_unit.py b/tests/protocols/transport/test_tcp_http_dispatch_unit.py new file mode 100644 index 0000000000..8fbdbc6438 --- /dev/null +++ b/tests/protocols/transport/test_tcp_http_dispatch_unit.py @@ -0,0 +1,124 @@ +# -*- coding: utf-8 -*- +"""TCP's HTTP ports dispatch through the version-identifying proxy. C.f. #682. + +:attr:`TCP.__proto__ ` bound +:class:`pcapkit.protocols.application.httpv1.HTTP` directly for ports 80 and +8080, so a segment on either port was HTTP/1 by assertion of the port number: +an HTTP/2 payload there was refused by the HTTP/1 parser and reported as +:class:`~pcapkit.protocols.misc.raw.Raw`. Both versions share those ports on the +wire -- :rfc:`9113#section-3.1` keeps the cleartext form on ``http`` -- so the +port cannot decide the version and the payload has to, which is what +:class:`pcapkit.protocols.application.http.HTTP` does. + +Nothing is parsed from :file:`examples/captures/`, so this is unit tier. The +segments below are built in memory, and the HTTP/2 payload is the *verbatim* +nine octets that :file:`examples/captures/options-transport.pcap` frame 34 +carries -- a fixture this library's own ``httpv2.HTTP.make`` produced -- rather +than a hand-rolled frame, because HTTP/2's declared length is whole-frame here +and a synthetic frame written to the payload-only convention would fail for a +reason that has nothing to do with dispatch. + +""" + +from __future__ import annotations + +import importlib.util +import unittest +import warnings + +from tests._support import purge_modules, time_limit + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: A 20-octet TCP header, data offset 5, ACK set, from port 50000 to port 80. +_TCP_TO_80 = bytes.fromhex('c350005000000001000000015010ffff00000000') +#: The same, to port 8080. +_TCP_TO_8080 = bytes.fromhex('c3501f9000000001000000015010ffff00000000') + +#: A nine-octet HTTP/2 ``DATA`` frame: declared length 9, type 0, no flags, +#: stream 0. Copied verbatim from ``options-transport.pcap`` frame 34. +_HTTP2_FRAME = bytes.fromhex('000009000000000000') +#: A minimal HTTP/1.1 request, which must keep decoding as HTTP/1.1. +_HTTP1_REQUEST = b'GET / HTTP/1.1\r\nHost: example.invalid\r\n\r\n' + + +class TCPHTTPDispatchTests(unittest.TestCase): + """TCP's port-80/8080 entries point at the proxy, and behave like it.""" + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def test_tcp_binds_the_http_proxy_on_both_http_ports(self) -> None: + """Ports 80 and 8080 resolve to ``application.http``, not ``application.httpv1``. + + The descriptor is checked rather than the imported class because all three + HTTP classes are named ``HTTP`` (c.f. #682's own subject, the registry key + collision), so the module name is the only thing that tells them apart. + + """ + from pcapkit.protocols.transport.tcp import TCP + + for port in (80, 8080): + with self.subTest(port=port): + descriptor = TCP.__proto__[port] + self.assertEqual(descriptor.module, 'pcapkit.protocols.application.http') + self.assertEqual(descriptor.name, 'HTTP') + + def test_tcp_and_udp_agree_on_the_http_ports(self) -> None: + """The TCP/UDP asymmetry #682 closed stays closed. + + UDP already bound the proxy for both ports; TCP bound ``httpv1`` for the + same two. Pinning the equality rather than each table separately is what + makes a future one-sided edit fail here. + + """ + from pcapkit.protocols.transport.tcp import TCP + from pcapkit.protocols.transport.udp import UDP + + for port in (80, 8080): + with self.subTest(port=port): + tcp, udp = TCP.__proto__[port], UDP.__proto__[port] + self.assertEqual((tcp.module, tcp.name), (udp.module, udp.name)) + + @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') + def test_http2_on_the_http_ports_decodes_as_http2_and_http1_still_as_http1(self) -> None: + """Both versions reach their own parser from a port-80/8080 segment. + + The HTTP/2 half is what the repoint buys: before it, these nine octets + decoded as ``TCP:Raw``, because ``httpv1.HTTP`` refused them and + :func:`~pcapkit.protocols.misc.raw.beholder` turned the refusal into + ``Raw``. The HTTP/1.1 half is the guard on the 231 HTTP/1.1 frames in the + fixture corpus that this change routes through + :meth:`HTTP._guess_version + ` for the first + time: they have to come back HTTP/1.1, and both halves are asserted here + so neither can be satisfied at the other's expense. + + """ + from pcapkit.protocols.transport.tcp import TCP + + cases = ( + (_TCP_TO_80, _HTTP2_FRAME, 'TCP:HTTP/2'), + (_TCP_TO_8080, _HTTP2_FRAME, 'TCP:HTTP/2'), + (_TCP_TO_80, _HTTP1_REQUEST, 'TCP:HTTP/1.1'), + (_TCP_TO_8080, _HTTP1_REQUEST, 'TCP:HTTP/1.1'), + ) + for header, payload, expected in cases: + with self.subTest(port=int.from_bytes(header[2:4], 'big'), chain=expected): + raw = header + payload + with warnings.catch_warnings(): + # An in-memory segment carries no checksum, which the parser + # is entitled to complain about; the chain is what is under + # test. + warnings.simplefilter('ignore') + with time_limit(5): + proto = TCP(raw, len(raw)) + + self.assertEqual(str(proto.protochain), expected) + self.assertEqual(type(proto.payload).__module__, + 'pcapkit.protocols.application.http') + + +if __name__ == '__main__': + unittest.main()