v2.0: Modernization (M1-M6, 44 tasks)#374
Draft
etr wants to merge 450 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% 67.55% -0.48%
==========================================
Files 34 60 +26
Lines 1730 3785 +2055
Branches 697 1415 +718
==========================================
+ Hits 1177 2557 +1380
- Misses 80 342 +262
- Partials 473 886 +413
... and 46 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>
Final step of the ratchet. The 2673-line webserver.cpp is decomposed
along the section markers already in the file into seven new TUs, each
focused on a single concern and well below the 500-line target. The
residual webserver.cpp keeps ctors / dtors / signal helpers / hooks
machinery and lands at 464 lines.
src/detail/webserver_setup.cpp 437 MHD option array
builders (add_base /
tls / gnutls / extended /
https_extra) + start-flag
composers, daemon lifecycle
(start, is_running, stop,
run, run_wait, get_fdset,
get_timeout, add_connection),
and block_ip / unblock_ip.
src/detail/webserver_register.cpp 360 register_path /
register_prefix /
register_resource (incl.
the shared register_impl_
funnel and detail::
register_v2_route mirror)
+ unregister_impl_ /
unregister_path /
unregister_prefix /
unregister_resource. Uses
the shared route_tier
helper.
src/detail/webserver_routes.cpp 411 on_methods_ funnel + the
seven on_* shortcuts +
both route() overloads +
the detail-namespace
lambda_shim helpers
(prepare_or_create_lambda_shim,
commit_handlers_to_shim,
insert_fresh_v1_entries,
upsert_v2_table_entry +
its sub-helpers). Uses
the shared route_tier.
src/detail/webserver_callbacks.cpp 477 MHD trampolines registered
with libmicrohttpd
(request_completed,
connection_notify, policy_
callback, error_log,
access_log, uri_log,
unescaper_func, PSK/SNI
cred handlers) + the
post_iterator family
(handle_post_form_arg,
setup_new_upload_file_info,
manage_upload_stream,
process_file_upload, the
post_iterator trampoline).
src/detail/webserver_websocket.cpp 227 HAVE_WEBSOCKET-gated TU.
decode_websocket_buffer
static helper +
upgrade_handler MHD
callback + the anonymous-
namespace handshake
helpers.
src/detail/webserver_dispatch.cpp 460 Dispatch support services:
not_found_page /
method_not_allowed_page /
internal_error_page /
log_dispatch_error /
run_internal_error_handler_safely
+ invalidate_route_cache +
lookup_v2 (the v2 3-tier
walk) + the route-table
helpers
(lookup_route_cache,
scan_regex_routes,
store_route_cache,
apply_extracted_params,
resolve_resource_for_request,
apply_auth_short_circuit,
dispatch_resource_handler).
src/detail/webserver_request.cpp 488 Request lifecycle:
should_skip_auth + its
normalize_path helper +
requests_answer_first_step /
requests_answer_second_step,
materialize_response /
decorate_mhd_response /
get_raw_response_with_fallback,
the websocket-upgrade
dispatch helpers
(validate_websocket_handshake,
complete_websocket_upgrade,
try_handle_websocket_upgrade),
materialize_and_queue_response,
finalize_answer,
complete_request,
resolve_method_callback,
and the answer_to_connection
entrypoint.
src/webserver.cpp 464 Residual: license / includes
/ signal helpers
(catcher, ignore_sigpipe)
/ webserver_impl ctor +
dtor / webserver ctor +
dtor + features() +
stop_and_wait() / the
TASK-045 hook bus
(register_hook_impl
anonymous-namespace helper
+ make_hook_handle_ +
the eleven add_hook
overloads).
One small companion header was extracted to share state across TUs:
src/httpserver/detail/route_tier.hpp Hoists the route_tier_kind
enum + route_tier_result
struct + classify_route_tier
from an anonymous namespace
in webserver.cpp into a
detail header. Both
webserver_register.cpp and
webserver_routes.cpp call
classify_route_tier; an
anonymous-namespace
definition no longer
suffices once the TU is
split. Marked inline so
the ODR holds across
translation units.
normalize_path (and its apply_normalized_segment helper) — formerly
file-scope statics in webserver.cpp — co-locate with their only caller
(webserver_impl::should_skip_auth) in webserver_request.cpp.
FILE_LOC_MAX drops from 2700 to 500, the long-term project target.
The header comment in scripts/check-file-size.sh records the seven
ratchet steps that drove it down.
Verification:
make check ALL PASS (includes hygiene,
install-layout, doxygen, examples,
readme, release-notes)
./scripts/check-file-size.sh PASS at FILE_LOC_MAX=500
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring README.md back to a section-by-section reference walkthrough (25 H2 sections, 52 H3 subsections, ~1820 lines) covering the current API surface: full create_webserver builder reference, http_resource virtuals, the three routing families (on_* / route() / register_path & register_prefix), full http_request and http_response references, authentication (Basic / Digest / centralized / mTLS / SNI / TLS-PSK), WebSocket, daemon introspection and external event loops, threading contract (DR-008 / §5.1), error propagation (DR-009 / §5.2), and feature availability. Examples are linked rather than inlined: the grouped index in the README mirrors examples/README.md. examples/hello_world.cpp: drop the PRD §3.4 callout from the file header so the comment stays self-contained; the README block remains byte-for-byte synced via scripts/check-readme.sh. scripts/check-readme.sh and scripts/check-examples.sh both pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the three connection-level lifecycle phases of the hook bus into the existing MHD callback sites. Closes long-standing feature request #332 (banned-IP log entry). Production code - accept_ctx extended from {peer} to {peer, accepted, reason}; `reason` is a std::optional<std::string_view> pointing at a string literal ("banned" / "not-allowed") with static storage duration. - Three new noexcept fire_* helpers on webserver_impl (declared in the dispatch sibling header, defined in src/hook_handle.cpp): each takes a shared_lock, snapshots the phase vector with reserve(8), releases the lock, then iterates with try/catch routed through log_dispatch_error (DR-009 §5.2). Mirrors TASK-027's route-cache promotion pattern. - connection_notify + policy_callback split out of webserver_callbacks.cpp into a new webserver_callbacks_lifecycle.cpp TU. The original would have overshot FILE_LOC_MAX after the firing- site code landed. webserver_callbacks.cpp shrinks to 432 lines. - MHD_OPTION_NOTIFY_CONNECTION closure pointer switched from nullptr to the owning webserver* so connection_notify can reach impl_->any_hooks_ / fire_connection_opened / fire_connection_closed. - policy_callback gains decision-derivation logic (accept_ctx.reason); extracted into anon-ns classify_decision() helper to stay under the CCN gate. - All three firing sites are gated by a relaxed atomic load on any_hooks_[phase] so the zero-hook path stays one branch + one atomic load (PRD-HOOK-REQ-008). - accept_decision's throwing-hook semantics are a structural guarantee: fire_accept_decision returns void and `decision` is captured in a local before the fire call. Pre-existing build fix - src/detail/webserver_dispatch.cpp was missing `using std::map` and `using httpserver::http::http_utils` directives (left out of the TASK-15f8083 7-way split). Added so fresh worktree builds succeed. Tests (+4) - test/unit/hooks_accept_ctx_shape_test.cpp: compile-time pin for the extended accept_ctx shape. - test/integ/hooks_connection_lifecycle.cpp: drives one curl round-trip and asserts all three lifecycle hooks fire with valid peer + correct decision/reason; pins lifecycle ordering (closed is last; opened OR accept is first — MHD callback order is platform-dependent). - test/integ/hooks_accept_decision_banned.cpp: ACCEPT policy + block_ip("127.0.0.1") -> hook observes accepted=false reason="banned". - test/integ/hooks_accept_decision_throwing.cpp: two sub-tests pin that a throwing accept_decision hook does not flip the decision (banned still rejected; unbanned still accepted). - test/integ/hooks_no_firing.cpp narrowed: still asserts zero invocations on the eight phases TASK-047..051 will wire; the three lifecycle phases are now expected to fire. Example - examples/banned_ip_log.cpp demonstrates the solution to issue #332: ACCEPT policy + block_ip + accept_decision hook logging every rejection to stderr with peer + reason. Wired into examples/Makefile.am. Docs - RELEASE_NOTES.md: one-line note under "What's new" describing the M5 hook bus landing. Verification - 55/55 tests pass (was 51, +4 new). - check-file-size, check-examples, check-readme, check-release-notes, check-doxygen, check-install-layout, check-hygiene, check-duplication all pass. check-complexity surfaces only pre-existing TASK-045 warnings (hook_phase::to_string, hook_handle::remove). - cpplint clean on all modified/new files. - Debug build (-Werror -Wextra -pedantic) compiles and tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply validation-loop polish from the review pass: - Extract a hooks_armed lambda in connection_notify to deduplicate the any_hooks_ relaxed-load + cast spelled out at each call site. - Collapse the three structurally-identical fire_* helpers in hook_handle.cpp onto a single fire_hooks_for_phase template, so the snapshot-lock-iterate-catch pattern lives in one place ahead of TASK-047..051 which would otherwise replicate it eight more times. Mark TASK-046 Done in the task index and persist the unworked review findings (24 minor, 0 major/critical) for later sweep.
…rt-circuit) Wires the two pre-routing, pre-handler hook phases that observe the inbound request. Both support short-circuit via hook_action::respond_with(...): a non-pass return populates mr->response_, sets the new mr->skip_handler flag, and routes through finalize_answer's new skip-handler branch. The body_chunk short-circuit also destroys any in-flight MHD_PostProcessor so its 32 KB buffer is freed (ASan-verified). Closes #273 (early 413 on oversize body) — demonstrated by examples/early_413.cpp. Partially addresses #272 (observation half of delayed body processing). Acceptance tests: - hooks_body_chunk_ctx_shape: compile-time pin for the TASK-045 ctx shapes (mutable http_request*, std::span<const std::byte> chunk, std::uint64_t offset, bool is_final). - hooks_request_received_short_circuit: 413 hook aborts the upload before any body bytes flow; downstream body_chunk hook never fires. Second sub-test pins the non-short-circuit path through to 200. - hooks_body_chunk_observes_progress: accumulated bytes equal body size; offsets are monotonic and start at zero. - hooks_body_chunk_short_circuit_no_leak: form-urlencoded POST forces MHD_PostProcessor allocation; first-chunk short-circuit must free it (sentinel for ASan). Implementation: - New fire_short_circuit_hooks_for_phase template in hook_handle.cpp (sibling to TASK-046's fire_hooks_for_phase) returns std::optional<http_response>; same shared_lock-snapshot-then-iterate pattern, throwing hook treated as pass per DR-009 §5.2. - New any_hooks_-gated firing sites in webserver_impl::requests_answer_first_step (request_received) and requests_answer_second_step (body_chunk). - modded_request gains a skip_handler bool; finalize_answer gains an early-exit branch that routes directly to materialize_and_queue_response when set. Both pipeline functions also re-check the flag so a request_received short-circuit suppresses body_chunk firings on subsequent MHD callbacks. - File-size mitigation: the two firing-site insertions pushed src/detail/webserver_request.cpp over the 500-LOC ceiling; mirrored the TASK-046 split pattern by carving src/detail/webserver_body_pipeline.cpp out (hosts both pipeline functions plus two anon-ns helpers that keep requests_answer_second_step at CCN <= 10). - hook_context.hpp doc comments now warn that body_chunk fires from arbitrary MHD worker threads at arbitrary granularity (no I/O, no per-chunk allocation; the chunk span aliases MHD-owned memory). - hooks_no_firing narrowed to drop request_received and body_chunk from its "must observe zero" set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mark TASK-047 as Done in specs/tasks/_index.md (was stale "Not Started"). Update the §4.10 phase table in specs/architecture/04-components/hooks.md to reference webserver_body_pipeline.cpp for request_received and body_chunk — the correct location after the webserver.cpp refactor. Co-Authored-By: Claude Sonnet 4.6 <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>
Convert each issue surfaced in specs/tasks/v2-branch-gap-audit.md into a workable task under specs/tasks/M7-v2-cleanup/, mirroring the M1..M6 format so groundwork's plan / implement / validate pipeline can drive them. Also tracks the audit document itself. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Eliminate both unscoped `#pragma GCC diagnostic ignored "-Warray-bounds"` directives flagged in specs/tasks/v2-branch-gap-audit.md §1 and add a lint-lane gate that prevents either watched file from regaining a file-scoped suppression. Site A — src/http_utils.cpp:62 The pragma sat above three orphan macros (CHECK_BIT, SET_BIT, CLEAR_BIT). Those macros had no remaining call sites in this TU after commit 7fc443a extracted the ip_representation body to src/detail/ip_representation.cpp; the pragma was guarding nothing. Delete both the pragma and the orphan macros. Site B — src/detail/ip_representation.cpp:55 The pragma sat above two used macros (CHECK_BIT, CLEAR_BIT) with five call sites. The historic -Warray-bounds false positive on these function-like macro shapes is the standard GCC VRP-loses-bound-across- macro-expansion pattern: at the call site `mask &= ~(1 << pos)` the value-range propagator can't see the loop-derived `[0, 15]` bound on `pos` and speculates a shift outside the storage GCC infers for the `uint16_t mask`. Replace both macros with anonymous-namespace `constexpr` helpers that take `pos` as `unsigned int` and force the shift through `1u`, with the bitwise-and-assign going through an explicit `static_cast<uint16_t>`. The function-call boundary plus explicit unsigned types is the documented recipe that silences the warning at the source on every supported GCC, so the file-scoped suppression can go away with no scoped push/pop fallback. All five call sites mechanically swap to the helper and explicitly cast their signed index expressions to `unsigned int` to keep the conversion visible. Guard — scripts/check-warning-suppressions.sh (new) Bash script wired into Makefile.am as `lint-warning-suppressions` and into the verify-build.yml lint lane next to lint-file-size / lint-complexity. For each watched file, it greps for top-of-line `#pragma GCC diagnostic ignored "-Warray-bounds"` and fails unless each hit is bracketed by an earlier `#pragma GCC diagnostic push` and a later `pop`. Watched-file list is intentionally narrow to the two TASK-060 files; future tasks broaden it as new suppressions are scoped. Acceptance criteria: - `grep -nE '^#pragma GCC diagnostic ignored "-Warray-bounds"' src/http_utils.cpp src/detail/ip_representation.cpp` returns no matches. - Debug build (`--enable-debug`, -Werror -Wall -Wextra -pedantic) on macOS Apple-Clang succeeds with no new warnings. - http_utils unit suite (412 checks across 87 tests, exercises ip_representation parsing) passes; ban_system integ suite passes in isolation, exercising block_ip / unblock_ip which round-trip through both rewritten helpers. - CI's GCC 11/12/13/14 matrix lanes will surface any residual -Warray-bounds regression by failing the compile. GCC-version diagnostic capture deferred to CI — the local host is Apple Clang and has no GCC. If a CI lane still emits the warning after the rewrite, the documented fallback (scoped push/pop with a __GNUC__-conditional version guard) lands in a follow-up commit on this branch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review follow-up to the initial TASK-060 commit.
scripts/check-warning-suppressions.sh
- Replace the narrow two-file WATCHED_FILES list with a runtime scan
of all .cpp files under src/. Safe because no other TU in src/
carries an unscoped -Warray-bounds pragma today, and the broad scan
means future TUs are guarded without anyone having to remember to
update a static list.
- Collapse the two per-candidate awk invocations (one for nearest
push-before, one for nearest pop-after) into a single awk pass that
emits both line numbers. Halves the work per candidate and keeps
the bracketing logic in one place.
- Document the known limitation of the "nearest push before / nearest
pop after" heuristic: an interleaved shape (push, pop, pragma, pop)
would slip through. Not present in the codebase; upgrade to a
depth counter if nested patterns ever appear.
Makefile.am
- Wire lint-warning-suppressions into check-local. Unlike the other
lint-* gates it has no external-tool dependency (bash/awk/grep
only), so it can run in every developer build and CI lane, not
just the dedicated lint lane. Updated the surrounding comment to
record that asymmetry and its rationale.
scripts/test_check_warning_suppressions.sh (new)
- Self-contained bash unit test that fixtures clean / bracketed /
unbracketed / mixed cases in a tmpdir, runs the script against
each, and asserts the expected exit code. Not wired into
`make check` (it builds throwaway src/ trees that would confuse a
parallel `make check` run); intended for direct invocation by
contributors editing the gate itself.
specs/tasks/M7-v2-cleanup/TASK-060.md / _index.md
- Mark all action items checked and flip status to Done.
specs/unworked_review_issues/2026-06-04_105130_task-060.md
- Reviewer log preserved for the audit trail; the one MAJOR finding
(single-pass awk) is addressed by this commit, the 37 MINORs are
catalogued for future passes.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Eliminate the two unscoped `#pragma GCC diagnostic ignored "-Warray-bounds"` directives flagged in specs/tasks/v2-branch-gap-audit.md §1 and add a `lint-warning-suppressions` gate (wired into both `make check` and the CI lint lane) that fails if any TU under src/ regains a file-scoped suppression. Followed by a review-driven simplification pass: scanner broadened to all of src/*.cpp, two-pass awk collapsed to one, and a self-contained unit test added for the gate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…s, stale doc references
- src/detail/webserver_register.cpp: complete the truncated TASK-029
block comment with the missing closing clause
("they are no longer reachable from the public API.").
- src/detail/webserver_register.cpp: install the full TASK-024 prologue
(six lines, ending "dangling resource pointer after the maps drop
their refs.") immediately above webserver::unregister_impl_, the
function the comment actually describes.
- src/detail/webserver_setup.cpp: remove the orphaned five-line
TASK-024 head that was sitting above the unrelated block_ip
(split-artifact from the 7-way webserver.cpp split).
- src/webserver.cpp:503-504: remove the two orphan comment fragments
left after removed logic ("dangling resource pointer..." and
"they are no longer reachable...") — both lines now have a single
canonical home in webserver_register.cpp after the steps above.
- test/Makefile.am:73-74: replace the stale "Currently in XFAIL_TESTS"
claim with a status-correct "Now an unconditional PASS" note that
cross-references the existing TASK-020 block at lines 533-536.
- scripts/check-readme.sh:273: drop the stale
"RELEASE_NOTES.md) continue ;; # created by TASK-042, not yet
present" case arm — TASK-042 shipped, the file now exists, and the
generic existence check at the bottom of the loop handles it.
- test/littletest.hpp: left untouched per planning decision (vendored
upstream liblittletest header, no fork policy authorising in-place
edits to stylistic comments).
Verification:
- make check: 87/87 PASS, 0 FAIL.
- check-file-size: PASS (FILE_LOC_MAX=750).
- check-readme.sh: returns 0 end-to-end.
- All five pre-edit failing grep guards now report zero matches.
- Each of the three reflowed sentence fragments has exactly one
canonical copy under src/.
Pre-existing on feature/v2.0 (NOT introduced by this task): the
check-doxygen.sh gate fails with seven warnings against unrelated
headers (create_webserver.hpp, hook_context.hpp,
webserver_websocket.hpp — none of which TASK-061 touches). Confirmed
by running check-doxygen against an unchanged feature/v2.0 checkout
and reproducing the same output.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mark all four action-item checkboxes as [x] and flip Status to Done in TASK-061.md; update the _index.md task table row to Done. 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
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