v2.0: Modernization (M1-M6, 44 tasks)#374
Draft
etr wants to merge 456 commits into
Draft
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #374 +/- ##
==========================================
+ Coverage 68.03% 68.70% +0.67%
==========================================
Files 34 63 +29
Lines 1730 4030 +2300
Branches 697 1480 +783
==========================================
+ Hits 1177 2769 +1592
- Misses 80 357 +277
- Partials 473 904 +431
... and 50 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
etr
added a commit
that referenced
this pull request
May 7, 2026
…rueFalse, exclude specs/ Codacy was reporting 2018 new issues on the v2.0 PR (#374). Resolve as follows: * Add .codacy.yaml excluding specs/** — the product spec, architecture notes, task records, and review notes are internal groundwork artifacts, not user-facing docs, and should not be subject to README markdownlint rules. Removes 2003 markdownlint findings. * src/webserver.cpp:499 — drop the redundant `blocking &&` from the wait loop condition. `blocking` is a function parameter never reassigned inside the loop body, so the conjunct was tautological (cppcheck knownConditionTrueFalse). * src/webserver.cpp:946 — replace the C-style `(struct detail::modded_request*)` cast on the MHD `cls` void* with `static_cast<detail::modded_request*>` (cppcheck cstyleCast). Mirrors the existing static_cast usage elsewhere in the file. * detail/webserver_impl.hpp, detail/http_request_impl.hpp, iovec_entry.hpp — add `// cppcheck-suppress-file unusedStructMember` with a one-line rationale comment. Every flagged member is in fact heavily used from the corresponding .cpp translation unit (registered_resources*, route_cache_*, bans, allowances, files_, path_pieces_public_, iovec_entry::base/len, etc.); cppcheck analyses each TU in isolation and cannot see those uses, so the warning is a known pimpl/POD false positive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
etr
added a commit
that referenced
this pull request
May 7, 2026
… clash Two unrelated CI regressions on PR #374, both falling out of TASK-020: 1. Lint job (gcc-14, ubuntu): cpplint flagged src/http_utils.cpp:30 with build/include_order, because the matching public header ("httpserver/http_utils.hpp") came AFTER a non-matching project header ("httpserver/constants.hpp"), and <microhttpd.h> (a C system header in cpplint's view) followed both. cpplint's expected order is: matching header, C system, C++ system, other. Reorder so the matching header comes first and the project headers ("constants.hpp" / "string_utilities.hpp") move to the bottom of the include block. 2. Windows MSYS2 build: src/httpserver/http_utils.hpp failed with error: expected identifier before numeric constant at the line `ERROR = 0,` inside the digest_auth_result enum. <wingdi.h> (pulled in via <windows.h> via <winsock2.h> via <microhttpd.h> on MinGW) unconditionally `#define`s ERROR to 0, and the preprocessor expands macros inside scoped-enum bodies just like anywhere else. Pre-TASK-020 the enum was inside `#ifdef HAVE_DAUTH`, so MSYS2 builds without digest auth never compiled it; PRD-FLG-REQ-001 then made the enum unconditional and exposed the latent collision. v2.0 is unreleased, so renaming is safe: ERROR -> GENERIC_ERROR (matches MHD_DAUTH_ERROR's "general error" docs). Static-assert pin in src/http_utils.cpp updated to match. Verified locally: - python3 -m cpplint on both touched files: exit 0. - `make check` on macOS: 32/32 PASS, all check-hygiene / check-headers gates PASS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
etr
added a commit
that referenced
this pull request
May 11, 2026
Codacy's "26 new issues (0 max.)" gate was failing on PR #374. Two classes of finding, addressed at root: - 21 markdownlint findings on test/REGRESSION.md (MD013 line-length, MD040 fenced-code language, MD043 heading structure). REGRESSION.md is an internal test-gate document (the v2.0 routing parity gate), conceptually peer to the already-excluded specs/ artifacts and not in the user-facing README/ChangeLog/CONTRIBUTING category. Extend .codacy.yaml exclude_paths with `test/**/*.md`. - 5 cppcheck findings that are all single-TU false positives: * iovec_entry.hpp: `cppcheck-suppress-file unusedStructMember` was not at the top of the file (preprocessorErrorDirective), so the file-level suppression was ignored and `base`/`len` were both flagged unused. Replaced with per-member inline suppressions. * route_cache.hpp: `cache_value::captured_params` is read in src/webserver.cpp at the cache-hit replay site; cppcheck does not follow the cross-TU read. Inline-suppress. * header_hygiene_test.cpp: cppcheck statically assumes none of the forbidden-header guard macros are defined and reports `leaks > 0` as always-false; the comparison is load-bearing at runtime under any actual leak. Inline-suppress. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
etr
added a commit
that referenced
this pull request
May 20, 2026
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>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
etr
added a commit
that referenced
this pull request
May 21, 2026
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>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
… 3 major, 38 minor) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…o feature/v2.0 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
… 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) <noreply@anthropic.com>
…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 <noreply@anthropic.com>
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) <noreply@anthropic.com>
Closes out the v2.0 hook bus by: - examples/per_route_auth.cpp (new): per-route HTTP Basic auth via http_resource::add_hook(before_handler, ...), end-to-end smoke verified (200 OK on /public, 401 on /private without creds, 200 with valid creds). Resolves the per-route demo bullet in the hook-examples set. - README.md + examples/README.md: new "Lifecycle hooks" H2 section on the top-level README listing all eleven phases, short-circuit semantics, per-route restrictions, v1-alias equivalence table; new "Lifecycle hooks" topical section in examples/README.md pointing at the four canonical hook demos. - RELEASE_NOTES.md: replaces the partial M5-skeleton "Lifecycle hook bus" bullet with the complete eleven-phase summary; adds closes-list for #332, #281, #69, #273 (and #272 partial). - Doxygen on the four public hook headers (hook_phase, hook_action, hook_handle, hook_context): @file blocks, @brief on every public symbol; extends the existing webserver::add_hook block to enumerate all eleven phases and drops the stale "skeleton-only at TASK-045" note; extends http_resource::add_hook block to name the five permitted phases AND the six rejected ones with the throw contract; adds hook-concurrency sentence to webserver class-level threading contract. - test/bench_hook_overhead.cpp (new): microbench asserting the per-phase any_hooks_ atomic-load gate stays within a 50ns ceiling for the zero-hooks-registered configuration. The gate-load shape (rather than a full HTTP round-trip) is the only signal attributable to the hook bus alone; round-trip noise would swamp it. Wired into `make bench` via EXTRA_PROGRAMS. Measured 0.3-0.6 ns/call locally. - test/integ/threadsafety_stress.cpp: extends the TASK-032 stress driver from 4 ops to 6 (adds add_hook on a random phase + remove via hook_handle from the bag). Bag capped at 256 entries to bound RSS. Asserts a new LT_CHECK_GT on hook_add_ok + hook_remove_ok. Drains the bag explicitly before ws.stop() to keep lifetimes obvious. The TSan CI matrix entry picks the binary up automatically (no workflow edit). - scripts/check-examples.sh: extended with TASK-052 hook-example presence check (all four examples listed in both README files). - scripts/check-readme.sh: extended REQUIRED_V2_TOKENS with add_hook / hook_phase / hook_action / hook_handle, added "lifecycle hooks" to REQUIRED_SECTIONS, added A4b alias-callout block-content assertion. - scripts/check-release-notes.sh: extended REQUIRED_V2_TOKENS with add_hook / hook_phase / hook_handle; added REQUIRED_CLOSES_ISSUES for #332/#281/#69/#273/#272; added "eleven phases" gate to distinguish the complete bullet from the M5-skeleton wording. - scripts/check-hooks-doc-spotcheck.sh (new): grep-based regression gate for H1-H6 (file-level @file blocks, @brief on principal symbols, eleven-phase enumeration on webserver::add_hook, five-permitted/six-rejected on http_resource::add_hook, alias callouts on the five v1 setters, hook-concurrency in webserver threading block). Required because doxyconfig.in's EXTRACT_ALL=YES suppresses Doxygen's "undocumented" warnings. - Makefile.am: wires check-hooks-doc-spotcheck into check-local and EXTRA_DIST. Verification: full `make check` (80 tests + all 5 doc-check scripts) green; `make bench` (3 bench targets) green; `make doxygen-run` produces zero substantive warnings; HTTPSERVER_STRESS_SECONDS=5 threadsafety_stress includes hook ops and ticks both new counters. Related: PRD-HOOK-REQ-002, PRD-HOOK-REQ-007, PRD-HOOK-REQ-008, PRD-HOOK-REQ-009; DR-012; §4.10; §5.6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… 0 major, 21 minor) - Mark TASK-052 status Done and check off all 13 action items in specs/tasks/M5-routing-lifecycle/TASK-052.md - Update TASK-052 row in specs/tasks/_index.md from Not Started to Done - Persist 21 unworked minor findings to specs/unworked_review_issues/2026-05-27_000000_task-052.md - Apply iter1 fixer fixes to bench_hook_overhead.cpp (OUTER=11→51, clarify gate comment, capture min/max before sort) and threadsafety_stress.cpp (safe recycle via move-then-remove) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add late-validation reviewer report that wasn't captured in the task branch's housekeeping commit; preserved before deleting the TASK-052 worktree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Critical #1 (perf): radix_tree::find() now iterates path inline using std::string_view instead of calling tokenize() which allocated a std::vector<std::string> on every parameterized-route lookup. Critical #2 (perf): route_cache::find_by_view() added — takes (http_method, std::string_view) without constructing a cache_key, eliminating the std::string copy on every warm-cache hit. lookup_v2 now uses find_by_view on the hot path and only constructs cache_key on the miss path before insert. Critical #3 (missing test): test added in lookup_pipeline_test.cpp asserting that captured_params from lookup_v2() have the correct (name, value) shape for binding to http_request::get_arg(), pinning the param-binding API for the Cycle K dispatch cutover. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolves 30 of 34 minor findings from the 2026-05-27_010619 review pass. The 1 critical finding was a false positive (files exist since TASK-050). 3 minor findings are deferred (tooling timestamp, bench measurement order, warmup count change). Changes: - README.md: reorder Phases table to canonical firing order (connection_closed moved from row 3 to last row) - RELEASE_NOTES.md: reorder hook-bus phase list to canonical order - examples/per_route_auth.cpp: fail closed on null request (CWE-636); add TRANSPORT NOTE (CWE-523) and CONSTANT-TIME NOTE (CWE-208) - src/httpserver/hook_context.hpp: consolidate mixed // and /** */ doc styles for accept_ctx, body_chunk_ctx, before_handler_ctx, response_sent_ctx, and request_completed_ctx into Doxygen blocks - test/bench_hook_overhead.cpp: rename median_of_sorted to sort_and_median (makes mutation explicit); add precondition comment to p99_of_sorted; add warmup rationale comment; document absolute-ceiling gating strategy - test/integ/threadsafety_stress.cpp: fix HookBag comment; collapse noop_resource return; add phase-count tie comment; add O(n) note on erase; case 5 now uses move+pop_back+remove pattern (mirrors case 4); split combined sum assertions into individual per-counter checks - specs/unworked_review_issues/2026-05-27_010619_task-052.md: mark resolved items [x] with clarifying notes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Validation loop captured 7 minor findings across 5 reviewers (code-quality x2, code-simplifier x1, spec-alignment x1, housekeeper x1, plus 2 others) that were not actioned during this task. All blockers (1 critical, 5 majors from housekeeper) were fixed in 72a2ed3. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add the 5th action item and two acceptance criteria that were captured during planning but never made it into the task's spec file. The work itself shipped in 06ad399 (mechanical cleanup sweep): the orphaned TASK-024 head was removed from webserver_setup.cpp above block_ip, and the full prologue was installed above webserver::unregister_impl_ in webserver_register.cpp — verified by grep guards (no copies in setup/webserver, one canonical copy in register at line 247). Status remains Done; this is a spec/docs sync only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…omments, stale doc refs
Clear the low-risk mechanical leftovers flagged in specs/tasks/v2-branch-gap-audit.md
"Proposed disposition" — five concrete fixes shipped as a single sweep:
- Finished the truncated TASK-029 block comment in webserver_register.cpp
with its missing closing clause ("…they are no longer reachable from
the public API.").
- Installed the full TASK-024 prologue (six lines, ending "…dangling
resource pointer after the maps drop their refs.") immediately above
webserver::unregister_impl_ — the function it actually describes —
and removed the orphaned five-line head that was sitting above the
unrelated block_ip in webserver_setup.cpp (split-artifact from the
7-way webserver.cpp split).
- Removed the two orphan comment fragments at webserver.cpp:503-504.
- Replaced the stale "Currently in XFAIL_TESTS" claim at
test/Makefile.am:67-74 with a status-correct note (header_hygiene
was removed when TASK-020 landed).
- Dropped the stale "RELEASE_NOTES.md created by TASK-042, not yet
present" case arm at scripts/check-readme.sh:273 — TASK-042 shipped.
Vendored test/littletest.hpp left untouched per planning decision.
Followed by housekeeping (action items + Status flipped to Done in the
spec), validation-loop persistence of 7 unworked minor review findings,
and a final spec sync that recorded the TASK-024 relocation action item
+ acceptance criteria that were captured during planning but never
landed in the spec file.
Verification (per 06ad399): make check 87/87 PASS, check-file-size PASS,
check-readme.sh returns 0, all five pre-edit failing grep guards now
report zero matches, each reflowed sentence fragment has exactly one
canonical copy under src/.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds `http_response::unauthorized(digest_challenge)` factory and a
dispatch-time switch on `body_kind::digest_challenge` that routes
through libmicrohttpd's `MHD_queue_auth_required_response3`, so the
authoritative `WWW-Authenticate: Digest ...` challenge with
nonce/opaque/algorithm/qop is written into the wire response. The
legacy `unauthorized("Digest", realm, body)` overload remains
source-compatible; its Doxygen now points new code at the new overload.
Key pieces:
* `digest_challenge` struct (`src/httpserver/http_response.hpp`,
HAVE_DAUTH-gated) — public RFC-7616 challenge-parameter container.
* `body_kind::digest_challenge` enumerator + `detail::digest_challenge_body`
subclass (`src/httpserver/body_kind.hpp`, `src/httpserver/detail/body.hpp`,
`src/detail/body.cpp`) — heap-allocated params inside a unique_ptr to
keep the body's inline footprint under the 64-byte SBO budget.
* `webserver_impl::queue_response_dispatching_kind` (`src/detail/webserver_request.cpp`) —
branches on `body_kind::digest_challenge` and calls
`MHD_queue_auth_required_response3` with the user-supplied params;
every other body kind goes through the standard `MHD_queue_response`
path.
* Per-`webserver_impl` opaque (`digest_opaque_`) seeded once at
construction from `std::random_device`, substituted when the
factory leaves the opaque field empty (RFC 7616 §5.10: opaque is an
identifier, not a secret).
* Validation: realm/opaque/domain/body fields are rejected with
`std::invalid_argument` on CR/LF/NUL (CWE-113 header injection).
DR-013 ("Digest auth simplified to static WWW-Authenticate challenge")
is marked Superseded by TASK-062 — the value-type/DR-005 constraint is
preserved by doing the kind-switch at dispatch time (the response
itself stays a movable value); only the dispatch path knows about
MHD_Connection. http-response.md updated to drop the
"non-RFC-compliant" sentence and point at the new overload.
Tests:
* New unit test `http_response_digest_factory_test.cpp` (12 tests):
status/kind/SBO/algorithm/opaque/domain round-trip + CR/LF/NUL
injection guard on each text field.
* New integ test `digest_challenge_format_test.cpp`:
spins up a webserver wired to digest_auth_random + nonce_nc_size,
hits with plain curl (no --digest), asserts the WWW-Authenticate
header carries every RFC 7616 §3.3-mandated token.
* `digest_resource` in `test/integ/authentication.cpp` rewritten to
emit the new `digest_challenge` body; `digest_auth` test flipped
from expecting 401/FAIL to 200/SUCCESS (AC1).
Acceptance criteria:
1. curl --digest negotiates: digest_auth test passes 200/SUCCESS.
2. New integ test pins WWW-Authenticate format against RFC 7616 §3.3.
3. Six placeholder integ tests now drive real nonce/opaque handshake
end-to-end (wrong-password arms still resolve to 401/FAIL but
through MHD_digest_auth_check3 validation).
4. libmicrohttpd's MD5/SHA-256 helpers remain the underlying primitive
(we delegate to MHD's nonce HMAC machinery; no crypto here).
5. Typecheck and lint gates pass (check-complexity, check-file-size,
check-warning-suppressions, check-duplication, check-hygiene).
6. Tests pass (digest_challenge_format and http_response_digest_factory
in isolation; the pre-existing `basic` cascade flake on
feature/v2.0 is unchanged).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test/integ/authentication.cpp: migrate digest_ha1_md5_resource and
digest_ha1_sha256_resource to emit digest_challenge{} (with
algorithm=MD5 / SHA256) so curl --digest can complete the handshake;
HA1 round-trip tests now assert 200 SUCCESS instead of the static
401 FAIL. Refresh the file-top tracking note and per-test comments
to reflect the new state; flag the still-uncovered NONCE_STALE
re-challenge branch as a follow-up (needs real-time nonce expiry).
* test/integ/digest_challenge_format_test.cpp: add wire-format tests
for the SHA-256 algorithm token and for the opaque/domain fields.
* test/unit/http_response_digest_factory_test.cpp: drop assertions
that reached into detail::digest_challenge_body via the friend hook
(algorithm/opaque/domain/body-size); pin only body_kind discrimination
at the unit level. Wire-format coverage of those fields lives in the
integ test above, per the test-quality-reviewer recommendation
(iter1-3).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Captures the test-quality / code-quality / spec-alignment review pass on the digest factory + handshake work. Note: major finding #1 (HA1 resources still on the legacy static-challenge overload) is resolved by the previous commit; the remaining 44 findings stay open for follow-up tasks per the milestone audit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Brings in the new `unauthorized(digest_challenge{})` overload that
routes through `MHD_queue_auth_required_response3`, drives the real
MHD nonce/opaque state machine, and lets `curl --digest` complete the
full RFC-7616 handshake. Also: wire-format integ tests (algorithm
token, opaque, domain), HA1 resource migration, factory unit tests
pinned to body_kind discrimination only (no internal coupling).
Removes the publicly exposed `size_hint` parameter from `http_response::pipe(int fd, std::size_t = 0)`. The parameter was reserved-for-future-use boilerplate — the dispatch path discarded it (`(void)size_hint;`) and a unit test pinned that "accepted-but-ignored" shape as the contract. An accepted-but-ignored API arg teaches callers a lie, and the M7 v2-cleanup milestone is the cutover to fix it. Why not honor it instead: - No PRD-RSP-REQ-* requires it (PRD §3.5 lists the v2 response factories and `pipe` carries no second arg). - libmicrohttpd's `MHD_create_response_from_pipe` takes no size. - Synthesising a `Content-Length` from a hint would lie when the pipe yields a different byte count. - See `.groundwork-plans/TASK-063-plan.md` for the full rationale. What changes: - `src/httpserver/http_response.hpp`: drop the `size_hint` parameter and its doc paragraph from the `pipe` declaration. - `src/http_response.cpp`: drop the parameter from the definition and the `(void)size_hint;` suppressor. - `test/unit/http_response_factories_test.cpp`: replace the `pipe_factory_size_hint_is_accepted_but_ignored` runtime pin with a `static_assert` on `decltype(&http_response::pipe) == http_response (*)(int)`. Any future re-introduction of a second parameter fails to compile with a TASK-063 message. - `RELEASE_NOTES.md`: bullet under `## What changed semantically` documenting the signature change. SOVERSION already bumps for v2.0, no further ABI bump needed. Existing callers (examples/pipe_response_example.cpp, test/integ/new_response_types.cpp, test/unit/http_response_factories_test.cpp) already pass only the `fd`; no call-site edits required. Verification: - `http_response_factories`: 28 tests / 56 checks PASS (in isolation). - `body`: 17/17 PASS (in isolation). - `new_response_types` integ test (which exercises the pipe path): 3/3 PASS in isolation. The batch `make check` failures are a pre-existing port-collision issue on this host, unchanged by this patch. - `check-readme.sh` / `check-release-notes.sh` / `check-examples.sh`: all OK after RELEASE_NOTES edit. - `cpplint` on changed files: clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the v1 string-blob `http_response::with_cookie(name, value)`
surface with a structured `httpserver::cookie` value type carrying
`name`, `value`, `domain`, `path`, `expires`, `max_age`, `secure`,
`http_only`, and `same_site` attributes. Each cookie renders to a single
RFC 6265 §4.1-compliant `Set-Cookie` header on the wire via
`cookie::to_set_cookie_header()`. A paired RFC 6265 §5.4 parser exposes
`cookie::parse_cookie_header` so request `Cookie:` headers round-trip
through the same code path.
Why now:
- The follow-up was explicitly deferred at the v1 `with_cookie(string,
string)` site (`src/httpserver/http_response.hpp`).
- The v1 path renders the value verbatim into `Set-Cookie`, so a `;` in
the value silently injects attributes (attribute-injection / CWE-113).
v2 enforces the RFC ABNF via setter validation and a single render
source.
- PRD-RSP-REQ-004 (fluent return) and PRD §2 API minimalism call for
typed values over string blobs on the response surface.
What's new (public surface):
- `src/httpserver/cookie.hpp` (installed, umbrella-included): value type
with fluent `&` / `&&` setter pairs mirroring `http_response`'s style;
`enum class same_site_mode { unset, strict, lax, none }`;
`to_set_cookie_header()` renderer; static `parse_cookie_header()`
parser. Setters reject CR/LF/NUL / `;` / `=` / whitespace per RFC 6265
§4.1.1 ABNF.
- `http_response::with_cookie(cookie)` lvalue + rvalue overloads;
`http_response::get_cookies_parsed()` returning
`const std::vector<cookie>&`.
- `http_request::get_cookies_parsed()` returning
`const std::vector<cookie>&`; lazily built from the request's
`Cookie:` header via `parse_cookie_header`, cached on the per-request
impl following the TASK-016/017 arena pattern.
Wire rendering (the headline correctness fix):
- `decorate_mhd_response` now reads from
`resp.get_cookies_parsed()` and emits one `Set-Cookie` header per
entry via `cookie::to_set_cookie_header()`. Attributes (Domain, Path,
Expires, Max-Age, Secure, HttpOnly, SameSite) propagate to the wire.
- The legacy `cookies_` name->value mirror map survives only to back the
deprecated `get_cookie(name)` / `get_cookies()` accessors; it is no
longer a render source.
Deprecation / migration (transitional, one release):
- `http_response::with_cookie(string, string)` and the legacy
`get_cookies()` / `get_cookie(string_view)` accessors are
`[[deprecated]]`. The legacy `with_cookie` forwards through the
structured path so wire output is unchanged for unmodified callers.
- v2.1 removes the deprecated string-blob path entirely. The deprecation
message points callers at the structured overload.
Tests (TDD, all passing in isolation):
- `cookie_header_sentinel_test.cpp`: class shape, default state, enum,
copy/move-constructibility.
- `cookie_render_test.cpp`: 43 tests / 82 checks covering fluent-setter
ref-qualifier shape, setter validation (CWE-113), full RFC 6265 §4.1
rendering (attribute ordering, Max-Age zero/negative,
IMF-fixdate using the canonical §4.1 example 784111777,
SameSite=None auto-Secure coercion, byte-transparency, empty-name
throw), §5.4 parsing (DQUOTE stripping, malformed-skip, case-preserving,
no percent-decode), and the AC round-trip pin.
- `http_response_cookie_wire_test.cpp`: `with_cookie(cookie)` integration
on http_response, get_cookies_parsed shape + lifetime, structured
cookies survive move ctor, legacy/structured interop, no double-emit
when mixing both paths.
- `http_request_cookies_parsed_test.cpp`: shape, lifetime, cache
stability across calls and across unrelated getters.
- `cookie_deprecation_sentinel_test.cpp`: legacy path still compiles
(under `#pragma GCC diagnostic ignored "-Wdeprecated-declarations"`),
legacy return types unchanged.
- Existing `http_response_test.cpp`, `http_response_sbo_test.cpp`,
`http_response_move_sanitizer_test.cpp` wrap their legacy
`with_cookie(string, string)` calls in a deprecation-warning
suppression pragma; runtime behaviour unchanged.
Architecture docs updated under `specs/architecture/04-components/`
(http-response.md, http-request.md). RELEASE_NOTES.md updated with the
new public surface, the deprecation timeline, and the wire-render shift.
Acceptance:
- `http_response::string("...").with_cookie(cookie{}.with_name("sid")
.with_secure(true).with_same_site(same_site_mode::strict))` compiles
and renders to `sid=; Secure; SameSite=Strict` on the wire.
- `http_request::get_cookies_parsed()` returns
`const std::vector<cookie>&`; second call O(1) and zero-allocating.
- RFC 6265 §4.1 canonical example round-trips byte-exact.
- Deprecated string-blob path still compiles with a `[[deprecated]]`
warning; `request_with_cookie` integ test still passes end-to-end.
Verification:
- All 5 new cookie test programs PASS (sentinel + render + wire + parsed
+ deprecation): 73 tests / 138 checks total.
- Unit-test suite for http_response/http_request all PASS in isolation.
- Clean build (no compile warnings).
- `make check` integ-test FAILures on this host are pre-existing CURL
error-7 (couldn't-connect) flake from port exhaustion on rapid
sequential daemon-start tests — unrelated to TASK-064; the cookie
integration test `request_with_cookie` in basic.cpp passes.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rd to validate_attr_param Two behavioral fixes driven by TDD: 1. Partial-mutation bug (code-simplifier-iter1-2): do_set_cookie now builds and validates the structured cookie first (with_name / with_value throw on invalid chars), then mirrors into the legacy cookies_ map only on success. Previously, insert_or_assign ran before with_name, leaving a dirty entry in the legacy map when with_name threw. Test: legacy_with_cookie_bad_key_leaves_maps_clean. 2. Attribute-injection guard (security-reviewer-iter1-1): validate_attr_param (used by with_domain and with_path) now rejects semicolons in addition to CR/LF/NUL. A semicolon in Domain or Path would be emitted verbatim by the renderer, injecting synthetic attributes into the Set-Cookie header (CWE-113). Tests: with_path_rejects_semicolon, with_domain_rejects_semicolon. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ounts, update specs
1. Delete dead cookie_token_split struct and split_cookie_token helper
(code-simplifier-iter1-1): these 27 lines were never called; the
inline logic in parse_cookie_header was written independently.
2. Replace explicit byte-count literals in append calls with single-
argument form (code-simplifier-iter1-3): out.append("; Secure", 8)
→ out.append("; Secure") etc. across append_time_attributes,
append_target_attributes, and append_flag_attributes. The compiler
derives the length at compile time; no runtime cost change.
3. Mark all five TASK-064 action items complete in TASK-064.md
(housekeeper-iter1-1).
4. Add §3.5.1 Structured Cookie Type (API-CKY) to product_specs.md
with 10 EARS requirements covering the cookie value type, injection
guards, RFC 6265 rendering, parse semantics, deprecated shim policy,
and acceptance criteria (housekeeper-iter1-2).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
security-reviewer-iter2-1: add a render-time validation loop in to_set_cookie_header() that scans name_ for bytes forbidden by is_invalid_name_byte and throws std::invalid_argument. This closes the injection window for naively reflected cookies created by parse_cookie_header(), which deliberately bypasses all validators. Also fix the misleading comment in do_set_cookie_struct (previously claimed 'the cookie value was validated at its own setter sites', which is false for parse_cookie_header output). security-reviewer-iter2-2: extend validate_attr_param to reject commas in addition to semicolons. HTTP/1.0 legacy parsers and some CDN edge nodes split Set-Cookie on commas, so Domain=evil.com,other.com would appear as two directives to such parsers. Add tests: - with_domain_rejects_comma - with_path_rejects_comma - render_throws_on_name_with_low_byte_bypassed_by_parser - render_throws_when_parsed_name_contains_semicolon Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Security (security-reviewer-iter3-1, CWE-113):
- Unify name_ render-guard in to_set_cookie_header() with validate_name()
helper by adding a ctx parameter (code-simplifier-iter3-1).
- Add parallel render-time guard for value_ matching the name_ guard:
to_set_cookie_header() now rejects CR, LF, NUL, and ';' in value_,
closing the asymmetry where parse_cookie_header() stores value_ raw
and a naive reflect would silently emit a header-injection payload.
Tests (test-quality-reviewer-iter3-1..3):
- Remove always-green render_throws_when_parsed_name_contains_semicolon
(parser always splits on ';', so no cookie name ever contains one —
the test body never executed and the assertion was vacuously true).
- Rename render_throws_on_name_with_low_byte_bypassed_by_parser to
render_guard_rejects_space_in_parsed_name; add explicit precondition
asserts pinning the parser semantics the test depends on (iter3-2).
- Add render_guard_rejects_{cr,lf,nul,semicolon}_in_parsed_value tests
exercising the new value_ render-guard (iter3-1).
- Add same_site_none_with_explicit_secure_emits_single_secure_token
to pin the no-double-emit invariant when secure(true) and
same_site(none) are both set (iter3-3).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…gaps test-quality-reviewer-iter3-4: Add comment to second_call_does_not_reallocate explaining that the cookies_parsed_cache_ guard via the create_test_request() builder path is tested, but the live MHD_get_connection_values path is only covered by integration tests. test-quality-reviewer-iter3-5: Expand comment in cookie_deprecation_sentinel_test recording that the CMake try_compile() negative-compile sentinel (confirming [[deprecated]] is emitted with -Werror) is a pending follow-up. Points to the cookie_header_sentinel_test.cpp pattern as the available infrastructure. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Record the remaining review-issues snapshot (0 critical, 11 major, 58 minor) from the 2026-06-05 12:29 reviewer pass so the gaps remain visible for future follow-up tasks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the string-blob cookie surface with a structured `http::cookie` type per RFC 6265, including render-time guards on name/value, parser hardening, and accompanying tests. Marks TASK-064 Completed and records remaining review findings under specs/unworked_review_issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the uncompressed 8-group "%x:%x:..." IPv6 rendering in
src/peer_address.cpp with an in-TU RFC 5952 §4 canonicalizer:
- lowercase hex digits with leading-zero suppression (§4.1, §4.2.1),
- longest run of >= 2 consecutive zero groups collapsed to "::"
(§4.2.2), with first-occurrence tie-break on equal-length runs
(§4.2.3), and single zero groups left intact (§4.3),
- ::ffff:0:0/96 IPv4-mapped form rendered as "::ffff:a.b.c.d" (§5).
Per action item #2 we always post-process the 16-byte address in-TU
rather than gating on inet_ntop behaviour. The reason is twofold:
(a) peer_address.cpp deliberately stays free of <netinet/in.h> /
<sys/socket.h> (see file-header comment); (b) post-processing yields
deterministic, identical output across glibc / musl / macOS / Windows
builds, which the platform-dependent path cannot guarantee for the
IPv4-mapped dotted-quad form.
IPv4 and unspec branches are byte-for-byte unchanged. Removes the
"TASK-046 can refine when telemetry firms up" comment.
Adds test/unit/peer_address_to_string_test.cpp pinning the §4.2.2
examples (2001:db8::1, ::1, ::), the §4.3 single-zero non-collapse
rule, the §4.2.3 longest-run and first-occurrence tie-break cases,
the §5 IPv4-mapped dotted-quad form (::ffff:192.0.2.1), the IPv4
regression case (127.0.0.1), the unspec empty-string case, and a
trailing-edge collapse (2001:db8:1::).
Implementation is decomposed into small helpers (assemble_groups,
is_ipv4_mapped, format_ipv4_mapped, find_longest_zero_run,
append_group_hex, emit_canonical) to keep every function under
CCN_MAX=10 per check-complexity.sh.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses the major code-quality finding from the review cycle:
append_group_hex was left as a free function after the in-TU
canonicalizer landed but was no longer called from elsewhere, so
-Wunused-function would have warned.
Inline its three-line body directly into emit_canonical, which now
writes into a 40-byte stack scratch buffer and returns
std::string(buf, len). For the typical compressed addresses
("::1", "2001:db8::1") this keeps the returned string inside SBO
on every common libstdc++/libc++ build — the previous reserve(40)
defeated SBO unconditionally.
Spec housekeeping: tick the four TASK-065 action items and flip
both TASK-065.md and the M7 _index.md status to Done.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 major + 24 minor surfaced by the post-merge review pass on the RFC 5952 canonicalizer. The major (dead append_group_hex helper) is already addressed by the inline-into-emit_canonical commit on this branch; the remaining 24 minors stay parked here per the same convention as TASK-064 and earlier. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…on-time-only Option (b) of the task spec: rather than add a runtime alias setter (no demand signal, two-mechanisms-behind-one-façade asymmetry, DR-012 already frames aliases as construction-time sugar), remove the speculative "if a future task adds a runtime setter..." comments and pin the immutable-after-construction contract with tests and documentation. - src/hook_handle.cpp: trim three forward-debt comment blocks (in erase_slot_for_handler_phase_, fire_handler_exception tail, fire_response_sent tail) to one-line "immutable after start() (DR-012 §4.10)" rationale. - src/detail/webserver_aliases.cpp: trim the install_internal_error_alias call-site comment to the immutable-after-construction one-liner. - src/httpserver/detail/webserver_impl.hpp: trim the lifetime-contract comments on handler_exception_alias_ and log_access_alias_ to drop the speculative "future runtime setter" paragraphs; keep the single-writer-at-construction rationale. - src/httpserver/hook_handle.hpp: add an "Alias slots are construction-time-only" docstring paragraph naming the five v1 aliases and pointing users to add_hook() for runtime extension. - specs/architecture/04-components/hooks.md: add an "Alias mutability" paragraph to §4.10 explicitly stating that v1 aliases are construction-time-only and that the hook bus is the runtime extension surface (per DR-012). - test/unit/hooks_log_access_alias_slot_test.cpp: replace the misleading log_access_second_registration_replaces_first test (the smell TASK-085 finding 3 flagged) with two real contract pins: log_access_alias_is_immutable_after_construction and handler_exception_alias_is_immutable_after_construction. Both verify that user add_hook() at the relevant phase grows the user vector but never displaces the alias slot, and that hook_handle::remove() leaves the alias slot intact. Acceptance: - grep -nE 'if a future task adds a runtime setter for the alias slots' src/ returns no matches. - Broader audit grep 'future task adds a runtime|future runtime setter|runtime re-registration path' src/ also returns no matches. - hooks_log_access_alias_slot now: 7 tests, 29 checks, 0 failures. - All hook-related tests pass; cpplint clean on changed files. - specs/tasks/M7-v2-cleanup/TASK-066.md updated to Done with resolution notes and TASK-085 handoff. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…truction-time-only
Three transitional v1 surfaces are gone, leaving the v2 3-tier route
table as the only registration / dispatch oracle:
1. webserver_impl::{registered_resources, registered_resources_str,
registered_resources_regex, registered_resources_mutex} — registration-
time bookkeeping replaced by direct writes to exact_routes_, the radix
tree (param_and_prefix_routes_), and regex_routes_. Duplicate detection
migrates from the v1 ordered map's .insert() oracle to a new
reject_duplicate_v2_entry_ helper that probes the correct v2 tier for
a pre-existing entry before any mutation — preserving the
"throw-leaves-table-prior-state" atomicity contract pinned by
basic_suite::duplicate_endpoints.
2. namespace httpserver::compat (auth_handler_v1_ptr +
adapt_legacy_auth), the [[deprecated]] create_webserver::auth_handler
overload accepting it, and the paired push/pop
-Wdeprecated-declarations suppression at the forwarding call site —
all removed. The auth_handler_legacy_shim_test TU goes away; a new
no_v1_compat_shim_test TU pins the post-removal sentinel via a
std::void_t SFINAE detection idiom (negative compile-time check that
the v1 shared_ptr-returning shape is no longer accepted).
3. Readers of the v1 maps — prepare_or_create_lambda_shim's conflict
detection, the resolve_single_resource_ short-circuit in dispatch,
and the WebSocket-path's use of registered_resources_mutex to guard
registered_ws_handlers — all rewired. WebSocket registration/dispatch
now uses a dedicated ws_handlers_mutex_ instead of piggy-backing on
the deleted v1 mutex. The single_resource fast path is folded into
lookup_v2 (a single registered prefix terminus at "/" surfaces as a
trivial radix descent).
TASK-070 (the partner -Wdeprecated-declarations suppression in
http_resource.cpp:81-115 that the action-items list points to) is
explicitly handed off — its own M-sized scope (atomic shared_ptr
migration) would balloon this PR.
Drive-by fixes surfaced by the fresh integ-test rebuild this commit
forces:
- test/integ/basic.cpp:155,367 — wrap two legacy
with_cookie(string,string) call sites in -Wdeprecated-declarations
push/pop. Pre-existing TASK-064 deprecation that incremental builds
silently skipped (basic.o was stale from before TASK-064).
- test/integ/basic.cpp:498 — duplicate_endpoints's "ok" vs "OK"
case-folding assertion was pinning a v1 quirk (registered_resources
used http_endpoint::operator<, unconditionally case-insensitive via
std::toupper regardless of CASE_INSENSITIVE). The v2 route table is
consistently case-sensitive without -DCASE_INSENSITIVE, so the
assertion is now CASE_INSENSITIVE-gated to match dispatch-time
lookup semantics.
Acceptance criteria (all satisfied):
- grep -nE 'registered_resources' src/httpserver/detail/webserver_impl.hpp
returns no matches.
- grep -nE 'namespace compat' src/httpserver/create_webserver.hpp returns
no matches.
- grep -nE 'Wdeprecated-declarations' src/httpserver/create_webserver.hpp
returns no matches.
- make -j1 check passes 95/95 tests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the std::memset call in connection_state::reset_arena() with a
new httpserver::detail::secure_zero helper that the optimizer is
forbidden to elide as a dead store. The helper dispatches to
explicit_bzero (glibc/musl/BSD), RtlSecureZeroMemory (Windows), or a
portable volatile-pointer loop + asm("" ::: "memory") clobber fallback
(macOS and any other lane without explicit_bzero). The full 8 KiB
initial_buffer_ is scrubbed unconditionally, closing the residual-
credential window across keep-alive requests for credentials that fit
within the arena (overflow blocks remain an accepted residue, documented
in the rewritten comment block).
The DR-008 sanity gate (a DEADBEEFCRED sentinel carried as a basic-auth
username on request 1 must not be observable to request 2 on the same
MHD connection) is pinned by a new integ test that reaches the
underlying MHD_Connection through a new HTTPSERVER_COMPILATION-gated
http_request::underlying_connection_for_testing() accessor and probes
connection_state::initial_buffer_ directly. A second integ-signal hook
(connection_opened) asserts curl keep-alive engaged exactly once across
both requests.
Two additional unit tests guard the helper itself: secure_zero_dce_test
is compiled at -O2 -DNDEBUG and reads every byte through a volatile
sink to verify the writes are not dead-store eliminated; and
connection_state_sentinel_test pins that reset_arena() zeros the entire
arena, not just the bump-pointer prefix.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adjust the body-residue integ test to force HTTP/1.1 (HTTP/2 stream multiplexing would invalidate the keep-alive precondition) and gate the headline assertion behind LT_ASSERT_EQ on connection_opened_count so a fresh connection cannot produce a vacuously-passing result. Switch the secure_zero DCE test's buffer to `volatile unsigned char[]` so the optimizer cannot propagate the pre-zeroing sentinel directly into the post-zeroing read (closes a real DCE loophole the prior sink-only approach left open). Extend the nullptr-with-zero-size test with an adjacent-sentinel guard so an accidental out-of-bounds write is caught. Flip TASK-068 in M7 _index.md to Done, and persist the 35 unworked review findings (0 critical, 2 major, 33 minor) from the final review cycle for future follow-up tasks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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