http: _guess_version trial-parses instead of identifying, so it answers HTTP/2 for garbage text and for the preface by accident

[JarryShaw]

Describe the bug

_guess_version decides the HTTP version by trial-parsing: try httpv1, and if that declines, try httpv2. That answers "did a parser accept this?" when the question is "what is this?" — and it gets both directions wrong. Measured on main:

real 24-octet HTTP/2 preface  -> answers version=2, but by misparsing b'PRI' as a frame
                                 header (Length 0x505249 = 5263433), with a ProtocolWarning
b'foo bar baz\r\nX: y\r\n\r\n'  -> answers version=2      <- garbage text classified as HTTP/2

It reaches the right answer on the preface for the wrong reason, and the wrong answer on text that is not HTTP at all. The maintainer's requirement, verbatim from #682:

We guess on the HTTP version and definitely have to make sure the guesing is correct and making sense.

Expected behavior

A positive identification before any parse attempt: compare the first 24 octets against b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'. RFC 9113 §3.4 designed that sequence for exactly this purpose — it is deliberately a well-formed HTTP/1.1 request line whose method PRI is reserved and unregistered, so an HTTP/1 parser rejects it and an HTTP/2 detector recognises it. A prefix compare cannot false-positive on valid HTTP/1 and needs no parse.

Then fall through to the existing HTTP/1 start-line regexes, which are correctly anchored (httpv1.py:60, :62, :72), and only then to a parse attempt.

What this does not fix, and should not pretend to

The Upgrade: h2c form (RFC 7540 §3.2, deprecated but not removed by RFC 9113 §3.1) is stateful and not expressible in this shape at all. On the wire the upgrade request is HTTP/1.1 and parses correctly today; the switch takes effect after the 101, so deciding that later segments on the same 4-tuple are HTTP/2 needs per-connection state. _guess_version receives one payload with no flow context. Recognising Upgrade: h2c in a request is possible; acting on it is not.

Likewise a mid-stream segment — a bare HTTP/2 frame header with no preface, or an opaque HTTP/1 body chunk — is genuinely undecidable from one payload. The honest outcome there is Raw, not a coin-flip. Any heuristic on the 9-byte frame header ("type ≤ 9, reserved bit clear") will misfire on binary HTTP/1 bodies, which is precisely how b'foo bar baz…' is classified HTTP/2 today.

Additional context

Sequencing — this is step 2 of 3, from the maintainer's ruling on #682 ("TCP:80 should use the proxy I think, since both HTTP/1 and HTTP/2 bind on them"):

1. httpv2: the frame guard tests the declared length, not the buffer, so a 4-octet frame can report length=16777215 #799 — httpv2's guard tests the declared length, not the buffer, so the sub-9 class behaves inconsistently. In flight.
2. This issue — positive preface identification, so the guessing is correct rather than merely reachable.
3. register_protocol's key space is still not unique: the three HTTP classes share one key (follow-up to #675) #682 — repoint TCP:80/8080 from httpv1 to the proxy.

The order matters and is not cosmetic. _guess_version is entered 0 times across all 1604 frames of the fixture corpus today, because every HTTP frame arrives over TCP and tcp.py:330 binds httpv1 directly. Repointing first would put 231 real HTTP/1 frames through a guess path that is known-wrong on non-HTTP input, trading an honest Raw for a confident wrong HTTP/2 on the main traffic path.

Also note the corpus cannot test this. No fixture uses UDP 80/8080 and none carries HTTP/2, so a preface test needs its own synthetic coverage — a real preface, a preface plus SETTINGS, garbage text, a mid-stream frame header, and an Upgrade: h2c exchange asserted to stay HTTP/1.1.

Related: #682, #799, #787, #789.

[JarryShaw]

Labelling blocked so it does not read as unheld work.

Checkable blocker: #799 merged. Both changes edit pcapkit/protocols/application/http.py's _guess_version, and #799 is in flight on it now — it narrows the arm-2 suppression back to ProtocolError once the sub-9 class is uniform. Adding a preface test on top of a _guess_version that is about to change means writing against a moving target, then re-deriving.

gh pr list -R JarryShaw/PyPCAPKit --search "799 in:body" --state all --json number,state,mergedAt

And the order is load-bearing rather than tidy. #799 makes the sub-9 class uniformly ProtocolError; only then does a preface test have a clean fall-through to sit in front of. Doing it the other way round means the preface test would be papering over an inconsistent arm 2 instead of complementing a clean one.

Unblocks the moment #799 lands, and #682's repoint is blocked on this one — that dependency is written on #682.

[JarryShaw]

Unblocked — #799 merged via #802 at 21:44:26Z as f046b38f8. Its httpv2 guard now tests the buffer as well as the declared length, so the sub-9 class is uniform and _guess_version has a clean fall-through to sit in front of.

Dispatching a worker. Two things #802 changed that this issue's body predates:

• Arm 2 is back to suppress(ProtocolError, struct.error) — fix(http): check the buffer's length, not just the declared one, in HTTP/2's frame guard #802 dropped the narrowing rather than widening the guard, because the root cause needs Schema.unpack and is tracked as corekit: a negative field length only warns in Schema.unpack, instead of raising ProtocolError #805. So do not assume the narrowed form when writing the preface test.
• read() now rejects schema.length > length, which is what made two pre-existing http: _guess_version's HTTP/2 arm is unreachable, and PayloadField.protocol does no case folding #787 tests fail — they had been passing only because b'not http at all' was read as declared length 7,237,492 and the preface's b'PRI' as 5,265,993. Both are now self-consistent fixtures. That second number is exactly why this issue exists: the preface "worked" by being misparsed as a frame header.

The rest of the plan stands as written — prefix-compare the first 24 octets against b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n' before any parse attempt, leave Upgrade: h2c explicitly out of scope, and answer Raw rather than guessing on a mid-stream segment. #682's repoint stays blocked on this.