fix(tcp): repoint TCP:80/8080 at the HTTP version proxy (#682)

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__ <pcapkit.protocols.transport.udp.UDP.__proto__>` points
-  its HTTP ports at the version-guessing
-  :class:`pcapkit.protocols.application.http.HTTP`, while
-  :attr:`TCP.__proto__ <pcapkit.protocols.transport.tcp.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 <https://github.com/JarryShaw/PyPCAPKit/issues/447>`__ -- has since been
-  fixed, and is no longer part of this request.
+* **Done.** :attr:`UDP.__proto__
+  <pcapkit.protocols.transport.udp.UDP.__proto__>` pointed its HTTP ports at the
+  version-identifying :class:`pcapkit.protocols.application.http.HTTP` while
+  :attr:`TCP.__proto__ <pcapkit.protocols.transport.tcp.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 <https://github.com/JarryShaw/PyPCAPKit/issues/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 <https://github.com/JarryShaw/PyPCAPKit/issues/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;

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'),

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

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'),
         },
     )
 

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__ <pcapkit.protocols.transport.tcp.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'),
 

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__ <pcapkit.protocols.transport.tcp.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
+        <pcapkit.protocols.application.http.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()