v2.0: Modernization (M1-M6, 44 tasks)#374
Draft
etr wants to merge 220 commits into
Draft
Conversation
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) <[email protected]>
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) <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #374 +/- ##
==========================================
- Coverage 68.03% 67.08% -0.96%
==========================================
Files 34 30 -4
Lines 1730 2579 +849
Branches 697 1006 +309
==========================================
+ Hits 1177 1730 +553
- Misses 80 186 +106
- Partials 473 663 +190
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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) <[email protected]>
… 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) <[email protected]>
…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) <[email protected]>
…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) <[email protected]>
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) <[email protected]>
…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 <[email protected]>
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) <[email protected]>
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) <[email protected]>
Mark all five action items complete and set task status to Complete. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
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 <[email protected]>
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) <[email protected]>
… 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) <[email protected]>
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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) <[email protected]>
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) <[email protected]>
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) <[email protected]>
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) <[email protected]>
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) <[email protected]>
Action items in TASK-045.md ticked off and Status flipped to Done (matched by the TASK-045 row in tasks/_index.md). Multi-agent validation loop on 4dd7217 returned 8/8 approve after one fix iteration (housekeeper request-changes → fixed); the 4 major + 30 minor unworked findings are persisted to specs/unworked_review_issues/2026-05-21_173303_task-045.md for follow-up sweeps. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Skeleton only: API surface compiles and hooks can register/remove, but no phase fires yet (TASK-046..051 wire the phases). Validation: 8/8 reviewer agents approve after one fix iteration.
Adds scripts/check-file-size.sh, a third lint-lane gate alongside
check-complexity.sh (CCN) and check-duplication.sh (CPD). Counts
physical lines via wc -l for every .cpp/.hpp under src/ and fails if
any file exceeds FILE_LOC_MAX.
FILE_LOC_MAX defaults to 2700, just above the current worst offender
(webserver.cpp at 2673), so CI stays green on landing. Long-term target
is 500 lines — matches the per-module SLOC ceiling used by the sibling
project under ../artistai and the natural break point where everything
else already complies. The script header lists the seven current
offenders and documents the ratchet-down strategy (one refactor commit
at a time tightens the bar), mirroring how CCN_MAX was driven to 10.
Wires the gate into:
- Makefile.am: new lint-file-size target + EXTRA_DIST entry
- .github/workflows/verify-build.yml: new step in the ubuntu lint
lane, conditional on build-type == 'lint' (same gating as the
sibling gates)
Architecture spec is not yet updated; the same gap exists for the
existing CCN/CPD gates (see specs/unworked_review_issues/
2026-05-21_121150_manual-validation.md) and is best closed in one
sweep.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
First step of the FILE_LOC_MAX ratchet. Splits ip_representation
(struct + the five private parse helpers carved out during the CCN-10
sweep) into its own public sub-header httpserver/ip_representation.hpp.
http_utils.hpp keeps a re-include of the new header so existing
consumers of <httpserver/http_utils.hpp> still see the type without
any code change; the umbrella picks up the new header explicitly too
(every public sub-header is listed in <httpserver.hpp>). Detail
headers and downstream tests are unaffected — webserver_impl.hpp still
sees http::ip_representation through its existing http_utils.hpp
include.
FILE_LOC_MAX stays at 2700. The bar is pinned by the largest unfixed
file (webserver.cpp at 2673), so the threshold can't drop until the
top offender is decomposed. Each smaller-first step removes one file
from the offender list without ticking the bar; the bar steps happen
when the worst file shrinks.
Offender list updated in scripts/check-file-size.sh: six files remain
above the 500-line target.
Verification:
make check 51/51 PASS (includes header
hygiene, install layout, doxygen,
examples, readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=2700
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Step 2 of the FILE_LOC_MAX ratchet. Moves the auth/credentials member
declarations of class http_request — basic auth getters (get_user,
get_pass, get_digested_user), the TLS / client-certificate cluster
(has_tls_session, has_client_certificate, get_client_cert_*,
is_client_cert_verified, get_client_cert_not_before/after), and the
digest-auth verification entrypoints (check_digest_auth,
check_digest_auth_digest) — into a sibling public header
httpserver/http_request_auth.hpp.
Mechanism: in-class-body #include. The sibling header carries
declarations only and gates itself behind
SRC_HTTPSERVER_HTTP_REQUEST_HPP_INSIDE_CLASS_, which is #define'd
just before the include inside class http_request { ... } and
#undef'd immediately after. Including the sibling in any other
context raises a #error with a pointer back to http_request.hpp.
Doxygen still picks up the declarations through textual inclusion,
so the generated docs are unchanged. The header is installed
(nobase_include_HEADERS) so consumers building against an installed
libhttpserver see it transitively via <httpserver.hpp> /
httpserver/http_request.hpp; it is NOT added to the umbrella's
sub-header list because the inner gate forbids standalone inclusion.
No public ABI change: the methods remain member functions of
http_request, declared in the same access section, same signatures,
same noexcept. Consumer code (test/, examples/) is untouched.
FILE_LOC_MAX stays at 2700 — webserver.cpp (2673) still pins it.
Offender list down to five files.
Verification:
make check 51/51 PASS (includes hygiene,
install-layout, doxygen,
examples, readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=2700
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Step 3 of the FILE_LOC_MAX ratchet. Two extractions:
connection_state -> httpserver/detail/connection_state.hpp
The per-MHD_Connection PMR arena anchor (initial_buffer_ +
monotonic_buffer_resource + reset_arena()). Self-contained,
no webserver_impl coupling. As a bonus, http_request.cpp can
eventually narrow its #include from webserver_impl.hpp down
to just connection_state.hpp, dropping a heavy header (which
transitively pulls microhttpd / pthread / gnutls / regex) off
http_request.cpp's compile-time footprint -- not done in this
commit to keep the diff focused on the LOC ratchet.
dispatch / start-helper / MHD-trampoline method declarations ->
httpserver/detail/webserver_impl_dispatch.hpp
The 280-line tail of method declarations on webserver_impl --
start-helper overloads (add_*_mhd_options, compose_*_flags),
dispatch chain (requests_answer_first/second_step, finalize_answer
+ its CCN-10 sub-helpers), route-lookup / route-cache helpers,
auth short-circuit, post-iterator helpers, MHD trampolines,
GnuTLS PSK/SNI callbacks. Same in-class-body #include pattern
introduced in TASK-step-2 for http_request_auth.hpp: the sibling
header carries declarations only and gates itself behind
SRC_HTTPSERVER_DETAIL_WEBSERVER_IMPL_HPP_INSIDE_CLASS_, which is
#define'd/#undef'd around the include inside class webserver_impl
body. Standalone inclusion produces a #error.
webserver_impl.hpp ends up at 330 LOC (data members + lifecycle
ctors + the in-class-body include for the dispatch surface).
Behaviourally inert -- no ABI change, no public surface change. The
order of declarations inside class webserver_impl is preserved
verbatim because the sibling is included at the original textual
location.
FILE_LOC_MAX stays at 2700 -- webserver.cpp (2673) still pins it.
Offender list down to four files.
Verification:
make check 51/51 PASS (includes hygiene,
install-layout, doxygen,
examples, readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=2700
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Step 4 of the FILE_LOC_MAX ratchet. Moves the ip_representation method
bodies — both ctors, parse_ipv4 / parse_ipv6 + the carved-out helpers
(compute_ipv6_omitted_segments, parse_nested_ipv4, apply_ipv6_part),
operator< + its anonymous-namespace helpers (accumulate_octet_score,
is_v4_mapped_prefix_octet_pair, ipv4_mapped_prefix_invalid) — into
src/detail/ip_representation.cpp.
Co-extracts get_ip_str / get_port (the sockaddr -> string helpers).
They're declared in httpserver/http_utils.hpp as namespace-level free
functions, but functionally they're "sockaddr -> IP textual form" —
the same concern as ip_representation's sockaddr ctor. Lives alongside
its peers in the new TU.
http_utils.cpp now carries the URL / filename helpers, http_unescape,
load_file, header/arg dump helpers, base_unescaper, the
MHD-thin-wrapper functions (reason_phrase, is_feature_supported,
get_mhd_version), and the TASK-020 static_assert enum pin block. The
file is purely "string + HTTP misc utilities" again; the network /
address concern has moved out.
The new TU adds itself to libhttpserver_la_SOURCES in src/Makefile.am.
The two local bit-twiddling macros (CHECK_BIT / CLEAR_BIT) are
re-declared at the top of the new TU; both http_utils.cpp and the new
TU need them, and a shared header for two one-line macros would be
overkill.
FILE_LOC_MAX stays at 2700 -- webserver.cpp (2673) still pins it.
Offender list down to three files.
Verification:
make check 51/51 PASS (includes hygiene,
install-layout, doxygen,
examples, readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=2700
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const ip_representation& b, int i, | ||
| int64_t& a_score, int64_t& b_score) { | ||
| if (!(CHECK_BIT(a.mask, i) && CHECK_BIT(b.mask, i))) return; | ||
| a_score += (16 - i) * a.pieces[i]; |
| int64_t& a_score, int64_t& b_score) { | ||
| if (!(CHECK_BIT(a.mask, i) && CHECK_BIT(b.mask, i))) return; | ||
| a_score += (16 - i) * a.pieces[i]; | ||
| b_score += (16 - i) * b.pieces[i]; |
Step 5 of the FILE_LOC_MAX ratchet. Three concern-grouped sibling
sub-headers, each included from inside the webserver class body via
the same in-class-body #include pattern used for http_request_auth.hpp
and webserver_impl_dispatch.hpp:
webserver_routes.hpp register_path / register_prefix /
register_resource (templated + shared_ptr
overloads), the on_* shortcuts (on_get,
on_post, on_put, on_delete, on_patch,
on_options, on_head), the table-driven
route() overloads, and the matching
unregister_path / unregister_prefix /
unregister_resource.
webserver_websocket.hpp register_ws_resource (templated +
shared_ptr) and unregister_ws_resource.
webserver_hooks.hpp add_hook (11 overloads, one per
hook_phase) and the
HTTPSERVER_COMPILATION-gated
make_hook_handle_ factory.
Each sub-header gates itself on SRC_HTTPSERVER_WEBSERVER_HPP_INSIDE_CLASS_,
which webserver.hpp #define's before the include block and #undef's
after, so standalone inclusion raises a #error. The headers are
installed (nobase_include_HEADERS) so consumers see the declarations
transitively through <httpserver.hpp> -> webserver.hpp -> sibling.
They are NOT added to the umbrella's sub-header list because the
inner gate forbids standalone inclusion.
Public API order is preserved verbatim. No ABI change, no semantic
change. The fully-qualified @ref tags inside webserver_hooks.hpp
(@ref httpserver::hook_phase / @ref httpserver::hook_handle ...) are
unchanged in target; only the textual form needed qualification
because doxygen parses sub-headers without the enclosing namespace
context.
FILE_LOC_MAX stays at 2700 -- webserver.cpp (2673) still pins it.
Offender list down to two files.
Verification:
make check 51/51 PASS (includes hygiene,
install-layout, doxygen,
examples, readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=2700
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Step 6 of the FILE_LOC_MAX ratchet. The 1175-line TU is decomposed
along the section markers already in the file into four self-contained
units, each well below the 500-line target:
src/detail/http_request_impl.cpp 356 detail::http_request_impl
method bodies — non-TLS
section (connection-value
lookup, headerlike caches,
build_request_args + the
DoS-guard accumulator,
build_request_querystring,
populate_args, path-pieces
cache, set_arg family,
fetch_user_pass under
HAVE_BAUTH); plus the
http_request_impl_deleter
dispatch.
src/detail/http_request_impl_tls.cpp 265 HAVE_GNUTLS section.
scoped_x509_cert RAII helper,
has_tls_session,
get_tls_session,
has_client_certificate, the
anonymous-namespace x509
extractors (extract_x509_*,
verify_peer_certificate), and
populate_all_cert_fields.
Whole TU wrapped in
`#ifdef HAVE_GNUTLS` so
non-TLS builds contribute
nothing.
src/http_request_auth.cpp 286 Public-API forwarders for
the auth/credentials surface:
get_user / get_pass /
get_digested_user,
check_digest_auth /
check_digest_auth_digest,
and the high-level TLS /
client-cert accessors
(has_tls_session,
has_client_certificate,
get_client_cert_*,
is_client_cert_verified,
get_client_cert_not_before/
after). Matches the
http_request_auth.hpp
declaration grouping.
src/http_request.cpp 392 Residual: ctors / dtor /
public-API forwarders for
everything that isn't auth
(path, method, version,
content, header, cookie,
footer, args, files,
querystring), the
connection-arena ctor
wiring (pick_resource +
delete_impl_heap /
destroy_impl_arena, both
`static` so they stay
co-located with the ctor
that takes their address),
private setters used by
webserver_impl dispatch, and
operator<<.
All method bodies are byte-for-byte unchanged. Wiring up:
- libhttpserver_la_SOURCES gains the three new TUs.
- No header changes needed — http_request_impl.hpp already declares
every method body the new TUs implement.
FILE_LOC_MAX stays at 2700 -- webserver.cpp (2673) is now the lone
remaining offender. Step 7 takes it down and drops the bar to 500.
Verification:
make check ALL PASS (includes hygiene,
install-layout, doxygen, examples,
readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=2700
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Final step of the ratchet. The 2673-line webserver.cpp is decomposed
along the section markers already in the file into seven new TUs, each
focused on a single concern and well below the 500-line target. The
residual webserver.cpp keeps ctors / dtors / signal helpers / hooks
machinery and lands at 464 lines.
src/detail/webserver_setup.cpp 437 MHD option array
builders (add_base /
tls / gnutls / extended /
https_extra) + start-flag
composers, daemon lifecycle
(start, is_running, stop,
run, run_wait, get_fdset,
get_timeout, add_connection),
and block_ip / unblock_ip.
src/detail/webserver_register.cpp 360 register_path /
register_prefix /
register_resource (incl.
the shared register_impl_
funnel and detail::
register_v2_route mirror)
+ unregister_impl_ /
unregister_path /
unregister_prefix /
unregister_resource. Uses
the shared route_tier
helper.
src/detail/webserver_routes.cpp 411 on_methods_ funnel + the
seven on_* shortcuts +
both route() overloads +
the detail-namespace
lambda_shim helpers
(prepare_or_create_lambda_shim,
commit_handlers_to_shim,
insert_fresh_v1_entries,
upsert_v2_table_entry +
its sub-helpers). Uses
the shared route_tier.
src/detail/webserver_callbacks.cpp 477 MHD trampolines registered
with libmicrohttpd
(request_completed,
connection_notify, policy_
callback, error_log,
access_log, uri_log,
unescaper_func, PSK/SNI
cred handlers) + the
post_iterator family
(handle_post_form_arg,
setup_new_upload_file_info,
manage_upload_stream,
process_file_upload, the
post_iterator trampoline).
src/detail/webserver_websocket.cpp 227 HAVE_WEBSOCKET-gated TU.
decode_websocket_buffer
static helper +
upgrade_handler MHD
callback + the anonymous-
namespace handshake
helpers.
src/detail/webserver_dispatch.cpp 460 Dispatch support services:
not_found_page /
method_not_allowed_page /
internal_error_page /
log_dispatch_error /
run_internal_error_handler_safely
+ invalidate_route_cache +
lookup_v2 (the v2 3-tier
walk) + the route-table
helpers
(lookup_route_cache,
scan_regex_routes,
store_route_cache,
apply_extracted_params,
resolve_resource_for_request,
apply_auth_short_circuit,
dispatch_resource_handler).
src/detail/webserver_request.cpp 488 Request lifecycle:
should_skip_auth + its
normalize_path helper +
requests_answer_first_step /
requests_answer_second_step,
materialize_response /
decorate_mhd_response /
get_raw_response_with_fallback,
the websocket-upgrade
dispatch helpers
(validate_websocket_handshake,
complete_websocket_upgrade,
try_handle_websocket_upgrade),
materialize_and_queue_response,
finalize_answer,
complete_request,
resolve_method_callback,
and the answer_to_connection
entrypoint.
src/webserver.cpp 464 Residual: license / includes
/ signal helpers
(catcher, ignore_sigpipe)
/ webserver_impl ctor +
dtor / webserver ctor +
dtor + features() +
stop_and_wait() / the
TASK-045 hook bus
(register_hook_impl
anonymous-namespace helper
+ make_hook_handle_ +
the eleven add_hook
overloads).
One small companion header was extracted to share state across TUs:
src/httpserver/detail/route_tier.hpp Hoists the route_tier_kind
enum + route_tier_result
struct + classify_route_tier
from an anonymous namespace
in webserver.cpp into a
detail header. Both
webserver_register.cpp and
webserver_routes.cpp call
classify_route_tier; an
anonymous-namespace
definition no longer
suffices once the TU is
split. Marked inline so
the ODR holds across
translation units.
normalize_path (and its apply_normalized_segment helper) — formerly
file-scope statics in webserver.cpp — co-locate with their only caller
(webserver_impl::should_skip_auth) in webserver_request.cpp.
FILE_LOC_MAX drops from 2700 to 500, the long-term project target.
The header comment in scripts/check-file-size.sh records the seven
ratchet steps that drove it down.
Verification:
make check ALL PASS (includes hygiene,
install-layout, doxygen, examples,
readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=500
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Bring README.md back to a section-by-section reference walkthrough (25 H2 sections, 52 H3 subsections, ~1820 lines) covering the current API surface: full create_webserver builder reference, http_resource virtuals, the three routing families (on_* / route() / register_path & register_prefix), full http_request and http_response references, authentication (Basic / Digest / centralized / mTLS / SNI / TLS-PSK), WebSocket, daemon introspection and external event loops, threading contract (DR-008 / §5.1), error propagation (DR-009 / §5.2), and feature availability. Examples are linked rather than inlined: the grouped index in the README mirrors examples/README.md. examples/hello_world.cpp: drop the PRD §3.4 callout from the file header so the comment stays self-contained; the README block remains byte-for-byte synced via scripts/check-readme.sh. scripts/check-readme.sh and scripts/check-examples.sh both pass. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Wires the three connection-level lifecycle phases of the hook bus into the existing MHD callback sites. Closes long-standing feature request #332 (banned-IP log entry). Production code - accept_ctx extended from {peer} to {peer, accepted, reason}; `reason` is a std::optional<std::string_view> pointing at a string literal ("banned" / "not-allowed") with static storage duration. - Three new noexcept fire_* helpers on webserver_impl (declared in the dispatch sibling header, defined in src/hook_handle.cpp): each takes a shared_lock, snapshots the phase vector with reserve(8), releases the lock, then iterates with try/catch routed through log_dispatch_error (DR-009 §5.2). Mirrors TASK-027's route-cache promotion pattern. - connection_notify + policy_callback split out of webserver_callbacks.cpp into a new webserver_callbacks_lifecycle.cpp TU. The original would have overshot FILE_LOC_MAX after the firing- site code landed. webserver_callbacks.cpp shrinks to 432 lines. - MHD_OPTION_NOTIFY_CONNECTION closure pointer switched from nullptr to the owning webserver* so connection_notify can reach impl_->any_hooks_ / fire_connection_opened / fire_connection_closed. - policy_callback gains decision-derivation logic (accept_ctx.reason); extracted into anon-ns classify_decision() helper to stay under the CCN gate. - All three firing sites are gated by a relaxed atomic load on any_hooks_[phase] so the zero-hook path stays one branch + one atomic load (PRD-HOOK-REQ-008). - accept_decision's throwing-hook semantics are a structural guarantee: fire_accept_decision returns void and `decision` is captured in a local before the fire call. Pre-existing build fix - src/detail/webserver_dispatch.cpp was missing `using std::map` and `using httpserver::http::http_utils` directives (left out of the TASK-15f8083 7-way split). Added so fresh worktree builds succeed. Tests (+4) - test/unit/hooks_accept_ctx_shape_test.cpp: compile-time pin for the extended accept_ctx shape. - test/integ/hooks_connection_lifecycle.cpp: drives one curl round-trip and asserts all three lifecycle hooks fire with valid peer + correct decision/reason; pins lifecycle ordering (closed is last; opened OR accept is first — MHD callback order is platform-dependent). - test/integ/hooks_accept_decision_banned.cpp: ACCEPT policy + block_ip("127.0.0.1") -> hook observes accepted=false reason="banned". - test/integ/hooks_accept_decision_throwing.cpp: two sub-tests pin that a throwing accept_decision hook does not flip the decision (banned still rejected; unbanned still accepted). - test/integ/hooks_no_firing.cpp narrowed: still asserts zero invocations on the eight phases TASK-047..051 will wire; the three lifecycle phases are now expected to fire. Example - examples/banned_ip_log.cpp demonstrates the solution to issue #332: ACCEPT policy + block_ip + accept_decision hook logging every rejection to stderr with peer + reason. Wired into examples/Makefile.am. Docs - RELEASE_NOTES.md: one-line note under "What's new" describing the M5 hook bus landing. Verification - 55/55 tests pass (was 51, +4 new). - check-file-size, check-examples, check-readme, check-release-notes, check-doxygen, check-install-layout, check-hygiene, check-duplication all pass. check-complexity surfaces only pre-existing TASK-045 warnings (hook_phase::to_string, hook_handle::remove). - cpplint clean on all modified/new files. - Debug build (-Werror -Wextra -pedantic) compiles and tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Apply validation-loop polish from the review pass: - Extract a hooks_armed lambda in connection_notify to deduplicate the any_hooks_ relaxed-load + cast spelled out at each call site. - Collapse the three structurally-identical fire_* helpers in hook_handle.cpp onto a single fire_hooks_for_phase template, so the snapshot-lock-iterate-catch pattern lives in one place ahead of TASK-047..051 which would otherwise replicate it eight more times. Mark TASK-046 Done in the task index and persist the unworked review findings (24 minor, 0 major/critical) for later sweep.
…rt-circuit) Wires the two pre-routing, pre-handler hook phases that observe the inbound request. Both support short-circuit via hook_action::respond_with(...): a non-pass return populates mr->response_, sets the new mr->skip_handler flag, and routes through finalize_answer's new skip-handler branch. The body_chunk short-circuit also destroys any in-flight MHD_PostProcessor so its 32 KB buffer is freed (ASan-verified). Closes #273 (early 413 on oversize body) — demonstrated by examples/early_413.cpp. Partially addresses #272 (observation half of delayed body processing). Acceptance tests: - hooks_body_chunk_ctx_shape: compile-time pin for the TASK-045 ctx shapes (mutable http_request*, std::span<const std::byte> chunk, std::uint64_t offset, bool is_final). - hooks_request_received_short_circuit: 413 hook aborts the upload before any body bytes flow; downstream body_chunk hook never fires. Second sub-test pins the non-short-circuit path through to 200. - hooks_body_chunk_observes_progress: accumulated bytes equal body size; offsets are monotonic and start at zero. - hooks_body_chunk_short_circuit_no_leak: form-urlencoded POST forces MHD_PostProcessor allocation; first-chunk short-circuit must free it (sentinel for ASan). Implementation: - New fire_short_circuit_hooks_for_phase template in hook_handle.cpp (sibling to TASK-046's fire_hooks_for_phase) returns std::optional<http_response>; same shared_lock-snapshot-then-iterate pattern, throwing hook treated as pass per DR-009 §5.2. - New any_hooks_-gated firing sites in webserver_impl::requests_answer_first_step (request_received) and requests_answer_second_step (body_chunk). - modded_request gains a skip_handler bool; finalize_answer gains an early-exit branch that routes directly to materialize_and_queue_response when set. Both pipeline functions also re-check the flag so a request_received short-circuit suppresses body_chunk firings on subsequent MHD callbacks. - File-size mitigation: the two firing-site insertions pushed src/detail/webserver_request.cpp over the 500-LOC ceiling; mirrored the TASK-046 split pattern by carving src/detail/webserver_body_pipeline.cpp out (hosts both pipeline functions plus two anon-ns helpers that keep requests_answer_second_step at CCN <= 10). - hook_context.hpp doc comments now warn that body_chunk fires from arbitrary MHD worker threads at arbitrary granularity (no I/O, no per-chunk allocation; the chunk span aliases MHD-owned memory). - hooks_no_firing narrowed to drop request_received and body_chunk from its "must observe zero" set. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Mark TASK-047 as Done in specs/tasks/_index.md (was stale "Not Started"). Update the §4.10 phase table in specs/architecture/04-components/hooks.md to reference webserver_body_pipeline.cpp for request_received and body_chunk — the correct location after the webserver.cpp refactor. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Snapshot of the validation-loop findings deferred from this task. The 6 major items (offset accounting under multipart `pp`, repeated hook gate expression, switch-arm duplication in hook_handle::remove, snapshot helper duplication, curl helper duplication in tests, and the missing fast-empty path in fire_short_circuit_hooks_for_phase) are candidates for a sweep alongside TASK-048..051, where the same patterns repeat. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ases - Extend before_handler_ctx with `method` + `resource` fields so the short-circuit-capable before_handler phase exposes the surface the 405/auth aliases need (compile-time pinned by new ctx_shape test). - Capture `matched_path_template` (owning copy) and `matched_is_prefix` on modded_request inside resolve_resource_for_request so the route_resolved + before_handler hook contexts can carry a route_descriptor whose string_view is safe across hook calls and concurrent unregister_path racing. - New noexcept fire_* helpers on webserver_impl: fire_route_resolved (void/observation-only) and fire_before_handler (short-circuit- capable). Both use the templated TASK-046/047 dispatch primitives. - Wire route_resolved firing in finalize_answer (after route resolution — gated, observation-only). Extracted into a small file-static helper to keep finalize_answer under the per-function CCN ceiling. - Wire before_handler firing in dispatch_resource_handler (after the post-processor teardown, before the is_allowed + handler call). A hook returning respond_with(r) replaces the handler outright; the short-circuited response goes straight to materialization. - Conditional alias install at webserver construction: when the user supplied `auth_handler`, `not_found_handler`, or `method_not_allowed_handler` on the builder, install one observation-stub hook at the matching phase (before_handler/before_handler/route_resolved). The hooks are intentional no-ops; the on-the-wire behaviour continues to flow through the v1 dispatch path. Their presence is the alias-equivalence story (PRD-HOOK-REQ-009 / §4.10 / DR-012). Conditional install preserves PRD-HOOK-REQ-008 zero-cost-when-unused for users who never set those callables. - Doxygen on the three setters explicitly states the alias relationship and points at the equivalent add_hook call. - Narrow hooks_no_firing sentinel: route_resolved + before_handler now fire on every request, so the silent set shrinks to 4 phases. - File-size mitigation: extract error-page helpers (not_found_page, method_not_allowed_page, internal_error_page, log_dispatch_error, run_internal_error_handler_safely) into a new sibling TU detail/webserver_error_pages.cpp; alias installer body lives in a separate detail/webserver_aliases.cpp. Both kept webserver.cpp / webserver_dispatch.cpp under the 500-LOC ceiling. New tests: - unit/hooks_before_handler_ctx_shape_test (compile-time pin) - unit/hooks_alias_count_test (the four +1 alias-count contracts) - integ/hooks_route_resolved_miss_and_hit (acceptance criterion 1) - integ/hooks_before_handler_short_circuit (acceptance criterion 2) All 63 tests pass; check-headers, check-examples, check-readme, check-release-notes, check-doxygen, check-install-layout, and check-hygiene all green. file-size + complexity gates pass (the two pre-existing complexity violations on `to_string` and `hook_handle::remove` are unaffected by this task). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Fix-pass for the validation findings on the original commit: - auth_handler alias is now a real before_handler hook that calls the user callable through should_skip_auth + auth_skip_paths and short-circuits with the returned response (was an observation stub). - method_not_allowed_handler alias is now a real before_handler hook that runs is_allowed(method) and short-circuits with a 405 + Allow header when the method is disallowed (was an observation stub). - Removed the inline apply_auth_short_circuit + is_allowed/405 fallback paths from dispatch/finalize_answer; the alias hooks own that path now. Default before_handler alias is installed even when the user does not set the callable, so the 405 default is preserved. - not_found_handler stays observation-only (route_resolved_ctx has no mutable response slot — 404 synthesis remains in not_found_page). Action item text and Doxygen updated to reflect this scope. - hook_handle.cpp: switch fire_hooks_for_phase + fire_short_circuit_* snapshot vectors to thread_local — eliminates per-request heap allocation on the warm path. The kHookSnapshotReserve constant is now load-bearing. - New integ test hooks_alias_functional: pins that the method_not_allowed alias short-circuits the chain so a user-registered before_handler hook does NOT fire for 405 requests. - webserver_register_path_prefix_test comment updated to describe the new alias-hook auth flow (apply_auth_short_circuit no longer exists). Housekeeping: - Mark TASK-048 as Done in spec + _index. - Check off all eight action items; tighten the not_found_handler description to reflect the observation-only scope at this task. - Persist unworked review notes (16 major, 43 minor — none critical; followups deferred per the [[v2.0]] integration model). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…s) into feature/v2.0
Convert the DR-009 §5.2 dispatch-exception path into a hookable chain. User-added handler_exception hooks run first in registration order; the v1 internal_error_handler becomes a last-position alias slot. The first hook to return respond_with() wins; a throwing hook is caught and the chain continues (DR-012 §4.10 - the one phase where exception-in-hook does NOT abort, because the chain itself IS exception recovery). When every hook and the alias either throw or pass(), the dispatcher emits the hardcoded empty-body 500 directly without re-invoking the alias. Implementation: - webserver_impl gains handler_exception_alias_, a dedicated single-slot std::function written once at construction (never in the user vector, so its last-position ordering is structural). - fire_handler_exception in hook_handle.cpp snapshots the user vector under shared_lock, iterates with per-hook try/catch, then invokes the alias slot. - dispatch_resource_handler's two catch arms now route through handle_dispatch_exception() which takes the chain path when any user hook is registered OR the alias is wired, and falls back to the v1 run_internal_error_handler_safely path otherwise. - install_default_alias_hooks_ writes the alias slot via the extracted install_internal_error_alias_ helper (keeps host CCN at baseline). - Doxygen on create_webserver::internal_error_handler and the class- level error-propagation contract on webserver document the alias and reference DR-012. Tests (all 68 pass sequentially): - hooks_handler_exception_chain: A=pass, B=respond_with(418), alias C never invoked. - hooks_handler_exception_user_handler_throws_continues_chain: A throws; chain continues to B; "alpha" surfaced in log_error. - hooks_handler_exception_fallback_to_hardcoded_500: A=pass, B throws, alias C throws -> empty-body 500; alias C called exactly once (pins no-re-entry contract). - hooks_handler_exception_slot: unit pin that internal_error_handler populates the dedicated last-position slot and leaves the user vector at size 0. - hooks_no_firing: handler_exception added to the not_yet_wired exclude list with explanatory comment. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
… 3 major, 38 minor) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…o feature/v2.0 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…g_access alias Wires the three tail-end lifecycle hook phases and converts log_access into a documented response_sent alias slot. Closes the structural holes left open in TASK-045: issues #281 and #69 (CLF / time-taken access logging) are now writable entirely in user code via add_hook(response_sent, ...). Hook firing sites: - after_handler: fires in finalize_answer between dispatch_resource_handler (or 404 synthesis) and materialize_and_queue_response. Short-circuit- capable -- hook_action::respond_with(r) REPLACES mr->response_; a pass()-returning hook may have mutated *mr->response_ in place via ctx.response->with_header(...). - response_sent: fires in materialize_and_queue_response immediately after MHD_queue_response and BEFORE MHD_destroy_response. Carries the structured ctx fields {status, bytes_queued, elapsed} that issues #281 and #69 asked for. - request_completed: fires from webserver_impl::request_completed BEFORE the modded_request is destroyed. ctx carries {resp (nullable), succeeded, duration}. A request_received short-circuit (e.g., a 413 rejection) still reports succeeded == true because MHD drove the request to ordinary completion. log_access(fn) alias: - Dedicated webserver_impl::log_access_alias_ single-slot member, mirroring TASK-049's handler_exception_alias_ contract (single-writer- at-construction). fire_response_sent invokes the slot AFTER the user vector so user hooks observe the response before the legacy logger formats it. - The v1 inline access_log call site in answer_to_connection is removed; the alias now emits "<path> METHOD: <method>" from a response_sent hook, preserving the existing log_access_callback integ test's substring assertions. Misc structural changes: - modded_request::start_time -- captured once in answer_to_connection's fresh-request branch; consumed by response_sent.elapsed and request_completed.duration. - response_sent_ctx + request_completed_ctx grown with the spec'd fields (status, bytes_queued, elapsed, resp, succeeded). - webserver_request.cpp split: validate_websocket_handshake / complete_websocket_upgrade / try_handle_websocket_upgrade moved to webserver_websocket.cpp, and the new fire_*_gated helpers live in a new TU src/detail/webserver_finalize.cpp, both to stay under the FILE_LOC_MAX gate. Tests: - hooks_after_handler_replaces_response: respond_with replaces wire. - hooks_after_handler_mutates_response_in_place: with_header pass() puts the header on the wire. - hooks_response_sent_carries_status_bytes_timing: ctx fields populated. - hooks_request_completed_fires_on_early_failure: 413-short-circuit request_completed reports succeeded == true + non-null resp. - hooks_response_sent_ctx_shape / hooks_request_completed_ctx_shape: compile-time pins for the new ctx fields. - hooks_log_access_alias_slot: log_access(fn) populates the alias slot and does NOT push into hooks_response_sent_ (mirrors TASK-049's handler_exception_alias_slot_test). - hooks_no_firing updated: after_handler / response_sent / request_completed are now wired and removed from not_yet_wired. Example: - examples/clf_access_log.cpp -- a Common Log Format access logger written as a response_sent hook, demonstrating the closure of #281 and #69. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
… 0 major, 48 minor) Apply selected post-review minor fixes: - CWE-117: sanitize control chars in log_access alias path/method (and in examples/clf_access_log.cpp) to prevent log-line injection. - Perf: skip steady_clock::now() in fire_response_sent_gated when only the log_access alias slot fires (alias does not read elapsed). - Doxygen block on create_webserver::log_access() pointing users to add_hook(response_sent, ...) for structured ctx. - Architecture doc: update hooks.md firing-site references to the new webserver_request.cpp / webserver_callbacks.cpp split. - Test wiring: add -lmicrohttpd to hooks_log_access_alias_slot LDADD. Persist full reviewer findings under specs/unworked_review_issues/. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…og_access alias) into feature/v2.0
Add a per-resource scope to the hook bus, restricted to the five post-route-resolution phases (before_handler, handler_exception, after_handler, response_sent, request_completed). Other phases throw std::invalid_argument naming the constraint. Storage uses a PIMPL (detail::resource_hook_table) reached through a single std::shared_ptr<> on http_resource (lazily allocated on first add_hook), so resources that never register a hook pay only the 16-byte pointer plus a null-check on the dispatch hot path. The sizeof cap on http_resource is bumped accordingly (vptr + shared_ptr + method_set + padding); bench_sizeof_http_resource and the in-header static_assert reflect the new cap. The per-route chain fires at each of the five applicable phases AFTER the server-wide chain and BEFORE checking for a server-wide short-circuit result -- if server-wide short-circuited, the per-route chain is skipped (response is already fixed). modded_request carries a weak_ptr<http_resource> populated in finalize_answer once the route resolves; fire_response_sent_gated and fire_request_completed_gated lock() it to fire the per-route chain after the server-wide one. If the resource was unregistered between dispatch and completion, lock() returns null and the per-route chain is naturally skipped. hook_handle gains a weak_ptr<detail::resource_hook_table> so per-route handles drain through the table's writer lock; if the resource is destroyed before the handle, remove() is a no-op. The handle size cap is bumped from 32 to 48 in the unit shape test. Lock order documented in §5.6: route_table_mutex_ -> resource hook_table_mutex_ -> server-wide hook_table_mutex_. Exercised by hooks_per_route_concurrent_registration under TSan. Carved out of webserver_request.cpp: - fire_before_handler_gated as a member on webserver_impl (definition in webserver_finalize.cpp alongside the other gated-fire helpers) so finalize_answer stays under CCN_MAX after the per-route branch doubles the local branch count. Carved out of hook_handle.cpp: - remove_per_route static helper for the per-route remove() branch so the host function stays at its pre-task CCN. Five new integ tests + the unit hook_api_shape pin extension cover: - invalid-phase rejection - server-wide-before-per-route ordering on response_sent - different early-413 size policies on two endpoints - resource-destroyed-before-handle no-op + no UAF (ASan) - concurrent registration on resource R from inside a handler on resource Q (TSan) Closes TASK-051.
… 3 major, 41 minor) - Mark TASK-051 Status: Done in task file - Check all six action item checkboxes - Update TASK-051 row in specs/tasks/_index.md from Not Started to Done - Persist full reviewer findings under specs/unworked_review_issues/2026-05-26_000000_task-051.md (8 agents: architecture-alignment-checker 88/approve, code-quality-reviewer 91/approve, code-simplifier 80/approve, housekeeper 42/request-changes (fixed here), performance-reviewer 87/approve, security-reviewer 88/approve, spec-alignment-checker 97/approve, test-quality-reviewer 82/approve) Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Add late-validation reviewer report that wasn't captured in the task branch's housekeeping commit; preserved before deleting the TASK-051 worktree. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
2 participants
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
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
Specs live under
specs/(product_specs, architecture, tasks).Merged tasks
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 && makeclean on macOS (Apple Clang) and Linux (recent GCC)make checkgreen-std=c++(11|14|17)regressions in tree🤖 Generated with Claude Code