Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ loosely while pre-1.0 (breaking changes can land on minor bumps).
## [Unreleased]

### Fixed
- **Inbound HTTP body honors Content-Length.** `parse_http_request` now
takes at most `Content-Length` bytes from the header-read leftover, so
`Content-Length: 0` stays empty and extra pipelined or smuggled bytes
past the declared length are not adopted as this request's body.
- **SSE mailbox terminal after overflow.** `sse_mailbox_push(...,
force_deliver=true)` now evicts oldest events and delivers a terminal
envelope even when a prior push already set `overflowed`. Request-event
Expand Down
11 changes: 11 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1310,6 +1310,17 @@ if(INDEX_BUILD_TESTS)
)
add_test(NAME unit_circuit_breaker COMMAND unit_circuit_breaker)

# --- Unit tests for inbound HTTP/1.1 request framing ---
add_executable(unit_http_request
tests/test_http_request.cpp
src/api/http_request.cpp
)
target_include_directories(unit_http_request PRIVATE
${CMAKE_SOURCE_DIR}/include
${CMAKE_SOURCE_DIR}/third_party/doctest
)
add_test(NAME unit_http_request COMMAND unit_http_request)

# --- Unit tests for CORS allowlist policy ---
add_executable(unit_cors
tests/test_cors.cpp
Expand Down
8 changes: 7 additions & 1 deletion src/api/http_request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,13 @@ bool parse_http_request(int fd, HttpRequest& req) {
}
static constexpr size_t kMaxBody = 16 * 1024 * 1024; // hard cap
if (want > kMaxBody) return false;
req.body = leftover;
// TCP often delivers the header sentinel plus extra bytes in one
// recv. leftover is that prefix — take at most `want` so CL:0
// stays empty and pipelined/smuggled bytes past Content-Length
// never become this request's body. Connection: close drops the
// rest (same framing as ApiClient::read_response_body).
if (leftover.size() > want) leftover.resize(want);
req.body = std::move(leftover);
char buf[4096];
while (req.body.size() < want) {
size_t remaining = want - req.body.size();
Expand Down
85 changes: 85 additions & 0 deletions tests/test_http_request.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// tests/test_http_request.cpp — inbound HTTP/1.1 request framing.

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include "doctest.h"
#include "api/http_request.h"

#include <string>
#include <sys/socket.h>
#include <unistd.h>

using namespace arbiter;

namespace {

bool parse_from_bytes(const std::string& raw, HttpRequest& req) {
int sp[2];
if (::socketpair(AF_UNIX, SOCK_STREAM, 0, sp) != 0) return false;
const ssize_t w = ::write(sp[0], raw.data(), raw.size());
::shutdown(sp[0], SHUT_WR);
const bool ok = parse_http_request(sp[1], req);
::close(sp[0]);
::close(sp[1]);
return ok && w == static_cast<ssize_t>(raw.size());
}

std::string request_with(const std::string& headers, const std::string& body) {
return "POST /v1/orchestrate HTTP/1.1\r\n" + headers + "\r\n" + body;
}

} // namespace

TEST_CASE("exact Content-Length leftover is the body") {
HttpRequest req;
REQUIRE(parse_from_bytes(
request_with("Content-Length: 5\r\n", "hello"), req));
CHECK(req.method == "POST");
CHECK(req.path == "/v1/orchestrate");
CHECK(req.body == "hello");
}

TEST_CASE("body is capped to Content-Length when leftover overshoots") {
HttpRequest req;
REQUIRE(parse_from_bytes(
request_with("Content-Length: 5\r\n", "helloXXXXEXTRA"), req));
CHECK(req.body == "hello");
}

TEST_CASE("Content-Length 0 does not adopt leftover as body") {
HttpRequest req;
REQUIRE(parse_from_bytes(
request_with("Content-Length: 0\r\n",
"{\"agent\":\"smuggled\"}"), req));
CHECK(req.body.empty());
}

TEST_CASE("GET without Content-Length drops leftover bytes") {
HttpRequest req;
REQUIRE(parse_from_bytes(
"GET /v1/health HTTP/1.1\r\nHost: x\r\n\r\nPOST /v1/orchestrate HTTP/1.1\r\n\r\n",
req));
CHECK(req.method == "GET");
CHECK(req.path == "/v1/health");
CHECK(req.body.empty());
}

TEST_CASE("short body after Content-Length is rejected") {
HttpRequest req;
CHECK_FALSE(parse_from_bytes(
request_with("Content-Length: 10\r\n", "short"), req));
}

TEST_CASE("duplicate Content-Length is rejected") {
HttpRequest req;
CHECK_FALSE(parse_from_bytes(
request_with("Content-Length: 5\r\nContent-Length: 5\r\n", "hello"),
req));
}

TEST_CASE("Transfer-Encoding is rejected") {
HttpRequest req;
CHECK_FALSE(parse_from_bytes(
request_with("Transfer-Encoding: chunked\r\nContent-Length: 5\r\n",
"hello"),
req));
}
Loading