From d6bcd33d1cefa98d894fefda11731e8760c164cc Mon Sep 17 00:00:00 2001 From: AgentDudu Date: Tue, 20 Oct 2026 15:39:07 +0700 Subject: [PATCH] feat: incremental request parser rejecting smuggling-prone requests Push-style parser owning its buffer: feed()/next()/reset() keep-alive with pipelining, byte-exact across split feeds. Enforces maxHeadBytes/maxBodyBytes/maxHeaderCount before consuming bodies. Rejections: bare-LF line endings, obsolete folding, header line without colon, duplicate Content-Length, Content-Length with Transfer-Encoding, non-token method/name chars, non-digit CL, non-1.0/1.1 versions. keepAlive() token-list aware; close wins. Traps found while testing: explicit ctor + '= {}' default arg is ill-formed (copy-list-init); head-size guard must only fire while the terminator is still missing or body bytes falsely trip it. --- CMakeLists.txt | 2 + docs/ARCHITECTURE.md | 7 +- src/http/request.cpp | 293 +++++++++++++++++++++++++++++++++++++++++ src/http/request.h | 83 ++++++++++++ tests/test_request.cpp | 184 ++++++++++++++++++++++++++ 5 files changed, 566 insertions(+), 3 deletions(-) create mode 100644 src/http/request.cpp create mode 100644 src/http/request.h create mode 100644 tests/test_request.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 6387ed8..20f9ad2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,6 +32,7 @@ set(HTTP_WARNINGS -Wall -Wextra -Wpedantic -Wshadow -Wconversion -Werror) add_library(http STATIC src/http/status.cpp src/http/headers.cpp + src/http/request.cpp src/http/response.cpp) target_include_directories(http PUBLIC src) target_compile_options(http PRIVATE ${HTTP_WARNINGS}) @@ -67,6 +68,7 @@ enable_testing() add_executable(unit_tests tests/test_main.cpp tests/test_socket.cpp + tests/test_request.cpp tests/test_response.cpp) target_include_directories(unit_tests SYSTEM PRIVATE external/doctest) # doctest is a vendored header: warnings from it are not ours to fix, diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index fc7bc23..9e96feb 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -17,9 +17,10 @@ until then. send/recv, reported to the reactor from poll as `ErrorKind::Interrupted`. - `glibc` needs `_GNU_SOURCE` with `-std=c++20` for `accept4`/`SOCK_CLOEXEC`. - glibc `setsockopt` requires `int` option size; Winsock is lenient. -- `http/` — status, headers, response serialization landed (strict head: CTL/CR/LF/NUL - rejected, implied Content-Length, chunk encoder). Pending: request parser, URL - decode/normalise, MIME. +- `http/` — status, headers, request parser, response serialization landed. Parser is + push/incremental, enforces head/body/count limits, and rejects bare-LF endings, + obsolete folding, duplicate `Content-Length`, `Content-Length`+`Transfer-Encoding`, + non-token names/methods, unknown versions. Pending: URL decode/normalise, MIME. - `app/` — signal handling (pending, win + posix) - `server/` — listener, connection, static file handler (pending) diff --git a/src/http/request.cpp b/src/http/request.cpp new file mode 100644 index 0000000..1f6050d --- /dev/null +++ b/src/http/request.cpp @@ -0,0 +1,293 @@ +#include "http/request.h" + +#include +#include + +namespace http { +namespace { + +bool isOWS(char c) { return c == ' ' || c == '\t'; } + +// Value bytes: printable, tab, or extended (obs); never CR/LF/NUL/other CTL. +bool isValueChar(char c) { + const unsigned char u = static_cast(c); + return u == '\t' || (u >= 0x20 && u != 0x7F); +} + +bool equalIgnoreCase(std::string_view a, std::string_view b) { + if (a.size() != b.size()) { + return false; + } + for (std::size_t i = 0; i < a.size(); ++i) { + if (std::tolower(static_cast(a[i])) != + std::tolower(static_cast(b[i]))) { + return false; + } + } + return true; +} + +} // namespace + +void RequestParser::feed(std::string_view data) { buf_.append(data); } + +void RequestParser::fail(Error e) { + error_ = e; + state_ = State::Failed; +} + +bool RequestParser::isTokenChar(char c) { + static constexpr std::string_view kDelims = "()<>@,;:\\\"/[]?={} \t"; + const unsigned char u = static_cast(c); + if (u <= 0x20 || u >= 0x7F) { + return false; + } + return kDelims.find(c) == std::string_view::npos; +} + +bool RequestParser::parseRequestLine() { + // RFC 9112: recipient MAY ignore empty lines before the request line. + while (pos_ < buf_.size()) { + if (buf_[pos_] == '\r') { + if (pos_ + 1 >= buf_.size()) { + return false; + } + if (buf_[pos_ + 1] != '\n') { + fail(Error::BadRequest); // bare CR + return true; + } + pos_ += 2; + } else if (buf_[pos_] == '\n') { + ++pos_; // lone LF empty line + } else { + break; + } + } + + const std::size_t start = pos_; + head_start_ = start; + const std::size_t nl = buf_.find("\r\n", start); + if (buf_.size() - start > limits_.maxHeadBytes) { + fail(Error::HeadTooLarge); + return true; + } + if (nl == std::string::npos) { + return false; + } + const std::string_view line(buf_.data() + start, nl - start); + const std::size_t sp1 = line.find(' '); + const std::size_t sp2 = sp1 == std::string_view::npos ? sp1 : line.find(' ', sp1 + 1); + if (sp1 == 0 || sp1 == std::string_view::npos || sp2 == std::string_view::npos || + line.find(' ', sp2 + 1) != std::string_view::npos) { + fail(Error::BadRequest); + return true; + } + req_.method = line.substr(0, sp1); + req_.target = line.substr(sp1 + 1, sp2 - sp1 - 1); + const std::string_view version = line.substr(sp2 + 1); + if (version != "HTTP/1.0" && version != "HTTP/1.1") { + fail(Error::VersionNotSupported); + return true; + } + req_.version = version.substr(5); // "1.0" / "1.1" + for (std::string_view tok : {req_.method, req_.target}) { + for (char c : tok) { + const unsigned char u = static_cast(c); + if (u < 0x21 || u > 0x7E) { + fail(Error::BadRequest); + return true; + } + } + } + for (char c : req_.method) { + if (!isTokenChar(c)) { + fail(Error::BadRequest); + return true; + } + } + pos_ = nl + 2; + scan_ = pos_ >= 2 ? pos_ - 2 : 0; // request line CRLF can start the block terminator + state_ = State::Headers; + return true; +} + +bool RequestParser::parseHeaderBlock() { + std::size_t end = buf_.find("\r\n\r\n", scan_); + scan_ = buf_.size() >= 3 ? buf_.size() - 3 : 0; + if (end == std::string::npos) { + if (buf_.size() - head_start_ > limits_.maxHeadBytes) { + fail(Error::HeadTooLarge); // head still growing past the cap + return true; + } + return false; + } + if (end + 4 - head_start_ > limits_.maxHeadBytes) { + fail(Error::HeadTooLarge); + return true; + } + const std::size_t head_start = head_start_; // lone-LF audit anchor + // A lone-LF line ending anywhere in the request head is a rejection: bare-LF + // requests must not be interpreted (smuggling class). A lone-LF terminator + // (\r\n\n) never matches the \r\n\r\n find above, so it just means NeedMore. + for (std::size_t i = head_start + 1; i < end + 2; ++i) { + if (buf_[i] == '\n' && buf_[i - 1] != '\r') { + fail(Error::BadRequest); + return true; + } + } + + std::size_t count = 0; + std::size_t ls = pos_; + const std::size_t body_start = end + 4; + while (ls < end) { + const std::size_t le = buf_.find("\r\n", ls); // ls..end block: present + const std::size_t lend = le == std::string::npos ? end : le; + if (buf_[ls] == ' ' || buf_[ls] == '\t') { + fail(Error::BadRequest); // obsolete line folding + return true; + } + const std::size_t colon = buf_.find(':', ls); + if (colon == std::string::npos || colon > lend || colon == ls) { + fail(Error::BadRequest); + return true; + } + const std::string_view name(buf_.data() + ls, colon - ls); + std::size_t vs = colon + 1; + while (vs < lend && isOWS(buf_[vs])) { + ++vs; + } + std::size_t ve = lend; + while (ve > vs && isOWS(buf_[ve - 1])) { + --ve; + } + const std::string_view value(buf_.data() + vs, ve - vs); + for (char c : name) { + if (!isTokenChar(c)) { + fail(Error::BadRequest); + return true; + } + } + for (char c : value) { + if (!isValueChar(c)) { + fail(Error::BadRequest); + return true; + } + } + if (++count > limits_.maxHeaderCount) { + fail(Error::TooManyHeaders); + return true; + } + const bool is_cl = equalIgnoreCase(name, "content-length"); + const bool is_te = equalIgnoreCase(name, "transfer-encoding"); + if (is_te) { + fail(Error::BadRequest); // chunked request bodies not supported yet + return true; + } + if (is_cl && req_.headers.contains("content-length")) { + fail(Error::BadRequest); // duplicate Content-Length + return true; + } + req_.headers.add(std::string(name), std::string(value)); + if (is_cl) { + std::size_t n = 0; + std::size_t cl = 0; + for (char c : value) { + if (c < '0' || c > '9' || cl > (limits_.maxBodyBytes + 1) / 10) { + cl = limits_.maxBodyBytes + 1; + break; + } + cl = cl * 10 + static_cast(c - '0'); + ++n; + } + if (n == 0 || value.size() != n) { + fail(Error::BadRequest); + return true; + } + if (cl > limits_.maxBodyBytes) { + fail(Error::BodyTooLarge); + return true; + } + body_len_ = cl; + } + ls = lend + 2; + } + + pos_ = body_start; + body_off_ = body_start; + state_ = body_len_ == 0 ? State::Done : State::Body; + return true; +} + +RequestParser::Result RequestParser::next() { + for (;;) { + switch (state_) { + case State::Failed: + return Result::Error; + case State::Done: + req_.body = body_len_ == 0 ? std::string_view{} + : std::string_view(buf_.data() + body_off_, body_len_); + return Result::Request; + case State::RequestLine: + if (!parseRequestLine()) { + return state_ == State::Failed ? Result::Error : Result::NeedMore; + } + break; + case State::Headers: + if (!parseHeaderBlock()) { + return state_ == State::Failed ? Result::Error : Result::NeedMore; + } + break; + case State::Body: + if (buf_.size() < body_off_ + body_len_) { + return Result::NeedMore; + } + state_ = State::Done; + break; + } + } +} + +bool RequestParser::keepAlive() const { + if (state_ != State::Done) { + return false; + } + bool has_close = false; + bool has_keepalive = false; + if (const std::string* conn = req_.headers.get("connection")) { + std::size_t s = 0; + while (s <= conn->size()) { + std::size_t c = conn->find(',', s); + if (c == std::string::npos) { + c = conn->size(); + } + std::string_view tok(conn->data() + s, c - s); + while (!tok.empty() && isOWS(tok.front())) { + tok.remove_prefix(1); + } + while (!tok.empty() && isOWS(tok.back())) { + tok.remove_suffix(1); + } + has_close = has_close || equalIgnoreCase(tok, "close"); + has_keepalive = has_keepalive || equalIgnoreCase(tok, "keep-alive"); + s = c + 1; + } + } + if (has_close) { + return false; + } + return req_.version == "1.1" ? true : has_keepalive; +} + +void RequestParser::reset() { + const std::size_t consumed = body_off_ + body_len_; + buf_.erase(0, consumed); + pos_ = 0; + scan_ = 0; + body_off_ = 0; + body_len_ = 0; + req_ = Request{}; + state_ = State::RequestLine; + error_ = Error::None; +} + +} // namespace http diff --git a/src/http/request.h b/src/http/request.h new file mode 100644 index 0000000..19cb72b --- /dev/null +++ b/src/http/request.h @@ -0,0 +1,83 @@ +#pragma once + +#include "http/headers.h" + +#include +#include +#include + +namespace http { + +// A complete HTTP/1.x request. method/target/body view bytes owned by the +// parser's buffer: valid until reset() or the next feed() that appends data — +// consume a request before feeding more (the reactor does). +struct Request { + std::string_view method; + std::string_view target; + std::string_view version; // "1.0" or "1.1" + Headers headers; // owned copies, parse order preserved + std::string_view body; // empty unless Content-Length was present +}; + +// Push parser: feed() raw bytes, call next() until Request/Error. Enforces the +// head/body limits and rejects smuggling-prone shapes: bare-LF line endings, +// obsolete header folding, duplicate Content-Length, Content-Length together +// with Transfer-Encoding, non-token method/name characters, unknown versions. +class RequestParser { + public: + struct Limits { + std::size_t maxHeadBytes = 64 * 1024; + std::size_t maxBodyBytes = 8 * 1024 * 1024; + std::size_t maxHeaderCount = 100; + }; + + enum class Result { NeedMore, Request, Error }; + enum class Error { + None, + BadRequest, // syntax, folding, duplicate CL, CL+TE, bad chars + HeadTooLarge, // head exceeds maxHeadBytes + TooManyHeaders, // header count exceeds maxHeaderCount + BodyTooLarge, // declared Content-Length exceeds maxBodyBytes + VersionNotSupported, // anything but HTTP/1.0 / HTTP/1.1 + }; + + explicit RequestParser(Limits limits) : limits_(limits) {} + RequestParser() = default; + + void feed(std::string_view data); + + // Call repeatedly after NeedMore. After Request, call reset() to consume it + // (leftover buffered bytes belong to the next pipelined request). + Result next(); + + const Request& request() const { return req_; } + Error error() const { return error_; } + + // Valid only after Result::Request. + bool keepAlive() const; + + void reset(); + + std::size_t bufferedBytes() const { return buf_.size() - pos_; } + + private: + enum class State { RequestLine, Headers, Body, Done, Failed }; + + bool parseRequestLine(); + bool parseHeaderBlock(); + void fail(Error e); + static bool isTokenChar(char c); + + Limits limits_; + std::string buf_; + std::size_t pos_ = 0; // first unparsed byte (request line / header scan cursor) + std::size_t scan_ = 0; // offset already searched for the header terminator + std::size_t head_start_ = 0; // first byte of the current request line + State state_ = State::RequestLine; + Error error_ = Error::None; + Request req_; + std::size_t body_off_ = 0; + std::size_t body_len_ = 0; +}; + +} // namespace http diff --git a/tests/test_request.cpp b/tests/test_request.cpp new file mode 100644 index 0000000..662040d --- /dev/null +++ b/tests/test_request.cpp @@ -0,0 +1,184 @@ +#include "doctest.h" + +#include "http/request.h" + +#include +#include + +using http::RequestParser; + +namespace { + +RequestParser::Result feedAndParse(RequestParser& p, std::string_view input) { + p.feed(input); + return p.next(); +} + +std::string feedAllBytewise(RequestParser& p, std::string_view input) { + for (std::size_t i = 0; i < input.size(); ++i) { + p.feed(input.substr(i, 1)); + if (const auto r = p.next(); r != RequestParser::Result::NeedMore) { + return r == RequestParser::Result::Request ? "request" : "error"; + } + } + return "needmore"; +} + +} // namespace + +TEST_CASE("valid minimal GET") { + RequestParser p; + CHECK(feedAndParse(p, "GET /a HTTP/1.1\r\n\r\n") == RequestParser::Result::Request); + const auto& r = p.request(); + CHECK(r.method == "GET"); + CHECK(r.target == "/a"); + CHECK(r.version == "1.1"); + CHECK(r.body.empty()); + CHECK(p.keepAlive()); // HTTP/1.1 default +} + +TEST_CASE("fields, headers, body") { + RequestParser p; + REQUIRE(feedAndParse(p, + "POST /up HTTP/1.0\r\nHost: h\r\nContent-Length: 5\r\nX-A: 1\r\n" + "X-A: 2\r\n\r\nhello") == RequestParser::Result::Request); + const auto& r = p.request(); + CHECK(r.method == "POST"); + CHECK(r.version == "1.0"); + CHECK(r.body == "hello"); + REQUIRE(r.headers.entries().size() == 4); + CHECK(r.headers.entries()[2].name == "X-A"); + CHECK(r.headers.entries()[2].value == "1"); + CHECK_FALSE(p.keepAlive()); // HTTP/1.0 default +} + +TEST_CASE("keep-alive header tokens") { + RequestParser p11; + REQUIRE(feedAndParse(p11, "GET / HTTP/1.1\r\nConnection: keep-alive, X\r\n\r\n") == + RequestParser::Result::Request); + CHECK(p11.keepAlive()); + RequestParser pc; + REQUIRE(feedAndParse(pc, "GET / HTTP/1.1\r\nConnection: close\r\n\r\n") == + RequestParser::Result::Request); + CHECK_FALSE(pc.keepAlive()); + RequestParser p10; + REQUIRE(feedAndParse(p10, "GET / HTTP/1.0\r\nConnection: Keep-Alive\r\n\r\n") == + RequestParser::Result::Request); + CHECK(p10.keepAlive()); +} + +TEST_CASE("never reads past declared body") { + RequestParser p; + REQUIRE(feedAndParse(p, "POST /x HTTP/1.1\r\nContent-Length: 3\r\n\r\nabcdef") == + RequestParser::Result::Request); + CHECK(p.request().body == "abc"); + p.reset(); + // the 3 leftover bytes are not body — they are the next request, and junk. + CHECK(feedAndParse(p, "\r\n") == RequestParser::Result::Error); + CHECK(p.error() == RequestParser::Error::BadRequest); +} + +TEST_CASE("pipelined requests, one buffer") { + RequestParser p; + REQUIRE(feedAndParse(p, + "GET /one HTTP/1.1\r\n\r\n" + "GET /two HTTP/1.1\r\n\r\n") == RequestParser::Result::Request); + CHECK(p.request().target == "/one"); + p.reset(); + REQUIRE(p.next() == RequestParser::Result::Request); + CHECK(p.request().target == "/two"); +} + +TEST_CASE("byte-at-a-time feed completes") { + RequestParser p; + CHECK(feedAllBytewise(p, "GET / HTTP/1.1\r\nA: b\r\n\r\n") == "request"); +} + +TEST_CASE("empty lines before request line are skipped") { + RequestParser p; + CHECK(feedAndParse(p, "\r\n\r\nGET / HTTP/1.1\r\n\r\n") == + RequestParser::Result::Request); + CHECK(p.request().target == "/"); +} + +TEST_CASE("bad requests are rejected") { + struct { + std::string_view name; + std::string_view input; + RequestParser::Error want; + } cases[] = { + {"no spaces", "GET /\r\n\r\n", RequestParser::Error::BadRequest}, + {"extra space", "GET / HTTP/1.1\r\n\r\n", RequestParser::Error::BadRequest}, + {"bad version", "GET / HTTP/2.0\r\n\r\n", RequestParser::Error::VersionNotSupported}, + {"version suffix", "GET / HTTP/1.15\r\n\r\n", + RequestParser::Error::VersionNotSupported}, + {"method not token", "G[T / HTTP/1.1\r\n\r\n", RequestParser::Error::BadRequest}, + {"header no colon", "GET / HTTP/1.1\r\nbroken\r\n\r\n", + RequestParser::Error::BadRequest}, + {"header no name", "GET / HTTP/1.1\r\n: v\r\n\r\n", RequestParser::Error::BadRequest}, + {"obs fold", "GET / HTTP/1.1\r\nA: 1\r\n b\r\n\r\n", RequestParser::Error::BadRequest}, + {"bare LF in headers", "GET / HTTP/1.1\r\nA: 1\nB: 2\r\n\r\n", + RequestParser::Error::BadRequest}, + {"duplicate CL", "POST / HTTP/1.1\r\nContent-Length: 1\r\nContent-Length: 1\r\n\r\n", + RequestParser::Error::BadRequest}, + {"CL plus TE", "POST / HTTP/1.1\r\nContent-Length: 1\r\nTransfer-Encoding: chunked\r\n\r\n", + RequestParser::Error::BadRequest}, + {"TE alone", "POST / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n", + RequestParser::Error::BadRequest}, + {"CL not numeric", "POST / HTTP/1.1\r\nContent-Length: 5x\r\n\r\n", + RequestParser::Error::BadRequest}, + {"CL empty", "POST / HTTP/1.1\r\nContent-Length: \r\n\r\n", + RequestParser::Error::BadRequest}, + }; + for (const auto& c : cases) { + CAPTURE(c.name); + RequestParser p; + CHECK(feedAndParse(p, c.input) == RequestParser::Result::Error); + CHECK(p.error() == c.want); + } +} + +TEST_CASE("limits") { + RequestParser::Limits limits; + limits.maxHeadBytes = 48; + limits.maxBodyBytes = 10; + limits.maxHeaderCount = 2; + { + RequestParser p(limits); + CHECK(feedAndParse(p, "GET / HTTP/1.1\r\nA: aaaa\r\nB: bbbb\r\nC: cccc\r\n\r\n") == + RequestParser::Result::Error); + CHECK(p.error() == RequestParser::Error::TooManyHeaders); + } + { + RequestParser p(limits); + CHECK(feedAndParse(p, "GET / HTTP/1.1\r\nA: 1\r\nB: 2\r\nC: 3\r\n\r\n") == + RequestParser::Result::Error); + CHECK(p.error() == RequestParser::Error::TooManyHeaders); + } + { + RequestParser p(limits); + CHECK(feedAndParse(p, "GET / HTTP/1.1\r\nLong: " + std::string(60, 'x') + "\r\n\r\n") == + RequestParser::Result::Error); + CHECK(p.error() == RequestParser::Error::HeadTooLarge); + } + { + // head keeps growing with no terminator in sight + RequestParser p(limits); + CHECK(feedAndParse(p, "GET / HTTP/1.1\r\nA: " + std::string(100, 'x')) == + RequestParser::Result::Error); + CHECK(p.error() == RequestParser::Error::HeadTooLarge); + } + { + // rejected at header time, before any body byte arrives + RequestParser p(limits); + CHECK(feedAndParse(p, "POST / HTTP/1.1\r\nContent-Length: 11\r\n\r\n") == + RequestParser::Result::Error); + CHECK(p.error() == RequestParser::Error::BodyTooLarge); + } + { + RequestParser p(limits); + CHECK(feedAndParse(p, "POST / HTTP/1.1\r\nContent-Length: 12345\r\n\r\n") == + RequestParser::Result::Error); + CHECK(p.error() == RequestParser::Error::BadRequest); + } +}