HTTP.make calls the versioned make unbound on the class, so it raises TypeError for every real call

[JarryShaw]

HTTP.make dispatches to the versioned implementation by calling make on the class rather than on an instance, so it raises TypeError for every real call. The existing tests do not catch it because their fakes declare make as a staticmethod.

Reproduction, on e2d8ed6d1

from pcapkit.protocols.application.http import HTTP

HTTP.make(version=1, receipt='request', method='GET', uri='/', version_1='1.1')
HTTP.make(version=2, ...)

HTTP.make(version=1) -> TypeError: HTTP.make() missing 1 required positional argument: 'self'
HTTP.make(version=2) -> TypeError: HTTP.make() missing 1 required positional argument: 'self'

Both versions, so neither path works.

Mechanism

pcapkit/protocols/application/http.py, at the end of make:

if version == 1:
    from pcapkit.protocols.application.httpv1 import HTTP as protocol
elif version == 2:
    from pcapkit.protocols.application.httpv2 import HTTP as protocol
else:
    raise ProtocolError(f"invalid HTTP version: {version}")
return protocol.make(**kwargs)          # <- unbound: `protocol` is the class

protocol is the imported class, and make is an ordinary instance method — pcapkit/protocols/application/httpv1.py:147 is def make(self, ...), and the abstract declaration at pcapkit/protocols/protocol.py:258 is def make(self, **kwargs) too. inspect.getattr_static(HTTPv1, 'make') returns a plain function, confirming it is neither a staticmethod nor a classmethod. So the call passes the first keyword argument into no self, and Python reports the missing positional.

Why the suite is green

The HTTP tests substitute fakes whose make is declared @staticmethod, which is callable on the class and so absorbs the unbound call. That makes the test double behave differently from the real class in exactly the way that matters here — worth noting for whoever fixes it, because a test written against the fake will keep passing whether or not the fix is right.

Fix

Either construct an instance and call make on it, the way read's sibling path constructs protocol(self._data, length, **kwargs), or make the versioned make a classmethod/staticmethod if a bare constructor is genuinely not wanted. The first matches how the rest of this class dispatches; the second changes a signature shared with ProtocolBase.make, so it is the more invasive option.

Whatever the fix, the regression test should exercise the real HTTPv1/HTTPv2 classes rather than a staticmethod fake, since the fake is what hid this.

Provenance

Found by the agent implementing #442 and #447, while checking — at my request — whether make shared the self._file/self._data mistake that #447 fixes in read. It does not: make never touches either, and this is an unrelated, pre-existing fault in the same method. Deliberately left out of PR #451, whose scope is read().

[JarryShaw]

Correcting my own reproduction in this issue — the defect is real, but the snippet I gave does not demonstrate it.

I wrote:

HTTP.make(version=1, receipt='request', method='GET', uri='/', version_1='1.1')
-> TypeError: HTTP.make() missing 1 required positional argument: 'self'

That fails at the outer HTTP.make, which is itself an ordinary instance method — so calling it on the class was always going to raise, and it says nothing about the inner protocol.make(**kwargs) this issue is actually about. Anyone checking the issue against the code would have found the snippet unconvincing, and rightly.

The real entry point is construction, not make. pcapkit/protocols/protocol.py:284 is

self.__header__ = self.make(**kwargs)

reached through the def __init__(self, **kwargs) overload at :517, so the supported way to build a packet is HTTP(version=…, …) — and that call reaches the inner unbound dispatch. Corrected reproduction, on da2422728:

HTTP(version=1, http_version='1.1', method='GET', uri='/')
-> TypeError: HTTP.make() missing 1 required positional argument: 'self'

Same exception, but now arrived at legitimately: __init__ → bound self.make(...) → protocol.make(**kwargs) on the class, where protocol is httpv1.HTTP and its make is def make(self, http_version='1.1', …) at httpv1.py:147. That is the unbound call, and it is genuinely unreachable-past for every real construction.

Everything else in this issue stands. inspect.getattr_static(HTTPv1, 'make') returning a plain function is still the right evidence that it is neither a staticmethod nor a classmethod; both versions still fail; and the note about the test doubles remains the important one — they declare make as @staticmethod, which absorbs an unbound call, so the fakes behave differently from the real classes in exactly the way that hid this. A regression test must use the real HTTPv1/HTTPv2.

One thing the corrected reproduction adds: with a candidate fix applied, the same call reaches ProtocolError: HTTP/1: invalid format — an in-library error raised inside HTTPv1.make from my deliberately minimal keyword arguments. That is the right shape for "the dispatch now works and the callee is validating", and it is a better fix-verification target than the absence of a TypeError.

Apologies for the noise; the diagnosis was right and the demonstration was not.