v2.0: Modernization (M1-M6, 44 tasks)#374
Draft
etr wants to merge 193 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) <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 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) <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>
… NFR, §13 documentation deliverable)
Refresh public-header doxygen blocks to match the v2.0 surface, and add a make-check gate (check-doxygen.sh) that fails on any substantive doxygen warning. The gate is wired into check-local via Makefile.am following the precedent established by TASK-040/041/042. Substantive doxygen-source warnings fixed (17 → 0): * `webserver.hpp` register_path/register_prefix/register_ws_resource: the shared_ptr overloads used `@copydoc <template-overload>` followed by an `@param res` override, which doxygen flags as "too many @param". Replace the @copydoc with a fresh self-contained block on each overload (same prose, no duplicate @param). * `http_request.hpp` check_digest_auth / check_digest_auth_digest: add explicit @param blocks for realm/password/nonce_timeout/max_nc/algo on check_digest_auth, and replace the @copydoc-based block on check_digest_auth_digest with a self-contained one (copydoc was inheriting `password` which is not in the digest variant's signature). * `http_request.hpp` get_or_create_file_info: add missing `@param key`. * `http_resource.hpp` render: name the previously-anonymous `req` parameter so the `@param req` in the doc block resolves. * `http_utils.hpp` get_ip_str: drop the stale `@param maxlen` block left over from the v1 two-arg signature. * `http_utils.hpp` header_comparator::operator() and arg_comparator:: operator(): rename `@param first/second` to match the actual `x, y` signature, and document the case-sensitivity contract on the arg variant. Acceptance-criterion content (per the task action items): * AC bullet 3 — threading: add a `### Threading contract (DR-008 / §5.1)` block on the `webserver` class summarising the 5-point contract (re-entrant registration; stop/destructor must not run on a handler thread; per-request http_request; exclusive http_response; concurrent user callbacks). * AC bullet 4 — error propagation: add a load-bearing block on `create_webserver::internal_error_handler` that spells out the DR-009 §5.2 6-point contract verbatim, cross-linking to @ref webserver, not_found_handler, method_not_allowed_handler, feature_unavailable. Also document `internal_error_handler_t` (the typedef) explaining the std::string_view ownership rules. * AC bullet 5 — feature_unavailable throw sites: enumerate the throw sites on the feature_unavailable class itself (webserver ctor for TLS/BAUTH/DAUTH, register_ws_resource / unregister_ws_resource for HAVE_WEBSOCKET, and every send_* / close on websocket_session). * AC bullet 2 — cross-links: `@see` pairs added between block_ip ↔ unblock_ip, register_path ↔ register_prefix, the unregister_* family, register_ws_resource ↔ unregister_ws_resource, and the three error-handler setters. * Class-level docs added on `create_webserver` (fluent-builder contract, eager validation, explicit webserver ctor), and on `websocket_session` / `websocket_handler` (per-method param/return/ throws blocks including the HAVE_WEBSOCKET-off feature_unavailable behaviour). Verification: * `make check` green (48 testsuite entries pass; check-headers, check-examples, check-readme, check-release-notes, check-doxygen, check-install-layout, check-hygiene all PASS). * `make doxygen-run` now produces zero substantive warnings; the remaining stderr lines (obsolete config tags, dot/graphviz env artifacts when graphviz is missing) are explicitly filtered by scripts/check-doxygen.sh. * Spot-check on 11 v2.0-renamed/reshaped public methods (register_path, register_prefix, internal_error_handler, block_ip, unblock_ip, route, on_get, use_ssl, basic_auth, register_ws_resource, feature_unavailable) — each has a current `///` or `/**` block with @param/@return/@see reflecting the v2.0 signature. The gate skips with exit 0 when doxygen is not installed (mirrors the tolerance pattern used by the other check-* scripts); in CI the gate runs whenever the doxygen package is present. PRD §2 documentation NFR; §13 documentation deliverable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…R, §13 documentation deliverable)
- configure.ac: bump MAJOR/MINOR to 2/0 so AC_INIT yields 2.0.0 and libtool -version-number emits libhttpserver.so.2 / .2.dylib. - ChangeLog: add 'Version 2.0.0' header (keeps validate-version.sh passing for the v2 tag). - scripts/check-soversion.sh + 'make check-soversion': stage a clean DESTDIR install and assert filename/symlink/SONAME/pkg-config/la/.a layout for v2.0. Linux asserts the full .so / .so.2 / .so.2.0.0 chain; Darwin asserts the two-file chain libtool emits on Mach-O with -version-number. - scripts/check-parallel-install.sh + 'make check-parallel-install': build master in an ephemeral git worktree, install both v1 and v2 into the same DESTDIR, and assert the SONAMEd runtime libraries coexist. Best-effort (SKIP on toolchain limitations). - Makefile.am: wire both scripts into EXTRA_DIST and PHONY targets; do not pull into 'make check' to keep per-PR CI fast. Verified locally: 48/48 tests pass, all 6 check-local sub-gates pass, verify-installed-examples builds every example against the v2 install, check-parallel-install confirms libhttpserver.0.dylib (master) and libhttpserver.2.dylib (this branch) coexist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, DR-011) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three CI failures on feature/v2.0 PR #374 run 26183259463: 1. cpplint: examples/hello_world.cpp was missing the copyright line. Added single-line copyright header (the file is the deliberately minimal lambda-form example, so the full LGPL block would defeat its purpose). 2. tsan ws_start_stop: webserver::stop() and is_running() read impl_->running with no lock while start() writes it from the blocking-server thread. Made the field std::atomic<bool> — fixes the genuine race without changing the mutex/cond_var discipline that gates the blocking wait. 3. tsan route_table_concurrency + threadsafety_stress: libstdc++'s std::ctype<char>::narrow lazily fills a 256-byte cache; the guard flag is not atomic so concurrent std::regex compiles inside http_endpoint::http_endpoint look like a race even though every initialiser computes the same bytes. Added test/tsan.supp scoped to that one libstdc++ symbol pair, plumbed via TSAN_OPTIONS only on the tsan matrix lane, and shipped via test/Makefile.am EXTRA_DIST. Libhttpserver-internal races stay fatal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous fix added a Copyright line to examples/hello_world.cpp to satisfy cpplint, but scripts/check-readme.sh enforces that the first ```cpp fence in README.md matches the file byte-for-byte (TASK-041 A1 invariant). Add the same line to the snippet so check-readme passes again. Verified locally with both check-readme.sh and check-examples.sh (still 9 LOC, within the <=10 bound). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These two jobs were red on master→feature/v2.0 before the recent lint/tsan fixes too, but the previous failures were masking them. Now that the green path runs further, they need attention. 1. msys2 MINGW64 (check-readme): the byte-for-byte diff of README's first ```cpp fence vs examples/hello_world.cpp failed because git for Windows checked out README.md with CRLF and the .cpp with LF (autocrlf=true). Strip \r from both sides before diffing — the intent of the gate is content equality, not line-ending equality. 2. msys2 MSYS (threadsafety_stress.cpp): the file unconditionally included <sys/wait.h>/<unistd.h> and used fork()/waitpid() for the opt-in stop_from_handler subtest (DR-008 wedge containment). Guard the POSIX includes and subtest body behind #ifndef _WIN32, and have the subtest print [SKIP] on Windows. The subtest is already opt-in via HTTPSERVER_RUN_STOP_FROM_HANDLER=1; the POSIX CI lanes still exercise it. Windows just builds now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two remaining failures from the previous run: 1. Windows MINGW64 timed out at the 6h ceiling. After my earlier check-readme CRLF fix unblocked that target, the next step on the make check-local pipeline (check-doxygen.sh -> make doxygen-run) wedged on a cmd.exe "Terminate batch job (Y/N)?" prompt produced by the autoconf-archive doxygen recipe under MSYS2/MINGW. The Linux lanes already exhaustively check the doxygen invariant, so drop doxygen+graphviz from the MINGW64 package list; check-doxygen detects the missing tool and self-skips (already designed for it). 2. route_table_concurrency cycles I and J failed under the valgrind memcheck lane with writer_ops=0 — the 500ms wall budget that the test relies on for at least one iteration is too tight when each register_path()/regex compile is running under heavy valgrind instrumentation. Keep the 500ms baseline but, if either counter is still zero, sleep in 100ms chunks up to a 30s ceiling. Normal lanes are unaffected (counters cross zero in well under 500ms); valgrind reliably converges within a couple seconds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires two new gates into the existing `lint` matrix lane and exposes them locally via `make lint-complexity` / `make lint-duplication`: * scripts/check-complexity.sh — wraps `lizard` and fails the build if any function under src/ exceeds CCN_MAX. Initial ceiling 52 (current worst offender + 1 headroom); ratcheted down one step per refactor commit until reaching 10 (the project-wide McCabe convention, matching the artistai max-complexity=10 in lint.mccabe). * scripts/check-duplication.sh — wraps PMD CPD with the C++ tokenizer at --minimum-tokens 100 (PMD's own default). Currently zero hits, so the gate is strict from commit 1. PMD 7.x is fetched from the official release zip in CI (the apt package is pinned to PMD 6.x and predates the modern `pmd cpd` CLI). Lizard installs via pip3, matching the cpplint install pattern. Scope: src/ only. test/ and examples/ legitimately duplicate fixture and demo scaffolding and are not the subject of either gate. The 14 existing CCN-over-10 offenders (largest: webserver::start at CCN 51, webserver_impl::finalize_answer at CCN 46) will be retired incrementally on this branch in follow-up commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both functions had byte-identical bodies aside from the MHD encode function called (MHD_websocket_encode_ping vs _pong, identical signatures). Extract a shared `send_control_frame` static helper parameterised on the encoder via decltype so the function-pointer type matches MHD's `enum MHD_WEBSOCKET_STATUS` return without restating it; a static_assert pins the ping/pong signature invariant in case MHD ever diverges them. Caught by `make lint-duplication` at CPD_MIN_TOKENS=50 (7 lines / 59 tokens). Below the day-1 gate at 100 tokens, but worth fixing since the dedup is trivial and the original was a verbatim copy. Build is unchanged on platforms without HAVE_WEBSOCKET (whole block is still under the existing #ifdef). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
webserver::start() was a 217-line monolith building an MHD option
array, composing a daemon-flag bitmask, and invoking the daemon. CCN
51 -- by far the worst offender in the codebase.
The MHD option-array build and the daemon-flag composition both live
on detail::webserver_impl now (the PIMPL already friends webserver
and includes <microhttpd.h>), split along logical groupings:
build_mhd_option_array -- orchestrator, appends sub-groups + END
add_base_mhd_options callbacks, thread/connection/memory limits
add_tls_mhd_options HTTPS cert + DAUTH random
add_gnutls_mhd_options GNUTLS cred type, PSK, SNI
add_extended_mhd_options backlog, reuse, increment, fastopen,
sigpipe, ALPN, discipline
add_https_extra_options dhparams, key password, priorities append
compose_start_flags -- orchestrator
compose_transport_flags SSL/IPv6/dual-stack
compose_runtime_flags debug/pedantic/deferred/turbo/...
start() now reads as: pre-condition -> build options -> compose
flags -> MHD_start_daemon -> handle blocking. CCN 8.
The local `gen` struct-with-operator() is replaced by a file-scope
`make_option` helper in namespace detail so every builder pushes
options uniformly.
Behavioural notes:
* Reordering: add_https_extra_options runs after add_extended_mhd_options.
In v1 order the HTTPS-extra trio (dhparams/key_password/priorities_append)
was interleaved with the extended options. MHD treats MHD_OPTION_ARRAY as
a set keyed by option tag, so the relative ordering among independent
value-setters is not observable.
* The THREAD_PER_CONNECTION precondition stays in start() so it remains
visible at the entry point of the public API.
* MHD_OPTION_END is appended by build_mhd_option_array and the
bind_address branch still passes a trailing MHD_OPTION_SOCK_ADDR.
Verified locally: full `make check` (48/48 pass) plus all check-local
invariants (headers, hygiene, install-layout, examples, readme,
release-notes, doxygen).
scripts/check-complexity.sh CCN_MAX ratcheted 52 -> 47 (the new
worst offender is webserver_impl::finalize_answer at CCN 46).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
finalize_answer was a 255-line monolith doing WebSocket upgrade
detection, v1 resource lookup with regex+LRU cache, centralized
auth, handler dispatch (with try/catch + method-not-allowed), and
response materialisation/queue all inline. CCN 46.
Decomposed into per-responsibility helpers on detail::webserver_impl
(which already has the parent back-pointer and includes the MHD
headers behind the PIMPL barrier):
try_handle_websocket_upgrade probe + delegate. optional<MHD_Result>.
validate_websocket_handshake RFC 6455 header validation.
complete_websocket_upgrade handler lookup, accept header, queue.
resolve_resource_for_request single-resource / exact / regex+cache.
lookup_route_cache LRU front-promote + match copy.
scan_regex_routes longest-match-wins linear scan.
store_route_cache LRU front-insert + eviction.
apply_extracted_params url_pars/chunks -> request args.
apply_auth_short_circuit centralized auth handler short-circuit.
dispatch_resource_handler try/catch around handler invocation
serialize_allow_methods method_set -> "GET, HEAD, ..." (TASK-021).
materialize_and_queue_response final stage with fallback path.
finalize_answer is now a 6-call orchestrator at CCN 6. Every new
helper sits at CCN <= 8.
Locking discipline preserved:
* resolve_resource_for_request takes the shared registered_resources
lock for the entire lookup phase and releases at scope exit, exactly
as the original {…} block did.
* lookup_route_cache / store_route_cache each take route_cache_mutex
internally (nested under registered_resources, matching the original
lock-order documented at the head of route_cache_v2).
* TASK-035: complete_websocket_upgrade takes the shared_ptr copy
under the shared lock before unlocking, preserving the
handler-alive-across-upgrade invariant.
Subtle reformulation in scan_regex_routes: the original guarded the
inner match() with `!found || pieces_len > len || (pieces_len == len
&& tot_len > tot_len)`. I inverted it to a `continue` for clarity
(skip when found AND not strictly longer in either dimension). The
boolean is identical.
Verified locally: full `make check` (all 48 unit tests + invariants).
scripts/check-complexity.sh CCN_MAX ratcheted 47 -> 35 (the new worst
offender is ip_representation::ip_representation at CCN 34).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-> 2)
ip_representation::ip_representation(const std::string&) was a
112-line monolith that parsed both IPv4 ("a.b.c.d") and IPv6
("...::1:2:3:4" with optional nested IPv4) addresses. CCN 34, dwarfed
only by webserver::start and finalize_answer which already landed.
Split into private member helpers on ip_representation:
parse_ipv4(ip) v4 path; CCN 5.
parse_ipv6(ip) v6 dispatch + per-part loop; CCN 3.
compute_ipv6_omitted_segments(parts)
handle "::" prefix and validate the
empty-count invariant; static; CCN 9.
apply_ipv6_part(parts, i, y, omitted)
decode one v6 part (wildcard / placeholder /
short hex / 4-char hex / nested v4 sentinel);
CCN 7.
parse_nested_ipv4(parts, i, y)
decode an embedded dotted-quad at the tail
of a v4-mapped v6 address; CCN 10.
The "(a != 0 && a != 255) || (b != 0 && b != 255)" prefix invariant
moves into a tiny anonymous-namespace helper ipv4_mapped_prefix_invalid
in http_utils.cpp so parse_nested_ipv4 stays at the bar.
The constructor itself drops to CCN 2: zero pieces[]+mask, dispatch on
':' to parse_ipv6 or parse_ipv4. All thrown messages and the
behavioural shape (when '*' wildcards mask off, when "::" expands)
preserved verbatim.
Verified locally: full `make check` (all 48 unit tests + invariants).
scripts/check-complexity.sh CCN_MAX ratcheted 35 -> 29 (the new worst
offender is webserver::on_methods_ at CCN 28).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
webserver::on_methods_ was a 171-line registration entry point doing
input validation, v1-map find-or-create-shim, atomicity pre-check,
slot installation, v1 insertion across three maps, and v2 3-tier
table mirror (radix vs exact vs regex_, with fresh-insert vs methods-
merge sub-paths). CCN 28.
Decomposed into private member helpers on detail::webserver_impl:
prepare_or_create_lambda_shim v1 find-or-create + atomicity pre-check
commit_handlers_to_shim install the handler into every slot
insert_fresh_v1_entries v1 maps insertion (str + regex tiers)
upsert_v2_table_entry v2 3-tier orchestrator
upsert_v2_param_route radix tier (read-merge-reinsert)
insert_fresh_v2_entry exact / regex_ first-insert
update_existing_v2_entry merge methods into existing entry
on_methods_ on webserver now reads as: validate args, build endpoint,
acquire registered_resources_mutex once for the v1 work, release it,
do the v2 work under route_table_mutex_ internally, invalidate cache.
CCN 7. Every new helper sits at CCN <= 7.
Also dedupes the pre-check loop (line 539) vs commit loop (line 558)
duplication flagged by `make lint-duplication` at low minimum-tokens
sensitivity: extracts a file-scope `for_each_requested_method` template
helper that iterates http_method::count_ in enum order (TASK-021's
Allow-header serialization order) and invokes the callback only for
methods set in the requested mask. Both former loops collapse to two-
line callback invocations.
Locking discipline preserved: registered_resources_mutex (unique_lock)
held across the v1 helpers, dropped at end of inner scope; route_table_mutex_
held inside upsert_v2_table_entry only. Lock order is registered_resources
-> route_table -> route_cache, identical to the original.
Verified locally: full `make check`.
scripts/check-complexity.sh CCN_MAX ratcheted 29 -> 23 (the new worst
offender is http_endpoint::http_endpoint at CCN 22).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
http_endpoint::http_endpoint(const string&, bool, bool, bool) was a
75-line registration-time URL parser doing case-insensitive lowercase,
slash normalization, per-piece dispatch between non-registration /
literal / parameter modes, and optional regex compilation. CCN 22.
Decomposed into private member helpers:
normalize_url_complete trim trailing '/', ensure leading '/'
process_url_part per-iteration mode dispatch
append_non_registration_part verbatim push to url_pieces
append_literal_url_part literal piece, respect '^' anchor
append_parameter_url_part "{name}" / "{name|regex}" parse
compile_regex_url trailing '$' + std::regex compile
The ctor itself drops to CCN 7: throw-if-regex-without-registration,
seed url_normalized, lowercase if CASE_INSENSITIVE, normalize slashes,
tokenize, loop over parts calling process_url_part, compile_regex if
requested. Every new helper sits at CCN <= 7.
Behaviour preserved verbatim: the leading-'^' anchor on the first
literal piece still REPLACES url_normalized's seed (rather than
appending), the parameter-mode validation still throws on parts of
size <3 or missing braces, and compile_regex_url still throws via
the same std::regex::extended | icase | nosubs path.
Verified locally: full `make check`.
scripts/check-complexity.sh CCN_MAX ratcheted 23 -> 22 (the new worst
offender is webserver_impl::post_iterator at CCN 21).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
webserver_impl::post_iterator is the MHD post-iterator trampoline:
called repeatedly per request as MHD feeds form fields and file
chunks. The original 82-line body wove together the non-file form-arg
path and the file-upload path (including memory and/or disk targets,
random vs sanitized filename, stream open/close on key/filename
transitions, write + size update). CCN 21.
Decomposed into per-responsibility helpers on detail::webserver_impl:
handle_post_form_arg static; form-arg path (set vs grow on off).
setup_new_upload_file_info per-instance; first-chunk filename choice
+ content_type/transfer_encoding seed.
Returns false if sanitize_upload_filename
fails so the caller maps it to MHD_NO.
manage_upload_stream static; close-on-transition, open-if-needed.
process_file_upload per-instance orchestrator for the disk
branch. Returns MHD_YES / MHD_NO.
post_iterator is now: dispatch (no filename -> handle_post_form_arg);
try-block with memory-target arg-flat update; disk-target dispatch to
process_file_upload; catch generateFilenameException -> MHD_NO. CCN 7.
Every new helper sits at CCN <= 8.
Static helpers are static because they don't read parent state
(handle_post_form_arg, manage_upload_stream); the disk-orchestrator
and setup_new_upload_file_info are instance methods because they
read parent->{file_upload_target, generate_random_filename_on_upload,
file_upload_dir}.
Behaviour preserved verbatim: the four-way OR that detects (filename,
key) transitions; the per-chunk grow vs replace based on @p off; the
unlink before opening to avoid appending to a leftover file; the
short-circuit returns of MHD_NO on sanitize failure and write failure.
Verified locally: full `make check`.
scripts/check-complexity.sh CCN_MAX ratcheted 22 -> 19 (the new worst
offenders are ip_representation::operator< and populate_all_cert_fields,
both CCN 18).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ields
Two unrelated functions, both at CCN 18; refactor bundled because the
shape of each extraction is small.
ip_representation::operator< (CCN 18 -> 7):
* accumulate_octet_score: shared "(16-i)*piece[i]" accumulator used
by the main 0..15 sweep (skipping 10/11) and again for the 10..11
tail. Pulls the two CHECK_BIT clauses out of three different sites.
* is_v4_mapped_prefix_octet_pair: collapses the nested
"((a == 0x00 || a == 0xFF) && (b == 0x00 || b == 0xFF))" check
into a named predicate. The composite if at the top of operator<
was contributing 8 boolean ops alone.
http_request_impl::populate_all_cert_fields (CCN 18 -> 5):
* extract_x509_string: parameterised two-pass GnuTLS string getter
(function pointer takes gnutls_x509_crt_get_dn or
gnutls_x509_crt_get_issuer_dn -- same signature, identical wrapping).
* extract_x509_common_name: get_dn_by_oid wrapper (separate because
of the extra OID/index/flags parameters).
* extract_x509_fingerprint_sha256: fingerprint hex-encode.
* verify_peer_certificate: wraps gnutls_certificate_verify_peers2 and
returns the verified bool.
populate_all_cert_fields now reads as a flat sequence: assign to each
pmr::string member via the per-field helper, then the two int64_t
validity times. The cross-allocator .assign(ptr, len) idiom is preserved.
Both refactors are HAVE_GNUTLS-gated in the case of cert handling;
local build skips it but the helpers still compile-test against the
operator< side of the change, and CI's GNUTLS-on lane exercises the
cert path end to end.
scripts/check-complexity.sh CCN_MAX ratcheted 19 -> 15 (new worst
offender is webserver::register_impl_ at CCN 14).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t_buffer / answer_to_connection) Three independent CCN-over-10 functions, bundled because each extraction is small. webserver::register_impl_ (CCN 14 -> 10): Extracted detail::webserver_impl::register_v2_route: a one-shot insert into the v2 3-tier route table with method_set::set_all() and no merge. Distinct from upsert_v2_table_entry (the on_*/route path with method-set merging). register_impl_ retains the v1 maps work and the input validation under registered_resources_mutex. decode_websocket_buffer (CCN 13 -> 4): Extracted dispatch_websocket_frame (the switch on the decode result) and handle_close_frame (RFC 6455 §5.5.1 close-payload parsing). decode_websocket_buffer is now just the recv-and-feed loop: MHD_websocket_decode + dispatch + break-on-zero-progress. webserver_impl::answer_to_connection (CCN 13 -> 4): Extracted resolve_method_callback: maps the wire-string HTTP method to mr->callback (pointer-to-member dispatch), mr->method_enum (is_allowed input), and mr->has_body (body-buffering branch). The 9-way strcmp chain stays in its own static method at CCN 10 (the bar), where it belongs as a single responsibility. Verified locally: full `make check`. scripts/check-complexity.sh CCN_MAX ratcheted 15 -> 13 (the new worst offender is webserver_impl::lookup_v2 caller-side wired into radix_tree::find at CCN 12). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes out the v2.0 cyclomatic-complexity sweep by retiring the last
three CCN-over-10 functions and lowering CCN_MAX to its long-term
target (10, matching artistai's [lint.mccabe] max-complexity = 10).
radix_tree::find (header-only template, CCN 12 -> 7):
Extracted match_root_terminus -- the exact-first-then-prefix scan on
the root node for empty-segment paths ("/"). The descent loop is
unchanged; only the leading early-return ladder is pulled out.
http_method::to_string (CCN 11 -> 2):
Replaced the 10-arm switch with a constexpr std::array<string_view>
indexed by the underlying enum value. The array size is tied to
http_method::count_, so a future enum addition that forgets to
extend the table fails compilation rather than silently returning
an empty view. The constexpr noexcept contract is preserved.
normalize_path (file-scope static, CCN 11 -> 7):
Extracted apply_normalized_segment -- per-segment dispatch ("" / "."
skip / ".." pop / push). normalize_path is now the tokenize-and-
rebuild loop without inline segment logic.
scripts/check-complexity.sh CCN_MAX 13 -> 10. The header comment is
updated to reflect that the bar is now stable: new offenders must be
brought below 10 at the same commit they are introduced; lifting
CCN_MAX is not allowed.
Final state summary (v2.0 branch):
* 14 v1 offenders -> 0 (largest was webserver::start at CCN 51,
finalize_answer at 46, ip_representation ctor at 34).
* New helpers across the sweep: 40+ small functions, each <= 10 CCN.
* No new public API surface added; every helper lives on
detail::webserver_impl, ip_representation private section,
http_request_impl private section, or in anonymous-namespace
file-scope statics. Public headers are unchanged from a consumer
standpoint.
* Duplication gate (PMD CPD --minimum-tokens 100) was clean from
commit 1 and remains so.
Verified locally: full `make check` (passes both gates + 48 unit tests
+ check-headers / hygiene / install-layout / examples / readme /
release-notes / doxygen).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds 20 unit tests across three existing files, driven by the test-quality-reviewer findings recorded in specs/unworked_review_issues/2026-05-21_121150_manual-validation.md. The helpers themselves were extracted in the v2.0 CCN-10 sweep with verbatim behaviour preservation; these tests pin the contracts before the v2.0 -> master merge. test/unit/http_utils_test.cpp (+10): sanitize_upload_filename — Unix path strip, Windows backslash strip, empty string, bare "." and "..", "..-suffix" strip, "."-suffix strip, trailing slash, mixed-separator basename. Pins the disk-write gate used by process_file_upload / setup_new_upload_file_info. test/unit/webserver_register_path_prefix_test.cpp (+5): normalize_path via the observable should_skip_auth effect — exact match served, ".." pop, "." elision, off-skip path blocked with 401, excess ".." clamped to root. Pins apply_normalized_segment indirectly through its only caller. test/unit/webserver_on_methods_test.cpp (+3 +2): serialize_allow_methods enum-declaration order — Allow header is emitted in GET/HEAD/POST/PUT/DELETE/CONNECT/OPTIONS/TRACE/PATCH order regardless of registration order (TASK-021 contract). upsert_v2_param_route — composition (GET+POST on the same parameterized path both served, args bound) and atomicity (failed duplicate GET leaves the original handler intact). specs/unworked_review_issues/2026-05-21_121150_manual-validation.md: Record of the validation-loop output (8 agents, 2 iterations). Lists the 12 majors + 44 minors that remained advisory after the test fixes landed — for follow-up sweeps; not blockers for the v2.0 PR. Verified locally: full `make check` (68 unit tests, all green). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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]; |
Planning-only commit. No code yet; subsequent task-branch PRs implement TASK-045..052 in order against feature/v2.0. Adds a multi-subscriber lifecycle hook system to v2.0, replacing v1's patchwork of single-slot callbacks (log_access, not_found_handler, method_not_allowed_handler, internal_error_handler, auth_handler) with one uniform webserver::add_hook(phase, callable) surface plus a per-route http_resource::add_hook(...) variant. Existing v1 setters survive as documented aliases (PRD-HOOK-REQ-009). Eleven phases spanning the connection -> request -> routing -> handler -> response -> cleanup lifecycle: connection_opened, accept_decision, request_received, body_chunk, route_resolved, before_handler, handler_exception, after_handler, response_sent, request_completed, connection_closed. Short-circuit allowed at four pre-handler phases (request_received, body_chunk, before_handler, handler_exception) and at the after_handler post-handler phase. Throwing hooks route through DR-9 §5.2. Closes (once TASK-046, 047, 050 land): #332 banned-IP log entry (accept_decision hook) #281 response-aware access log (response_sent context) #69 Common Log Format w/ time-taken (response_sent context) #273 early 413 on oversize body (request_received short-circuit) Partially addresses #272 (body_chunk observation; the buffer-steal half remains a v2.1 candidate needing a streaming-body API). Files added: specs/architecture/11-decisions/DR-012.md specs/architecture/04-components/hooks.md (§4.10) specs/tasks/M5-routing-lifecycle/TASK-045.md .. TASK-052.md Files updated: specs/product_specs.md - new §3.8 with PRD-HOOK-REQ-001..009 - §4 traceability line for API-HOOK specs/architecture/05-cross-cutting.md - new §5.6 hook lifecycle contract - four new public headers added to §5.5 header tree specs/tasks/_index.md - M5 milestone row updated - 8 task-status rows (045..052) - dependency-graph branch - PRD-HOOK coverage rows - DR-012 coverage row Per-route hooks (TASK-051) are restricted to phases that fire after route resolution. v1 alias retention is covered in TASK-048 (404/405/auth), TASK-049 (internal_error_handler), TASK-050 (log_access), and re-documented in TASK-052. TASK-052 explicitly touches back into the already-Done TASK-040 (examples), TASK-041 (README), TASK-042 (RELEASE_NOTES), TASK-043 (Doxygen) — the planned M6 touch-back called out when this scope was approved for inclusion in PR #374. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d_hook)
Lands the public types and per-phase storage for the lifecycle hook bus
per §4.10 / DR-012 / PRD-HOOK-REQ-001..002. No phase fires yet; phases
start firing in TASK-046..051. After this task the API surface compiles
and a hook can be registered + removed.
Public surface (four new MHD-clean headers):
src/httpserver/hook_phase.hpp -- enum class hook_phase + count_ (11)
src/httpserver/hook_action.hpp -- pass / respond_with / take_response &&
src/httpserver/hook_handle.hpp -- move-only RAII receipt
src/httpserver/hook_context.hpp -- 11 per-phase ctx structs + peer_address
+ route_descriptor
webserver::add_hook -- 11 overloads, one per phase, distinguished by the
std::function signature; each validates the runtime phase tag, allocates
a fresh slot_id (monotonic uint64), takes hook_table_mutex_ unique_lock,
pushes into the matching per-phase vector, flips any_hooks_[phase] true.
hook_handle::remove() / dtor re-takes the lock, linear-scans for slot_id,
erases, and clears the gate if the vector is now empty.
Storage lives on webserver_impl: shared_mutex hook_table_mutex_, atomic
uint64 next_slot_id_, std::array<atomic<bool>, count_> any_hooks_, and
11 individually-typed std::vector<phase_entry<Sig>> members (signatures
differ per phase). hook_handle is ABI-pinned <= 32 B via static_assert.
Tests (3 new entries, total now 51):
test/unit/header_hygiene_hooks_test.cpp -- per-header preprocessor
sentinel that the four new hook headers don't transitively pull
<microhttpd.h>/<gnutls/gnutls.h>/<sys/socket.h>/<sys/uio.h>.
test/unit/hook_api_shape_test.cpp -- compile-time SFINAE gates
(move-only, signature mismatch rejected) + runtime add/remove,
double-remove no-op, RAII destruction, detach() disarm, any_hooks_
gate flip semantics.
test/integ/hooks_no_firing.cpp -- registers one hook on every phase,
drives one full HTTP round-trip, asserts all 11 counters stay zero.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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.
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