Repository navigation
HTTP.make calls the versioned make unbound on the class, so it raises TypeError for every real call #452
Description
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 classprotocol 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().
Activity
JarryShaw commentedon Sep 18, 2026
Correcting my own reproduction in this issue — the defect is real, but the snippet I gave does not demonstrate it.
I wrote:
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 innerprotocol.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:284isreached through the
def __init__(self, **kwargs)overload at:517, so the supported way to build a packet isHTTP(version=…, …)— and that call reaches the inner unbound dispatch. Corrected reproduction, onda2422728:Same exception, but now arrived at legitimately:
__init__→ boundself.make(...)→protocol.make(**kwargs)on the class, whereprotocolishttpv1.HTTPand itsmakeisdef make(self, http_version='1.1', …)athttpv1.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 plainfunctionis still the right evidence that it is neither astaticmethodnor aclassmethod; both versions still fail; and the note about the test doubles remains the important one — they declaremakeas@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 realHTTPv1/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 insideHTTPv1.makefrom 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 aTypeError.Apologies for the noise; the diagnosis was right and the demonstration was not.