Skip to content

v2.0: Modernization (M1-M6, 44 tasks)#374

Draft
etr wants to merge 63 commits intomasterfrom
feature/v2.0
Draft

v2.0: Modernization (M1-M6, 44 tasks)#374
etr wants to merge 63 commits intomasterfrom
feature/v2.0

Conversation

@etr
Copy link
Copy Markdown
Owner

@etr etr commented Apr 30, 2026

Summary

Integration branch for the v2.0 modernization effort. Tasks land here individually (one merge commit per task) so the full v2.0 ships as a single reviewable PR.

This PR will remain draft until all milestones are complete.

Milestones

  • M1 — Foundation (TASK-001 … TASK-007): toolchain + build-system prerequisites
  • M2 — Response (TASK-008 … TASK-013)
  • M3 — Request (TASK-014 … TASK-020)
  • M4 — Handlers (TASK-021 … TASK-026)
  • M5 — Routing & Lifecycle (TASK-027 … TASK-036)
  • M6 — Release (TASK-037 … TASK-044)

Specs live under specs/ (product_specs, architecture, tasks).

Merged tasks

  • TASK-001 — Bump C++ standard floor to C++20

Test plan

Per-task validation runs through the groundwork validation loop on each task branch before merging here. Pre-merge of v2.0 to master:

  • ./configure && make clean on macOS (Apple Clang) and Linux (recent GCC)
  • make check green
  • CI matrix green across all supported toolchains
  • No -std=c++(11|14|17) regressions in tree
  • ChangeLog and README reflect v2.0 changes

🤖 Generated with Claude Code

etr and others added 3 commits April 30, 2026 23:24
Raises the project's C++ standard floor from C++17 to C++20 so that
subsequent v2.0 work can rely on concepts, std::span, <bit>,
designated initializers, and std::pmr without per-feature gates.

- m4/ax_cxx_compile_stdcxx.m4: replaced with upstream serial 25
  (autoconf-archive). The vendored serial 12 only accepted [11], [14],
  [17] and m4_fatals on anything else; serial 25 adds [20] and [23]
  alternatives plus the C++20 feature-test bodies.
- configure.ac:47: AX_CXX_COMPILE_STDCXX([17]) -> ([20], [noext],
  [mandatory]). [noext] keeps -std=c++20 (no gnu++20 extensions in
  ABI surface); [mandatory] aborts cleanly on too-old toolchains.
- configure.ac:224: dropped redundant -std=c++17 from the
  --enable-debug AM_CXXFLAGS branch. AX_CXX_COMPILE_STDCXX already
  appends -std=c++20 to $CXX, so leaving the override in would
  silently downgrade debug builds.
- Verified Makefile.am, src/Makefile.am, test/Makefile.am, and
  examples/Makefile.am: no per-subdirectory -std= overrides exist.
- .github/workflows/verify-build.yml:
  - Pruned gcc-9, clang-11, clang-12 matrix rows (incomplete C++20
    support: missing concepts/<bit>/<span> in libstdc++/libc++).
  - Bumped IWYU CXXFLAGS from -std=c++11 to -std=c++20.
- README.md: bumped Requirements to "g++ >= 10 or clang >= 13
  (Apple Clang from Xcode 15+)" and "C++20 or newer". Added a
  one-liner about gcc-toolset-14 on RHEL 9.
- README.CentOS-7: updated to reflect the C++20 floor and the
  gcc-toolset-14 workaround.
- ChangeLog: noted the standard bump under 0.20.0.

Verification (Apple Clang 21 on macOS):
- ./configure && make: succeeds with -std=c++20.
- make check: 17/17 tests pass.
- ./configure --enable-debug && make: clean under
  -Wall -Wextra -Werror -pedantic -std=c++20.
- make check (debug): 17/17 tests pass.
- grep -RE '-std=(c\+\+11|c\+\+14|c\+\+17|gnu\+\+(11|14|17))'
    configure.ac Makefile.am src test  -> zero matches.

Refs: PRD §2 NFR (modern C++ idioms), DR-001.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First task in the v2.0 milestone series (M1-Foundation). Raises the
project's C++ standard floor from C++17 to C++20.
Local planning artifacts from groundwork task scaffolding shouldn't be tracked.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

❌ Patch coverage is 73.44961% with 137 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.73%. Comparing base (d8b055e) to head (369c2a8).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/webserver.cpp 48.55% 12 Missing and 77 partials ⚠️
src/http_response.cpp 82.31% 1 Missing and 28 partials ⚠️
src/detail/body.cpp 89.06% 1 Missing and 6 partials ⚠️
src/httpserver/feature_unavailable.hpp 40.00% 0 Missing and 6 partials ⚠️
src/httpserver/detail/body.hpp 93.18% 2 Missing and 1 partial ⚠️
src/httpserver/http_method.hpp 93.75% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #374      +/-   ##
==========================================
+ Coverage   68.03%   69.73%   +1.70%     
==========================================
  Files          34       22      -12     
  Lines        1730     1953     +223     
  Branches      697      735      +38     
==========================================
+ Hits         1177     1362     +185     
- Misses         80      107      +27     
- Partials      473      484      +11     
Files with missing lines Coverage Δ
src/detail/http_endpoint.cpp 60.00% <ø> (ø)
src/http_resource.cpp 25.00% <100.00%> (+8.33%) ⬆️
src/http_utils.cpp 66.97% <100.00%> (ø)
src/httpserver/create_webserver.hpp 96.98% <ø> (ø)
src/httpserver/detail/http_endpoint.hpp 85.00% <ø> (ø)
src/httpserver/detail/modded_request.hpp 100.00% <ø> (ø)
src/httpserver/file_info.hpp 100.00% <ø> (ø)
src/httpserver/http_request.hpp 88.37% <ø> (ø)
src/httpserver/http_resource.hpp 85.10% <100.00%> (ø)
src/httpserver/http_response.hpp 100.00% <100.00%> (+4.00%) ⬆️
... and 7 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8b055e...369c2a8. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

etr and others added 26 commits May 1, 2026 00:49
Tighten the public/private header split so detail headers and the
HTTPSERVER_COMPILATION macro cannot leak to downstream consumers, and
add make-check assertions that protect the surface going forward.

Changes:
  - src/httpserver.hpp: #undef _HTTPSERVER_HPP_INSIDE_ after all child
    includes so the macro does not survive into a consumer's TU.
  - src/Makefile.am: move httpserver/details/http_endpoint.hpp out of
    nobase_include_HEADERS into noinst_HEADERS — distributed in the
    tarball but never installed under $prefix/include. Add
    -DHTTPSERVER_COMPILATION to AM_CPPFLAGS so the lib's own TUs see it.
  - test/Makefile.am: add -DHTTPSERVER_COMPILATION to AM_CPPFLAGS so
    first-party unit tests that legitimately include detail headers
    still compile.
  - configure.ac: stop injecting -DHTTPSERVER_COMPILATION into global
    CXXFLAGS. Scope is now per-directory (lib + tests only); examples
    build as true consumers via <httpserver.hpp>.
  - Makefile.am: new check-headers target with four sub-checks
    (A.1 direct public include must fail, A.2 direct detail include
    must fail, A.3 umbrella must compile cleanly, A.4 post-umbrella
    direct include must still fail) and a new check-install-layout
    target that runs `make install DESTDIR=...` to a stage and asserts
    no `details/` directory or `*_impl.hpp` file leaks. Both wired into
    check-local.
  - test/headers/: four one-line consumer TUs driving the checks.

Per the plan's Phase 3a-i, the detail-header gate stays dual-mode
(_HTTPSERVER_HPP_INSIDE_ || HTTPSERVER_COMPILATION) because
webserver.hpp still transitively includes details/http_endpoint.hpp;
TASK-014's PIMPL split will let a future change tighten that gate to
HTTPSERVER_COMPILATION-only.

Acceptance criteria verified:
  - 17/17 existing tests pass under release and --enable-debug.
  - check-headers A.1 fires with the gate error string.
  - check-install-layout: staged install has no details/ and no
    *_impl.hpp; httpserver.hpp + httpserverpp symlink installed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… for MSYS

TASK-001 raised the C++ floor to C++20, which broke matrix entries
running gcc-10, clang-14, and clang-15 (the autoconf C++20 feature
test rejects them). Drop those entries from extra/none, and bump the
lint and performance jobs (which were pinned to gcc-10) to gcc-14 so
they still exercise an older-but-supported toolchain.

The MSYS native job started failing with "microhttpd.h not found"
because the runner image no longer ships libmicrohttpd transitively.
Add libmicrohttpd-devel to the explicit pacman install line.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…set check

libmicrohttpd's <microhttpd.h> hard-asserts that _SYS_TYPES_FD_SET is
defined on Cygwin/MSYS, otherwise emitting `#error Cygwin with winsock
fd_set is not supported`. newlib defines that macro via <sys/select.h>,
included from <sys/types.h> only when __BSD_VISIBLE -- which in turn is
gated on _DEFAULT_SOURCE. Strict ANSI C++ (-std=c++NN, the floor we
adopted in TASK-001 with AX_CXX_COMPILE_STDCXX noext) suppresses
newlib's auto-define of _DEFAULT_SOURCE, so the macro never lands and
microhttpd.h refuses to compile.

This is unrelated to the C++ language mode -- _DEFAULT_SOURCE only
controls feature-test gating in system headers -- so defining it here
preserves DR-001's "noext" portability promise while fixing the build
on every Cygwin/MSYS consumer (not just our CI).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n PPA, revert MSYS libmicrohttpd-devel

Three small follow-ups now that the _DEFAULT_SOURCE Cygwin/MSYS fix has
landed:

1. The four test/headers/consumer_*.cpp gate tests added in TASK-002
   were missing the project's standard LGPL/copyright header, tripping
   the lint job once gcc-14 was running cpplint over them.

2. The "Install Ubuntu test sources" step was running
   add-apt-repository ppa:ubuntu-toolchain-r/test which talks to
   launchpad and has been hitting 504 Gateway Time-out across runs.
   With the C++20 floor we no longer need the toolchain PPA -- gcc-11
   through gcc-14 ship in stock ubuntu-22.04/24.04 repos, and
   clang-13/16-18 likewise. Keep just apt-get update.

3. The earlier "add libmicrohttpd-devel to MSYS pacman" attempt was
   wrong -- there is no such MSYS native package. The actual fix was
   the configure.ac _DEFAULT_SOURCE define landed in 5b78014; revert
   the bogus pacman entry so the install step stops failing first.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduce a new public header `src/httpserver/feature_unavailable.hpp`
defining `class feature_unavailable : public std::runtime_error`. The
constructor takes `(std::string_view feature, std::string_view
build_flag)` and composes a `what()` message that names both, e.g.
`"feature 'tls' unavailable: built without HAVE_GNUTLS"`.

The class is header-only and inline. It has no library dependencies
(only <stdexcept>, <string>, <string_view>), so any TU — including
later tasks like TASK-034 that need to throw it from sites in
build-time-disabled code paths — can include it without circular
header coupling. Keeping it inline also avoids ABI churn for what is
effectively a labelled std::runtime_error and keeps libhttpserver_la
sources untouched.

The header is re-exported from the umbrella `<httpserver.hpp>`
unconditionally (no `#ifdef HAVE_*` wrap): even a build with no
optional features must let consumers name `feature_unavailable` so
they can write `try { ... } catch (const httpserver::feature_unavailable&)`.

The TASK-002 inclusion gate is applied verbatim — direct inclusion of
the header without the umbrella or `HTTPSERVER_COMPILATION` errors
out, and `_HTTPSERVER_HPP_INSIDE_` does not leak post-umbrella (both
verified by the existing check-headers A.1–A.4 recipes).

A new unit test `test/unit/feature_unavailable_test.cpp` provides:
- a TU-scope `static_assert(std::is_base_of_v<std::runtime_error,
  httpserver::feature_unavailable>)` (acceptance criterion 1),
- a test that catches as `std::runtime_error` and asserts both the
  feature name and the build flag appear in `what()` (AC 2),
- a test that catches as the concrete type and confirms it slices to
  `runtime_error` correctly,
- a test with a different (feature, flag) pair to guard against
  hard-coded message text.

Verified locally:
- `make check`: 18/18 PASS (was 17, +1 for feature_unavailable),
- check-headers A.1–A.4 PASS,
- check-install-layout PASS (no details/ leak),
- staged install ships exactly one feature_unavailable.hpp at
  $(prefix)/include/httpserver/feature_unavailable.hpp,
- debug build (--enable-debug, -Werror -Wextra -pedantic) builds and
  tests cleanly.

Refs: PRD-FLG-REQ-004, PRD-FLG-REQ-005; §7 (feature availability).
Introduces a library-defined POD `httpserver::iovec_entry { const void* base;
std::size_t len; }` in a new public header `<httpserver/iovec_entry.hpp>`,
included by `<httpserver/http_response.hpp>` and the umbrella header. The
type replaces POSIX `struct iovec` at the public API surface, keeping
`<sys/uio.h>` out of every public header.

Layout pinning lives in `src/iovec_response.cpp` as six unconditional
static_asserts: three against POSIX `struct iovec` (size + iov_base /
iov_len offsets) per the spec, and three parallel asserts against
libmicrohttpd `MHD_IoVec` because that is the actual cast target on the
dispatch path. The MHD_IoVec asserts are an addition over the spec —
without them the reinterpret_cast bridge is the unsafe one. A TODO
sentinel comment (LIBHTTPSERVER_TODO_TASK004_MEMCPY_FALLBACK) documents
the memcpy fallback strategy that would activate if a divergent-layout
platform ever trips one of the asserts. Today every supported platform
(glibc, musl, macOS, FreeBSD, NetBSD, OpenBSD, illumos) shares the same
layout so the asserts pass and the reinterpret_cast is well-defined.

`iovec_response::get_raw_response()` now builds a contiguous
`std::vector<iovec_entry>` from its owned std::strings and
reinterpret_casts to `const MHD_IoVec*` when calling MHD. This proves
the cast bridge in production code today; TASK-010 will move the same
line into the future `details/body.hpp` factory.

Two new TDD-driven test programs:
- `test/unit/iovec_entry_test.cpp` — verifies POD traits (standard
  layout, trivially copyable), member types, layout equivalence with
  POSIX `struct iovec` from a consumer perspective, and the
  reinterpret_cast bridge round-trip.
- `test/unit/header_hygiene_iovec_test.cpp` — declares a colliding
  `struct iovec` before including `iovec_entry.hpp` directly. The TU
  compiling at all proves the new public header pulls in nothing from
  `<sys/uio.h>`. (The broader umbrella-leak concern — current umbrella
  transitively pulls `<sys/uio.h>` via gnutls and `<sys/socket.h>` —
  is out of scope for TASK-004 and is the remit of TASK-007's
  header-hygiene CI gate.)

Build: 20/20 tests pass under both default and `--enable-debug`
(-Wall -Wextra -Werror -pedantic -O0). `grep -E '#include\s+<sys/uio\.h>'
src/httpserver/*.hpp` returns no results. `make install` ships the new
header at `$prefix/include/httpserver/iovec_entry.hpp`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…vements

- Delete copy constructor and copy assignment on iovec_response to close
  CWE-416 use-after-free: the owning constructor stores entries_ as raw
  void* into owned_buffers_ strings; a defaulted copy would shallow-copy
  entries_ while deep-copying owned_buffers_ to new addresses, making
  entries_ dangle after source destruction. Move semantics are safe and
  kept. Static asserts in iovec_response_test.cpp guard this invariant.

- Remove the spurious '#include "httpserver/iovec_entry.hpp"' from
  http_response.hpp; http_response itself never uses iovec_entry, and
  iovec_response.hpp already includes it directly.

- Add @attention Doxygen contract to the non-owning iovec_response
  constructor documenting that caller buffers must outlive MHD_destroy_response.

- Remove duplicate offsetof/sizeof/alignof layout-pinning static_asserts
  from iovec_entry_test.cpp; authoritative copies live in iovec_response.cpp
  where the reinterpret_cast actually occurs.

- Add iovec_response_test.cpp (was untracked) with content-type forwarding
  tests and move-semantics tests for both constructor variants.

- Commit iovec_response.hpp, iovec_response.cpp, and test/Makefile.am that
  were modified/added in iter-1 but never staged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces the type-safe HTTP-method primitives that http_resource,
the route table, and lambda registration will consume.

- enum class http_method : std::uint8_t { get, head, post, put, del,
  connect, options, trace, patch, count_ }. Identifier `del` avoids
  the C++ keyword; wire token returned by to_string is "DELETE".
- struct method_set { std::uint32_t bits = 0; } with constexpr
  contains/set/clear/set_all/clear_all and defaulted operator==.
- Free constexpr noexcept bitwise operators (|, &, ^, ~, |=, &=, ^=)
  on http_method and method_set, including mixed (set, enum) overloads.
  All operators usable in constant expressions and at runtime ("consteval-
  friendly" without forbidding runtime use, which the route-table writer
  path needs).
- to_string(http_method) returning std::string_view for logging and
  the 405 Allow: header. Total over the 9 enumerators; out-of-range
  returns an empty view so logging stays robust against stale values.
- Layout/width invariants pinned at namespace scope:
  count_ <= 32, standard layout, trivially copyable,
  sizeof(method_set) == sizeof(uint32_t).
- Re-exported from <httpserver.hpp> and installed via
  nobase_include_HEADERS in src/Makefile.am.
- Test driver test/unit/http_method_test.cpp covers both compile-time
  static_asserts (round-trip, layout, bitwise composition, complement
  bounding, to_string totality) and 13 runtime LT_BEGIN_AUTO_TEST
  cases including a contract check that to_string matches
  libmicrohttpd's MHD_HTTP_METHOD_* tokens.

All 22 testsuite entries pass under the default build and under
--enable-debug (-Wall -Wextra -Werror -pedantic).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move every value-form #define from public headers into
inline constexpr declarations under httpserver::constants:

- DEFAULT_WS_PORT      -> std::uint16_t (9898)
- DEFAULT_WS_TIMEOUT   -> int           (180 seconds)
- DEFAULT_MASK_VALUE   -> std::uint16_t (0xFFFF)
- NOT_FOUND_ERROR      -> std::string_view ("Not Found")
- METHOD_ERROR         -> std::string_view ("Method not Allowed")
- NOT_METHOD_ERROR     -> std::string_view ("Method not Acceptable")
- GENERIC_ERROR        -> std::string_view ("Internal Error")

The new header src/httpserver/constants.hpp uses the established
two-token gate (_HTTPSERVER_HPP_INSIDE_ + HTTPSERVER_COMPILATION),
is re-exported from <httpserver.hpp>, and is registered in
nobase_include_HEADERS so it ships in the install layout.

Internal callers in webserver.cpp, http_utils.cpp,
create_webserver.hpp, and http_utils.hpp are migrated to the
namespaced names. The string_response call sites materialize a
std::string from the string_view to satisfy the existing ctor
signature.

A new unit test (test/unit/constants_test.cpp) pins the values
and types via static_assert, and uses #ifdef sentinels to
witness that the v1 macro names no longer leak into consumer
namespace after #include <httpserver.hpp>.

NOT_METHOD_ERROR has no in-tree caller; retained for v1 API
parity per the v2.0 mechanical-migration policy.

Acceptance:
- 23/23 tests pass (release + debug -Werror -Wall -Wextra)
- Filtered grep on src/httpserver/*.hpp shows no leftover
  value-constant #defines (include guards, _WINDOWS,
  _WIN32_WINNT, and COMPARATOR are out of scope per plan §2)
- Installed-header layout includes httpserver/constants.hpp

Closes TASK-006.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mark all five action items complete and set task status to Complete.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Update specs/tasks/_index.md to change TASK-006 status from 'In Progress'
to 'Done', matching the completed state in TASK-006.md and the pattern
used by TASK-003, TASK-004, and TASK-005.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a two-layer header-hygiene gate that locks in the "no backend
headers leak through <httpserver.hpp>" invariant from PRD-HDR-REQ-001..003.

Layer 1 -- compile/runtime sentinel (test/unit/header_hygiene_test.cpp):
  Includes only <httpserver.hpp>, then checks well-known include-guard
  macros (MHD_VERSION, _PTHREAD_H{,_}, GNUTLS_GNUTLS_H, _SYS_SOCKET_H{,_},
  _SYS_UIO_H{,_}). At runtime it prints the leaked headers and exits 1.
  Per-target CPPFLAGS overrides AM_CPPFLAGS so HTTPSERVER_COMPILATION
  and the build-tree -I src/httpserver/ entries are NOT in scope --
  mimics a real consumer translation unit.

Layer 2 -- preprocessor grep against staged install (`make check-hygiene`):
  Stages `make install DESTDIR=$(CHECK_HYGIENE_STAGE)` to a clean tree,
  preprocesses test/headers/consumer_umbrella_no_backend.cpp using ONLY
  -I$(CHECK_HYGIENE_STAGE)$(includedir), then greps cpp line markers
  for forbidden backend headers. HEADER_HYGIENE_STRICT controls
  fatality (default no -> informational; yes -> hard fail at TASK-020).

Both gates are wired into `make check`:
- header_hygiene runs as a check_PROGRAMS test, marked XFAIL_TESTS
  until M5 lands and the umbrella is clean. Automake's XPASS-as-error
  default is the explicit signal for TASK-020 to remove the marker.
- check-hygiene runs via check-local; in non-strict mode it prints an
  EXPECTED-FAIL banner with diagnostics and exits 0 so `make check`
  stays green during M2-M5 while keeping leak progress visible.

CI surface: new header-hygiene matrix entry in verify-build.yml runs
`make check-hygiene` as a focused, named GitHub Actions check.

TASK-020.md updated with explicit M5 close-out steps (delete
XFAIL_TESTS line + flip HEADER_HYGIENE_STRICT default).

Verified locally on macOS/aarch64 with gnutls 3.x, libmicrohttpd 1.0.5,
Apple Clang 15+: 24 tests / 23 PASS / 1 XFAIL (header_hygiene); the
sentinel correctly reports microhttpd, pthread, gnutls, sys/socket,
sys/uio leaks; check-hygiene reports EXPECTED-FAIL on staged install
(webserver.hpp still references private detail header until TASK-014).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… hygiene CI matrix

- check-local: build one DESTDIR=.shared-check-stage and pass it to both
  check-install-layout and check-hygiene via CHECK_*_SHARED=yes, halving
  the install cost of `make check`. Standalone invocations still do their
  own install.
- check-hygiene: gate the staged install behind a $(HYGIENE_STAMP) mtime
  sentinel so repeated standalone runs are no-ops when public headers
  haven't changed; bypassed when CHECK_HYGIENE_SHARED=yes.
- check-hygiene grep: anchor HEADER_HYGIENE_FORBIDDEN to a leading "/"
  so leak detection only matches absolute paths, not arbitrary substrings.
- clean-local: remove the stage directories on `make clean`.
- CI: header-hygiene matrix entry skips the unconditional `make check`
  step (the dedicated `make check-hygiene` step is the gate for that job).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the polymorphic body hierarchy that http_response's SBO buffer will
host (TASK-009) and the public body_kind enum that http_response::kind()
will return (TASK-011). TASK-008 ships only the standalone hierarchy:
each subclass is independently constructible, destructible, and
materializable, mirroring the corresponding v1 *_response::get_raw_response.

New public header (umbrella-included):
- httpserver/body_kind.hpp: enum class body_kind : std::uint8_t {
  empty, string, file, iovec, pipe, deferred }; empty=0 so a
  value-initialised body_kind matches the no-body state.

New private header (HTTPSERVER_COMPILATION-only, never installed):
- httpserver/details/body.hpp: abstract detail::body + 6 final
  subclasses (empty_body, string_body, file_body, iovec_body, pipe_body,
  deferred_body) plus per-subclass static_assert(sizeof <= 64) and
  static_assert(alignof(deferred_body) <= 16) for the SBO budget
  (DR-005).

Out-of-line definitions in src/details/body.cpp:
- materialize() per subclass mirrors v1 byte-for-byte
  (string=PERSISTENT, file=open/fstat/lseek/from_fd, iovec=CWE-190
  guard + reinterpret_cast to MHD_IoVec, pipe=from_pipe, deferred=
  from_callback with a static trampoline).
- Layout-pinning static_asserts duplicated from iovec_response.cpp
  (TASK-013 will remove the originals).
- pipe_body::~pipe_body() closes fd_ only if materialize() was never
  called (MHD owns it after a successful materialise).

New test:
- test/unit/body_test.cpp drives every subclass through MHD's
  daemon-independent inspection APIs (no daemon spun up). 12 tests, 29
  checks; the deferred trampoline is exposed as a public static so it
  can be unit-tested directly. Linked with explicit -lmicrohttpd
  (mirrors uri_log).

Observed sizes on libc++/arm64: empty=16, string=32, file=40, iovec=40,
pipe=16, deferred=40. All well under the 64 B SBO budget — TASK-010
will not need the heap-fallback branch on supported toolchains.

Out of scope (TASK-009/010): http_response wiring, body_inline_
fallback, kind() accessor, removal of v1 *_response subclasses.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies fixes from the iter1 review pass on the detail::body hierarchy:

file_body (CWE-367 / perf):
- Open + fstat moved to constructor; size() is now accurate immediately.
- Drops lseek(SEEK_END); materialize() uses st_size from fstat.
  Closes the TOCTOU window between size discovery and the fd handed
  to MHD_create_response_from_fd, and removes the side-effect on the
  fd's read position.
- Adds destructor that closes fd_ only when MHD never took ownership
  (materialized_ stays false until from_fd returns non-null).

deferred_body (CWE-476):
- trampoline() guards against null cls and empty producer_ before
  invoking the std::function. MHD's callback path doesn't catch C++
  exceptions, so a bad_function_call would terminate in MHD's IO
  thread; the guard returns MHD_CONTENT_READER_END_WITH_ERROR instead.
- Constructor asserts producer_ is non-empty (debug-only precondition).

Header docs:
- file_body: documents path-canonicalisation contract (O_NOFOLLOW only
  blocks the final component) and fd ownership lifecycle.
- iovec_body: documents the borrowed-pointer lifetime contract
  (iov_base buffers must outlive the MHD_Response*) and the heap
  allocation note from DR-005.
- deferred_body: documents the std::function SBO caveat — capturing
  more than the implementation-defined threshold silently heap-allocates.

Tests:
- file_body_size_known_before_materialize: size() must be correct at
  construction (21 bytes for test_content), not only after materialize.
- deferred_body_trampoline_null_cls_returns_error: trampoline with
  cls==nullptr returns MHD_CONTENT_READER_END_WITH_ERROR rather than
  dereferencing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Brings in the polymorphic detail::body hierarchy plus iter1 review-pass
fixes (file_body TOCTOU, deferred_body null-callable guard, header
lifetime/ownership docs, and accompanying tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sweeps in groundwork-generated planning content that had been left
untracked across recent task work, and adds .DS_Store to .gitignore so
macOS metadata stops appearing as untracked.

Planning content:
- specs/product_specs.md — top-level product spec.
- specs/architecture/ — system overview, architectural drivers,
  per-component specs (body-hierarchy, create-webserver, http-method,
  http-request, http-resource, http-response, route-table, webserver,
  websocket-handler), cross-cutting concerns, integration, feature
  availability, build/packaging, testing, observability, the DR-001..011
  decision records, open questions, documentation, and appendices.
- specs/tasks/M{1..6}-*/TASK-*.md — task definitions for the v2.0
  milestones (M1 foundation through M6 release). Pre-existing tasks
  TASK-006/007 were already tracked from prior commits; this adds the
  rest, including the M2 response, M3 request, M4 handlers, and M5
  routing-lifecycle definitions.

Review records:
- specs/unworked_review_issues/2026-04-30..2026-05-03_*.md — outputs
  from the iter1 review passes on TASK-001 through TASK-008. Captured
  for traceability; "unworked" denotes issues not yet folded back into
  task scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When TASK-003..008 were merged into feature/v2.0 they were not pushed
individually, so the cumulative push surfaced regressions across the
matrix. This sweeps them up.

Build error (basic ubuntu / valgrind / windows-IWYU):
- test/unit/body_test.cpp:56-60: static_cast<int>(uint8_t-enum) >= 0
  is always-true, breaking -Werror=type-limits. Replace with
  enumerator != body_kind::empty so the compile-time reference still
  guards against a missing enumerator without the bogus comparison.

cpplint (17 errors → 0):
- Include order:
  - src/details/body.cpp, src/iovec_response.cpp,
    src/httpserver/details/body.hpp,
    test/unit/{body_test,header_hygiene_test,http_method_test,
    iovec_entry_test}.cpp: move <microhttpd.h> and <sys/uio.h> into
    the C-system-header group so the layout is primary, c, c++, other.
- Missing includes:
  - src/details/body.cpp, src/iovec_response.cpp: add <string> for
    std::string in the file_body / iovec_response signatures.
  - src/iovec_response.cpp: add <utility> for std::move.
- Header guard:
  - src/httpserver/details/body.hpp: cpplint expects #ifndef GUARD as
    the first non-comment line. Move the SRC_HTTPSERVER_DETAILS_BODY_HPP_
    guard above the HTTPSERVER_COMPILATION #error block (which now
    lives inside the guard).
- Misc:
  - body_kind.hpp: NOLINT(build/include_what_you_use) on the `string`
    enumerator (cpplint mistook it for std::string).
  - body_test.cpp:251: split single-line if-with-multiple-statements.
  - http_method_test.cpp:121: add space between [] and { in lambda.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
etr and others added 30 commits May 3, 2026 18:28
…pace)

Rename `httpserver::details` namespace → `httpserver::detail` across all
remaining source/test files so the namespace name matches the renamed
directory (per TASK-009 directory rename `src/{,httpserver/}details/` →
`src/{,httpserver/}detail/`). Carried-over headers and CPPs that still
qualified internal types as `httpserver::details::X` would otherwise
fail to compile with the now-canonical `httpserver::detail` declarations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Mark TASK-009 action items complete and flip status to Done
  (specs/tasks/M2-response/TASK-009.md, specs/tasks/_index.md).
- Document the deferral of `final` keyword + std::is_final_v AC from
  TASK-009 to TASK-013 (v1 response subclasses still inherit at this
  point, so sealing must wait until TASK-013 removes them).
- Sync architecture docs (DR-002, DR-005, 03-system-overview,
  04-components/{body-hierarchy,http-response,webserver},
  05-cross-cutting, 06-backend-integration, 09-testing,
  12-open-questions) and forward-looking task specs (TASK-014,
  TASK-015, TASK-020) to use `detail/` (singular) consistently.
- Persist remaining minor validation findings to
  specs/unworked_review_issues/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These edits were prepared in the parent worktree before the TASK-009
worktree branch landed, and the validation-fixer's iter-1 pass missed
absorbing them into the housekeeping commit:

- TASK-010: clarify the heap-fallback factory contract — must use
  `::operator new(sizeof(...))` + placement-new (not plain `new`) so
  that http_response's destructor (which always calls ~body() and then
  ::operator delete on the heap path) does not double-destroy.
- TASK-013: add the `final` action item and `static_assert(is_final_v)`
  AC that were deferred from TASK-009 because the v1 subclasses still
  inherit at TASK-009 time. This is the receiving end of the deferral
  documented in TASK-009.md's AC.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the seven canonical factories on http_response (string, file, iovec,
pipe, empty, deferred, unauthorized) plus a public kind() accessor.

Each factory placement-news the corresponding detail::body subclass into
the SBO buffer through a single private emplace_body<T> helper, so the
matched ::operator new(sizeof(T)) / ::operator delete pairing the
destructor relies on (TASK-009 OQ-4) lives in exactly one place — a
stray plain `new T(...)` in any factory would mismatch and trip ASan.

Status-code defaults match v1: 200 for content-bearing bodies, 204 for
empty(), 401 for unauthorized(). The unauthorized() factory replaces v1's
basic_auth_fail_response and digest_auth_fail_response with a single
scheme-parameterised entry; the digest path emits a static
WWW-Authenticate challenge and does NOT participate in libmicrohttpd's
nonce/opaque state machine (documented contract gap).

Tests: 17 LT_BEGIN_AUTO_TESTs in test/unit/http_response_factories_test.cpp
exercise kind(), default and overridden Content-Type, file-missing
non-throw semantics, iovec span deep-copy, pipe fd ownership transfer
(destructor closes), deferred capture lifetime, and the AC-mandated
byte-for-byte WWW-Authenticate header. All 27 testsuite entries pass
locally with -j1; cpplint clean on the three changed files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion)

http_response::unauthorized() now rejects scheme/realm values containing
CR, LF, or NUL with std::invalid_argument (CWE-113), and escapes
embedded double-quote characters in realm per RFC 7235 §2.1
quoted-string rules (CWE-116). Without these guards a caller passing
attacker-influenced input through scheme or realm could splice
additional headers into the response or produce a malformed
WWW-Authenticate value that downstream parsers misinterpret.

Tests: 8 new LT_BEGIN_AUTO_TESTs covering CR / LF / CRLF / NUL in both
scheme and realm, plus a quote-escape test asserting the
backslash-escaped form `Basic realm="foo\"bar"`. All factories suite
tests pass locally; cpplint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mark TASK-010 as Done in both the task file and specs/tasks/_index.md,
tick the three Action Items checkboxes, and record the
2026-05-03 20:41 review run's 34 minor unworked findings under
specs/unworked_review_issues/. No critical or major findings; the
deferred items are predominantly cosmetic/style suggestions and
forward-task hooks for TASK-011 (accessor const-correctness) and
TASK-012 (fluent setters).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the v1 insert-on-miss accessors on http_response with
const, string_view-returning lookups that do NOT mutate the underlying
maps. Fixes PRD-RSP-REQ-002 (callable on const&) and
PRD-RSP-REQ-003 (no insert on miss).

Changes:
- http_utils.hpp: add `is_transparent` to header_comparator so
  header_map::find(string_view) is a heterogeneous lookup.
- http_response.hpp:
  * get_header / get_footer / get_cookie now take string_view, are
    const, and return std::string_view (empty on miss).
  * get_headers / get_footers / get_cookies marked noexcept and
    return const http::header_map&.
  * Add get_status() (noexcept) as the v2 spelling of the status
    code accessor; get_response_code() retained as a compatibility
    alias while v1 subclasses still inherit (TASK-013 removes both
    together with webserver.cpp:1336's dispatch path).
  * Lifetime contract documented above the single-key accessors.
- http_response.cpp: out-of-line definitions for the three single-key
  accessors via a shared header_map_find_view helper.
- test/unit/http_response_test.cpp: add 11 tests covering const
  callability, no-insert-on-miss for headers/footers/cookies, empty
  view on miss, read-back after with_header from const&, get_status
  / kind / get_headers noexcept, single-key accessors take
  string_view, case-insensitive lookup, view reflects replacement.
  Also tighten existing `auto headers = resp.get_headers()` lines
  to `const auto&` (avoids needless map copies now that the
  accessor returns by reference).
- test/integ/basic.cpp: silence -Werror,-Wunused-variable on the
  smoke `auto checksum = response->get_footer(...)` line; the
  variable was harmless under the v1 const-string& return but a
  string_view dtor is trivial so the warning now fires.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the void-returning v1 with_header/with_footer/with_cookie
methods with ref-qualified overload pairs that return http_response&
(for lvalue chains) and http_response&& (for factory rvalue chains),
and add a matching with_status(int) setter that did not exist before.
This unblocks the AC chain

    auto r = http_response::string("hi")
                 .with_header("X-Foo", "bar")
                 .with_status(201);

end-to-end while preserving SBO inline placement (no intermediate
move-construction). String parameters are taken by value and
forwarded into the underlying header_map via insert_or_assign so an
rvalue caller pays no extra allocation.

Cookie API decision (action item #4): keep the v1 (name, value)
string-pair shape; structured cookie type with first-class
attribute fields is intentionally deferred to a follow-up task and
can be added as a non-breaking overload alongside the existing API.
Documented on the with_cookie Doxygen.

Backward compatibility is preserved: all ~30 existing statement-form
call sites in test/unit/http_response_test.cpp, examples/, and
src/webserver.cpp:1348 keep compiling unchanged because the new
return type is a non-[[nodiscard]] reference.

Tests: 9 new tests in http_response_test.cpp pin the contract
(factory chain, lvalue chain identity, ref-qualifier dispatch via
static_assert, statement-form regression, with_status round-trip
and composition-safety, observable mutation through returned ref,
move-friendly by-value parameters). One additional test in
http_response_factories_test.cpp asserts the SBO-inline invariant
through the http_response_sbo_test_access friend struct. Full
testsuite: 27 entries, 26 PASS + 1 XFAIL (header_hygiene, expected
until M5), under both release and --enable-debug -Werror builds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… overload helper)

- Add CRLF/NUL validation to with_header, with_footer, with_cookie:
  reject key or value containing \r, \n, or \0 via std::invalid_argument
  (CWE-113 header injection, security-reviewer-iter1-1/2/3)
- Add range validation to with_status: reject codes outside [100,599]
  per RFC 9110 §15 (security-reviewer-iter1-4)
- Refactor 8 overload pairs to delegate to private do_set_* helpers,
  eliminating duplicated mutation logic (code-simplifier-iter1-1)
- Add 16 new unit tests covering CRLF/NUL rejection and status bounds,
  following the TDD RED→GREEN cycle

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
29 minor + 2 major findings deferred for follow-up; no critical issues.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deletes the eight public *_response subclasses (string/file/iovec/pipe/
deferred/empty/basic_auth_fail/digest_auth_fail) and the dispatch virtuals
(get_raw_response/decorate_response/enqueue_response) from http_response,
making the new factory-based surface (TASK-009..012) the only way to
build a response.

Phase 1 — migrate every consumer to the v2 surface:
  * webserver.cpp/http_resource.cpp internal callers switched off
    string_response to http_response::string()/empty().
  * test/unit + test/integ + examples/* migrated to factories +
    with_status()/with_header() chains.
  * test/unit/iovec_response_test.cpp deleted (covered by
    http_response_factories_test).

Phase 2 — delete the v1 surface:
  * Eight v1 hpp + cpp pairs deleted.
  * src/Makefile.am drops the deleted sources/headers and the
    HAVE_BAUTH conditional.
  * <httpserver.hpp> umbrella drops the eight removed includes.
  * http_response gains `final`; destructor de-virtualised; the legacy
    2-arg constructor (security-reviewer #24 from TASK-012) and the
    get_response_code() shim are gone; struct MHD_Connection/MHD_Response
    forward declarations dropped from the public header.
  * webserver gains static dispatch helpers materialize_response() and
    decorate_mhd_response(); friended into http_response so it can read
    body_ without widening the public API.
  * file() factory now defaults Content-Type to application/octet-stream
    (matching v1 file_response).
  * empty_render returns a default-constructed http_response so the
    status_code = -1 sentinel keeps routing to internal_error_page (the
    default_render_method test pins this).
  * finalize_answer's catch blocks now wrap internal_error_page() with
    an inner try/catch that falls back to force_our=true so a throwing
    user-supplied internal_error_resource can never escape into MHD.

Acceptance:
  * grep 'class \w+_response :' src/httpserver/*.hpp returns no public
    subclass declarations.
  * grep 'get_raw_response|decorate_response|enqueue_response'
    src/httpserver/*.hpp returns only doxygen prose.
  * static_assert(std::is_final_v<http_response>) compiles
    (test/unit/http_response_sbo_test.cpp).
  * make check: 25 PASS / 1 XFAIL (header_hygiene, expected pre-M5).

Behaviour change accepted per plan §2 / §10 and PRD §3.5: v1's
basic_auth_fail and digest_auth_fail bound to MHD's nonce/opaque
state machine via MHD_queue_basic/auth_required_response3; v2's
unauthorized() emits a static WWW-Authenticate challenge and
enqueues via MHD_queue_response. Four digest-auth round-trip tests
in test/integ/authentication.cpp updated to assert the v2 contract
(challenge issued, handshake does not complete, body remains FAIL).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ll-safety, digest-test contract)

- Reuse anonymous-namespace kForbiddenFieldChars in unauthorized() instead
  of the duplicate local kForbidden (code-quality-iter1-8, DRY).
- Move the operator<< friend declaration out of the now-misleading
  protected: section in http_response.hpp; http_response is final, so the
  section was never reachable by subclasses.
- Collapse the duplicate catch(const std::exception&)/catch(...) block in
  finalize_answer to a single catch(...). Both branches did the same work.
- Add LT_CHECK_EQ(http_code, 401) to the canonical digest_auth integration
  test and TODO(v2-digest) markers to the four sibling digest_auth_*
  variants documenting why they are currently behaviourally redundant
  under the v2 static-challenge path (test-quality-iter1-10/28).
- Migrate examples/service.cpp render_* methods from
  shared_ptr(new http_response(...)) to make_shared<http_response>(...)
  (code-quality-iter1-6, idiomatic v2 form).
- Add iovec_factory_empty_span and iovec_factory_single_entry edge-case
  tests, with an expanded comment block on iovec_factory_deep_copies_span
  spelling out the span deep-copy / caller-buffer lifetime contract
  (test-quality-iter1-29).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Mark TASK-013 status as Done (matches the convention used by all other
  M2 tasks; addresses housekeeper-iter1-16) and tick off the six action
  items now satisfied by the implementation commit.
- Sync specs/tasks/_index.md row from "Not Started" → "Done".
- Sync specs/architecture/04-components/http-response.md to reflect the
  sealed `final` http_response and the ref-qualified `& / &&` `with_*`
  setter overloads added in TASK-009/TASK-012.
- Record the 33 unworked findings (1 major, 32 minor) from the validation
  pass in specs/unworked_review_issues/2026-05-04_013108_task-013.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves webserver's backend-coupled state (MHD_Daemon*, pthread mutexes,
ban set, allowance set, route table, route cache, websocket registry,
optional GnuTLS SNI cache) into a new internal class
detail::webserver_impl, leaving the public httpserver/webserver.hpp
carrying only the const config bag plus a single std::unique_ptr<impl>.

Acceptance criteria:
- grep '#include <microhttpd.h>' src/httpserver/webserver.hpp returns
  nothing.
- webserver is now non-copyable and non-movable (PIMPL ABI lock-down,
  asserted at compile time by the new webserver_pimpl test).
- sizeof(webserver) shrank by ~half (the backend state moved behind
  impl_); the new test bounds it at 144 * sizeof(void*) so future
  regressions surface immediately.
- All 26 v1 integration + unit tests still pass; header_hygiene stays
  XFAIL until TASK-020 finishes scrubbing the umbrella.

Implementation notes:
- detail/webserver_impl.hpp is gated #if !defined(HTTPSERVER_COMPILATION)
  -- strict one-mode form, since it is reachable only from the library
  TUs and never from the public umbrella.
- Dispatch helpers (request_completed, answer_to_connection,
  post_iterator, finalize_answer, complete_request, materialize_response,
  decorate_mhd_response, etc.) and the auxiliary MHD callbacks
  (policy_callback, error_log, access_log, uri_log, unescaper_func)
  moved to webserver_impl as static members so their MHD-typed
  signatures stay off the public header.
- webserver_impl carries a back-pointer `parent` to the owning webserver
  so dispatch helpers can read the const config (tcp_nodelay, unescaper,
  regex_checking, auth_handler, ...).
- http_response, http_request, http_resource, websocket_handler, and
  http::file_info gain `friend class detail::webserver_impl;` next to
  the existing `friend class webserver;` so the migrated dispatch
  helpers retain the same private access they had before.
- webserver.hpp drops <microhttpd.h>, <pthread.h>, <gnutls/gnutls.h>,
  <list>, <map>, <mutex>, <set>, <shared_mutex>, <unordered_map>, and
  the transitive include of detail/http_endpoint.hpp.
- detail/http_endpoint.hpp had a stale forward declaration of
  `class http_resource;` inside namespace httpserver::detail; removed
  because (a) it shadowed the real httpserver::http_resource for any
  caller doing name lookup from inside namespace detail, and (b) it
  was unused.
- test/unit/webserver_pimpl_test.cpp -- TASK-014 sentinel asserting the
  PIMPL-shaped invariants (non-copyable, non-movable, bounded size).
- test/unit/uri_log_test.cpp updated to call the migrated
  detail::webserver_impl::uri_log static through a thin trampoline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Set TASK-014 status to "In Progress" in TASK-014.md and _index.md row.
  Structural action items are checked off but the validation pass surfaced
  3 major and 24 minor unworked findings, so the task is not yet Done.
- Record the 27 unworked findings (0 critical, 3 major, 24 minor) from the
  validation pass in specs/unworked_review_issues/2026-05-04_115707_task-014.md
  for follow-up work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move webserver's backend state (MHD_Daemon*, mutexes, ban set, connection
table, route cache) into detail/webserver_impl.hpp behind a unique_ptr
impl_ pointer. Public webserver.hpp no longer includes <microhttpd.h> or
<pthread.h>; <sys/socket.h> residual is tracked under TASK-020.

Status: In Progress — 27 unworked validation findings (3 major, 24 minor)
recorded in specs/unworked_review_issues/2026-05-04_115707_task-014.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move every backend-coupled member of http_request behind a
unique_ptr<detail::http_request_impl>. The public header
src/httpserver/http_request.hpp no longer includes <microhttpd.h>
or <gnutls/gnutls.h>; the only backend-typed names that remain
visible at the public surface are forward-declared (MHD_Connection*
in the HTTPSERVER_COMPILATION-gated private ctor, and a narrow
typedef forward-decl of gnutls_session_t for the still-public
get_tls_session() return type that TASK-019 will remove).

Outer keeps: path, method, content, version, content_size_limit,
plus the impl_ unique_ptr. Everything else (the live MHD_Connection*,
unescaper, file table, parsed-args / cookies / cert caches, the
MHD trampolines build_request_*, fetch_user_pass(),
populate_all_cert_fields()) lives on http_request_impl. Public
methods are one-line forwarders.

The dtor is out-of-line in http_request.cpp so unique_ptr<impl>
sees the complete impl type. Move ctor/assign remain defaulted
and operate on the unique_ptr.

A new sentinel test/unit/http_request_pimpl_test.cpp asserts that
http_request appears non-copy/non-move-constructible from external
scope (private moves), and locks sizeof(http_request) at <= 24
pointer widths. Currently 14 pointers (112 B on LP64).

Acceptance criteria all green:
- grep -E '#include <(microhttpd|gnutls/gnutls)\.h>' on the public
  header returns nothing.
- All v1 request-side tests pass (basic, file_upload,
  authentication, http_resource, threaded, nodelay, ws_start_stop,
  uri_log, etc. -- 27 PASS + 1 XFAIL header_hygiene umbrella,
  unchanged from baseline; the umbrella sweep is TASK-020).
- sizeof(http_request) = 14 * sizeof(void*); asserted in the new
  sentinel.
- noinst_HEADERS lists the impl header so it ships in the source
  tarball but never installs to $prefix/include.
- No test reaches across the boundary into http_request_impl.
create_test_request now allocates the impl_ and stores headers/footers/
cookies/args/querystring/auth/requestor/tls flags on it, instead of on
http_request directly. The MHD-touching accessors in http_request_impl
(get_connection_value, get_headerlike_values, populate_args, fetch_user_pass,
has_tls_session) and in http_request (get_querystring, get_digested_user,
get_requestor) now branch on connection_ == nullptr to read from the
local maps when running through the test-request path.

Also adds a string_body::get_content() helper used by the new
create_test_request unit suite, friend-grants create_test_request access
to http_request::impl_, and ships the create_test_request unit binary
from the autotools build.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Persist the validation-loop reviewer findings (0 critical / 10 major /
54 minor) under specs/unworked_review_issues/. Status flag and index
sync were already part of the skeleton commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nly)

Brings in:
- e9175c1 TASK-015: http_request_impl skeleton (PIMPL split, structural only)
- fd6c984 TASK-015: wire test-request fallback through http_request_impl
- 01c425b TASK-015: housekeeping (review record)
Allocate http_request_impl from a std::pmr::monotonic_buffer_resource
owned by connection_state so the warm path stops touching the global
heap on every request. The arena is wired in via MHD's
NOTIFY_CONNECTION callback (new on STARTED, delete on CLOSED) and
release()-rewound by request_completed once modded_request -- and
therefore the impl's destructor -- has run. A keep-alive connection
reuses the same buffer for every subsequent request.

Internal pmr-aware containers (header_local/footer_local/cookies_local
stay default-allocated for the test path; querystring, requestor_ip,
client_cert_*, unescaped_args, path_pieces, and the auth strings move
to std::pmr) propagate the arena allocator through scoped construction.
files_ stays default-allocated by design: file_info owns disk-side
state and decoupling its lifecycle from the arena keeps that reasoning
local.

The public header keeps <memory_resource> hidden: the impl deleter is a
small forward-declared struct holding only a function pointer, so
sizeof(unique_ptr<impl, deleter>) is two pointers regardless of where
the impl was allocated. The deleter dispatches between operator-delete
(heap fallback / test-request path) and destructor-only (arena path).

Tests:
- New unit test test/unit/http_request_arena_test.cpp covers the three
  acceptance facts: (a) arena_.release() rewinds the bump pointer,
  (b) warm-path http_request_impl construction does not touch the
  upstream resource (custom counting upstream verifies zero allocs),
  (c) two consecutive impls land at the same address across release().
- Sequential make check passes except for the pre-existing
  create_test_request::method_uppercase failure (unrelated to TASK-016;
  reproduces on the baseline).
- AddressSanitizer reports no use-after-free / heap-use-after-free
  across any of basic/threaded/http_request_arena/http_request_pimpl/
  authentication/deferred under -fsanitize=address.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- security: document arena lifetime contract on http_request public API
- security: cap headers/args via max_args_count + max_args_bytes
- security: zero arena memory on reset_arena() to prevent disclosure
- perf: bump arena initial buffer from 4 KiB to 8 KiB
- perf: add debug fallback counter + stderr warning when pick_resource
  spills to heap so undersized arenas surface in test runs
- tests: add coverage for arg-size guards, arena zeroing, and warm-path
  zero-upstream-allocs with PMR containers populated

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant