Phase 2.1: incremental HTTP request parser - #16
Merged
Conversation
Header names/values now get the same CTL-byte hardening the URI got in Phase 2.1: names reject uc<0x20 and 0x7F (NUL truncates CGI env names in 3.3), values reject the same except HTAB (legal inside field-content per RFC 7230). Empty Host is also rejected -- nginx does the same, and 2.5's vhost matcher shouldn't need a fallback for a zero-length host. 8 new checks; total suite 61 passed, 0 failed.
…: mark 2.1 completed
There was a problem hiding this comment.
Pull request overview
This PR adds an incremental, line-oriented HTTP/1.x request parser (Request) designed to safely handle arbitrary TCP fragmentation and request pipelining, and introduces a framework-free unit test suite plus Makefile support to build/run it.
Changes:
- Implement
Requestas an incremental state machine with size caps, strict request-line/header validation, and error reporting via HTTP status codes (no exceptions). - Add a standalone unit test binary (
make unit) with broad coverage, including exhaustive fragmentation split-point sweeps. - Update build system for header dependency tracking (
-MMD -MP) and ignore the unit test binary in git.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/test_request.cpp |
Adds framework-free unit tests validating incremental parsing behavior, limits, and hardening cases. |
src/http/request.cpp |
Implements the incremental request parser state machine, validation rules, caps, and consumed-byte accounting. |
src/http/http.cpp |
Removes now-unneeded content (per diff, file is effectively emptied/removed). |
Makefile |
Adds dependency tracking and a unit target to build/run the new unit test binary. |
kanban.md |
Marks 2.1 complete and updates the parseFromBuffer contract description to match the implementation. |
includes/http.hpp |
Introduces the Request API and parsing state machine definition in the public header. |
.gitignore |
Ignores the unit test binary output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Requestclass: line-oriented incremental state machine (S_REQUEST_LINE → S_HEADERS → S_COMPLETE / S_ERROR)parseFromBufferreturns bytes-consumed so 2.5 can re-feed leftovers to a freshRequestmake unit-MMDheader-dependency tracking added so header edits trigger rebuildsScope
Standalone parser only. No server wiring (task 2.5), no body parsing (task 3.1) — both have clean hooks left in place.
docs/superpowers/specs/plans are local-only (gitignored).Test plan
make unit→ 88 passed, 0 failedmake re→ full server builds warning-free under -Wall -Wextra -Werror -std=c++98make fcleanleaves no artifacts;leaksreports 0 leaked bytes