Repository navigation
http: _guess_version's HTTP/2 arm is unreachable, and PayloadField.protocol does no case folding #787
Description
Activity
Correcting this issue's own reproduction. #789's worker challenged it and is right — and the truth is worse than what I wrote. Measured on
main:-- CONSTRUCTOR, which this issue named as the repro -- PayloadField(protocol='http'): _protocol='http' .protocol=http <- a bare str, not Raw PayloadField(protocol='HTTP'): _protocol='HTTP' .protocol=HTTP <- also broken PayloadField(protocol='TCP'): _protocol='TCP' .protocol=TCP -- SETTER -- f.protocol = 'http' -> _protocol=None .protocol=<class '...misc.raw.Raw'> f.protocol = 'HTTP' -> _protocol=<class '...application.http.HTTP'> f.protocol = 'tcp' -> _protocol=None .protocol=<class '...misc.raw.Raw'>So there are two defects on different paths and I conflated them:
- The constructor never resolves at all.
__init__assigned_protocoldirectly, bypassing the property, so.protocolreturns the bare string — for'HTTP'exactly as much as for'http'. Case was never this path's problem, and the value is notRaw, it is astrwhere a class is expected. - The setter case-folds wrongly, which is the
.upper()omission this issue described — real, but only reachable by assigning to the property.
My repro line paired path 1 with path 2's symptom. #789 fixes both by routing
__init__through the property; the registry import stays inside theisinstance(str)branch so schema class bodies still do not importpcapkit.protocolsmid-import.Worth recording that the two agents that measured this did not disagree — one measured the setter, one the constructor. The issue text was the thing that was wrong.
- The constructor never resolves at all.
- added 4 commits that reference this issue
on Sep 25, 2026 Fixed and merged — #789 landed at 18:34:14Z as
0928abb2c.Closing by hand: #789 carried no
Closeskeyword, so this stayed open despite being fixed. Same defect I caught on #784 earlier today and did not check for here.What landed, against this issue's two halves:
_guess_version's HTTP/2 arm is reachable.httpv1now raises a chainedProtocolErrorat the sites a non-HTTP/1 payload lands short on, rather than a bareValueErrorthatsuppress(ProtocolError)could not catch. A bounding test,test_httpv1_never_lets_a_bare_exception_escape, asserts nothing escapeshttpv1that is not aProtocolError— 13 subtest failures against the pre-fix library, so it bites. The cross-review then proved the raise-site enumeration complete two independent ways, including a 20,000-payload fuzz that found exactly three exception kinds, no fourth.PayloadFieldname resolution. Both halves fixed. Worth recording that this issue's own reproduction was wrong, corrected on the thread at 14:43Z:PayloadField(protocol='http')returned the bare string'http', notRaw, because__init__assigned_protocoldirectly and bypassed the property — and'HTTP'was equally broken, so case was never the constructor's problem. The.upper()omission was real but only reachable through the setter. Two defects on two paths, conflated in what I filed.
One consequence disclosed rather than hidden: four input classes that previously read
UDP:Rawnow readUDP:HTTP/2, listed per conversion site in the PR body rather than by example — the count had crept 1→3→4→5 across review rounds precisely because it was being enumerated by example.Two follow-ups carry the remainder, both tracked: #799 —
httpv2's frame guard tests the declared length rather than the buffer, so a 4-octet frame can reportlength=16777215; fixing it lets_guess_versiondrop thestruct.errorsuppression entirely and retire the residual this PR had to accept. And theHTTPv2-direct sub-9struct.error, which #799 covers as the same defect from a more consequential angle.- added 6 commits that reference this issue
on Sep 25, 2026
Found while investigating #682, and independent of how that issue is decided. Two separate defects in the same neighbourhood; both measured.
1.
_guess_version's HTTP/2 arm is unreachablepcapkit/protocols/application/http.py:202-207tries HTTP/1 then HTTP/2, each undercontextlib.suppress(ProtocolError):But
httpv1.HTTPraises a bareValueErroron HTTP/2 wire bytes, not aProtocolError.ValueErroris not suppressed, so it propagates out of the first arm and the HTTP/2 arm is never reached.Measured on
b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'plus a SETTINGS frame:httpv2.HTTPparses it and reportsversion='2', while the proxy raisesValueError: not enough values to unpack (expected 2, got 1).The asymmetry is visible two screens up: the explicit
version=path at:115-120does wrap it —— so
read(version=2)works andread()cannot. Combined with the port bindings (tcp.py:330-331binds 80/8080 to concrete HTTP/1, andhttpv2.HTTPis the value of no port on any transport), HTTP/2 is reachable only by explicitversion=2or direct instantiation, never automatically. That undercuts the proxy's purpose as a dispatcher.Either suppress
ValueErroralongsideProtocolErrorin_guess_version, or makehttpv1.HTTPraise aProtocolErrorfor a malformed request line the way the explicit path already normalises it to. The second is the better fix ifValueErrorfrom that constructor is never meaningful to a caller — worth checking before choosing.2.
PayloadField.protocoldoes no case foldingpcapkit/corekit/fields/misc.py:265-268:__proto__is keyed onprotocol.__name__.upper()(pcapkit/foundation/registry/protocols.py:219), but this reader does not.upper()its argument and uses.get, so a lowercase or mixed-case name silently resolves toNone.PayloadField(protocol='http')therefore yieldsRawrather than HTTP, with no warning — today, independent of any registry displacement.Every other reader compares against
id()as well and so tolerates the miss; this one does not. One line plus a test.Both are separable from #682's design question and from each other.