Skip to content

fix(http): HTTP.from_data raises 'invalid HTTP version: 0' - #1162

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1154-http-from-data-version
Oct 7, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1154-http-from-data-version

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — ran pylint (Makefile flags), mypy and isort on the two touched files directly, not via make; no new findings beyond the existing protected-access pattern
  • make test passes, and a test case covers the change — the modules listed below only, not the full suite
  • Added a changelog entry — N/A, added centrally after the wave

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1154.

  • http.HTTP._make_data looked up a version key that neither versioned data class has, so it always raised invalid HTTP version: 0. It now dispatches on the data class and returns the version that make needs.
  • httpv2.HTTP._make_data returned length, which ProtocolBase.__init__ takes as the parse length (payload only), so HTTP/2 from_data failed even directly (packet (6 octet(s)) shorter than the 9-octet frame header). Dropped; make computes the Length.
  • Two test_http_unit cases drove _make_data with a fake version key (and pinned length); they now use parsed data.

Probe, HTTP.from_data(frame['HTTP'].info) over the captures: before, 222/4/5 × invalid HTTP version: 0 (http/options-transport/test.pcap). After, all 231 rebuild byte-exact with equal info.

New test_http_from_data_1154_unit: 4 passed, 31 subtests. Without both fixes: 32 failed, 3 passed. Without only the httpv2 fix: 28 failed. Also passing: test_http_unit, test_http_guess_http2_1141_unit, test_http_runtime, four test_httpv2_*, test_construction_keyword_check_unit, test_option_roundtrip_unit, test_tcp_http_dispatch_unit, test_frame_from_data_runtime, tests/project (389 passed, 1 skipped).

- http: _make_data read a `version` key that neither HTTP/1.* nor HTTP/2
  data carries; dispatch on the data class instead, and return the
  `version` that make() dispatches on.
- httpv2: _make_data returned the frame's `length`, which
  ProtocolBase.__init__ took as the parse length (payload only, nine
  octets short), so every HTTP/2 from_data raised; make() computes the
  Length itself, so the key is dropped.
- tests: new test_http_from_data_1154_unit; two test_http_unit cases that
  drove _make_data with a fake `version` key now use parsed data.

Closes #1154
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on cb7afe6ba: GOOD TO GO (ran on Sonnet; author Opus)

  • Both root causes confirmed on main. In http.py, _make_data dispatched on data.get('version', 0), but neither data class has that field. In httpv2.py, _make_data returned the payload-only length, and ProtocolBase.from_data (protocol.py:798) passes that length to unpack as the parse length. I re-read both myself.
  • Byte-exact: 231/231 HTTP frames rebuild with equal bytes and info. With test(generators): HTTP/2 sample frames are all written on stream 0 #1161 applied the count is 237/237.
  • Edited tests: the fake version key pinned a defect. The only caller of _make_data is from_data. The old message invalid HTTP version: N is still raised from read/make (:193, :230), and nothing pins the _make_data text.
  • Fails without the fix: 32 failed with both sources reverted, and 28 with only httpv2.py reverted.
  • Follow-ups, not blocking:
    • A frame parsed after an HTTP/2 connection preface rebuilds without the preface (39 → 15 octets), because info does not carry it.
    • link/l2tpv2.py:367 _make_data also returns length. That is likely the same class of bug, but it is unverified on a real capture.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.87% (unit tier, Python 3.14, cb7afe6ba, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1878 91 580 22 94.34%
pcapkit/dumpkit 142 0 42 0 100.00%
pcapkit/foundation 2422 104 842 40 94.24%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15690 188 3960 163 98.18%
pcapkit/toolkit 487 65 144 1 85.42%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit e679bbe into main Oct 7, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1154-http-from-data-version branch October 7, 2026 00:51
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 7, 2026
JarryShaw added a commit that referenced this pull request Oct 7, 2026
- 4 new 1.5.0 entries: HTTP/2 sample stream identifiers (#1161),
  TCP option re-padding (#1163, breaking), format='pcap' output
  (#1164), declared lengths on truncated captures (#1166, breaking)
- extend the PCAP Header (#1159), HIP (#1160) and from_data
  round-trip (#1162) entries rather than duplicating them
- regenerate CHANGELOG.md with util/changelog_md.py
- process.rst: entry-count pin re-measured, 189 -> 193

tests/project: conventions_doc_claims, changelog_md and
documentation_claims pass; changelog_md.py --check is in step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(http): HTTP.from_data raises 'invalid HTTP version: 0'

1 participant