Skip to content

fix(httpv1): accept HTTP/1 messages with no header fields (#1041) - #1049

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1041-httpv1-headerless
Oct 5, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1041-httpv1-headerless

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort)
  • make test passes, and a test case covers the change
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

Closes #1041. _read_http_header refused a header with no CRLF, i.e. every message with zero field lines; _test_start_line mirrored it, so the dispatcher said unknown HTTP version too. Both now read it as a start line with zero fields.

That check was also what kept the HTTP/2 preface (header PRI * HTTP/2.0) out of HTTP/1. It is now refused by name (_HTTP2_PREFACE_HEADER, derived from http._HTTP2_PREFACE), same HTTP: invalid format; a regression test pins it in the parser, predicate and dispatcher.

New tests: headerless request, headerless response with body, make→read round trip, preface guard. The first three fail on main. Two existing tests that asserted the old ProtocolError as proof of delegation now assert the built bytes. Changelog pin 162 → 163.

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 5, 2026
@JarryShaw
JarryShaw force-pushed the fix/1041-httpv1-headerless branch from 2ea2efd to 0ab5320 Compare October 5, 2026 20:33
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 5, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

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

  • Headerless messages parse. GET / HTTP/1.0\r\n\r\n and HTTP/1.0 200 OK\r\n\r\nbody now parse directly and through HTTP. On main they raised invalid format and unknown HTTP version.
  • The HTTP/2 preface is still refused, with the same exception types and messages as main. This holds for the full, truncated and corrupt-tail preface across HTTPv1, _test_start_line and HTTP. The only change is that __cause__ is now None.
  • Disclosed widening, judged acceptable. Whitespace variants such as PRI * HTTP/2.0\r\n\r\n now parse as HTTP/1 version 2.0. The dispatcher's byte-exact 24-octet preface check still catches every real preface first. main already parsed PRI * HTTP/2.0 with fields as HTTP/1.
  • The predicate and the parser agree on ~45 crafted inputs and 40,000 fuzzed ones. The one disagreement is an orphan continuation line, which main has too.
  • No consumer assumes at least one header field. The edited tests still test what their names say.
  • New tests: they fail on main (6 + 1) and pass on the head.
  • Changelog: changelog_md.py --check is in step, and the pin is 164 (measured 164).

Not breaking: no signature or pack path changes.

Nits: test_httpv1_only_raises_protocol_error still labels the preface entry "no CRLF" under the converted sites, and no test pins the whitespace variants.

Follow-up filed as #1050: from_data fails on any empty-bodied message. That is pre-existing on main, and I reproduced it.

@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 5, 2026
RFC 9112 allows zero field lines, but `_read_http_header` unpacked the
header's CRLF split two ways and refused a header that was only the start
line, so `GET / HTTP/1.0\r\n\r\n` raised `ProtocolError` and
`HTTPv1(method='GET', uri='/', http_version='1.1')` could not read back the
bytes its own `make` built. `_test_start_line` mirrored the check, so the
dispatcher answered `unknown HTTP version` for the same bytes.

A CRLF-less header is now a start line with zero fields in both. That check
was also what refused the HTTP/2 connection preface, whose header is exactly
`PRI * HTTP/2.0`; both now refuse it by name (`_HTTP2_PREFACE_HEADER`), to
the same `HTTP: invalid format`.
@JarryShaw
JarryShaw force-pushed the fix/1041-httpv1-headerless branch from 0ab5320 to 82ec132 Compare October 5, 2026 21:21
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 5, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 82ec132a8: GOOD TO GO (ran on Sonnet; rebase re-check).

The diff over pcapkit/ and tests/ matches the reviewed head 0ab5320fa exactly: identical git patch-id --stable and the same three files. Only 1.5.0.rst, CHANGELOG.md and the process.rst:104 pin changed. The count is 165, matching the pin, and changelog_md.py --check is in step. The #1038 and #1041 entries each appear once, with no conflict markers. test_http_unit gives 64 passed, test_conventions_doc_claims 37 passed and test_changelog_md 60 passed. The earlier verdict stands.

@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 5, 2026
@JarryShaw
JarryShaw merged commit eeb3ab9 into main Oct 5, 2026
73 checks passed
@JarryShaw
JarryShaw deleted the fix/1041-httpv1-headerless branch October 5, 2026 23:50
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 5, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
@JarryShaw JarryShaw moved this to Done in PyPCAPKit Oct 6, 2026
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.

httpv1: a message with no header fields fails to parse

1 participant