v2.0: Modernization (M1-M6, 44 tasks)#374
Draft
etr wants to merge 374 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.08% -0.96%
==========================================
Files 34 30 -4
Lines 1730 2579 +849
Branches 697 1006 +309
==========================================
+ Hits 1177 1730 +553
- Misses 80 186 +106
- Partials 473 663 +190
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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>
…ings recorded) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds test/integ/threadsafety_stress.cpp with two sub-tests: 1. concurrent_register_block_from_handlers_no_data_race — 16 curl clients hit an on_get lambda that re-enters the public webserver surface (register_path / unregister_path / block_ip / unblock_ip) for 60 s (default; HTTPSERVER_STRESS_SECONDS overrides). The TSan-clean rerun is the headline acceptance: the existing build-type=tsan matrix entry in verify-build.yml invokes make check, which auto-picks up the new check_PROGRAMS entry — no workflow edit required. Port 0 + get_bound_port() avoids collisions when the 60-s soak runs alongside other integ tests. 2. stop_from_handler_deadlocks_as_documented — opt-in (set HTTPSERVER_RUN_STOP_FROM_HANDLER=1) reproducer for the DR-008 negative case. Forks a child that calls stop() inside an on_get handler; on this libmicrohttpd, MHD detects pthread_join(self) returning EDEADLK and aborts with "Failed to join a thread." The fork contains the abort so the parent test binary stays healthy. Either a non-zero child exit or a 5-second timeout (child SIGKILLed by parent) counts as positive observation of the contract. Doxygen on webserver::stop() and ~webserver() now spells out the deadlock-or-abort consequence of calling stop() from a handler thread, pointing at DR-008 and §5.1. Test wall-clock: 60 s default, ~5 s minimum locally. Acceptance criteria all met: register_ok + unregister_ok > 0 and block_ok + unblock_ok > 0 in every observed run. Full make check -j1 passes 42/42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ings recorded) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse 17 paired `foo()/no_foo()` setters into single `foo(bool enable = true)` setters on create_webserver, rename the remaining one-way negative flags (`no_listen_socket`, `no_thread_safety`, `no_alpn`) to their positive counterparts (`listen_socket`, `thread_safety`, `alpn`), and add setter-time validation that throws `std::invalid_argument` with a parameter-named message: - port: int overload validates [0, 65535]; uint16_t overload preserved - max_threads, max_connections, memory_limit, connection_timeout, per_IP_connection_limit, max_thread_stack_size, nonce_nc_size, listen_backlog, address_reuse, tcp_fastopen_queue_size: reject < 0 - client_discipline_level: reject < -1 (-1 is the unset sentinel) - file_upload_dir: reject empty string - bind_address(string): error message now prefixed with "bind_address:" Header collapsed from 593 lines (feature/v2.0) / 554 lines (master baseline) to 253 lines — 54% under the v1 baseline (PRD §3.3 target was 30%). Acceptance grep `grep -E '^\s*create_webserver& no_' src/httpserver/create_webserver.hpp` now returns empty. Internal `webserver`-side field names (`no_listen_socket`, `no_thread_safety`, `no_alpn`) are unchanged so `webserver.cpp` does not churn — only the public API is renamed. Tests: new unit tests in test/unit/create_webserver_test.cpp cover port-out-of-range, every numeric validator, file_upload_dir empty, bind_address parameter-name message, and the bool-arg shape for every toggled flag (including the renamed listen_socket/thread_safety/alpn and widened tcp_nodelay/turbo/suppress_date_header/sigpipe_handled_by_app). 77 tests, 81 checks pass. Full `make check` green (42/42). Internal callers rewritten: test/integ/ws_start_stop.cpp, test/integ/file_upload.cpp, test/unit/routing_regression_test.cpp, examples/file_upload.cpp, examples/external_event_loop.cpp (comment). README.md updated to document the bool-arg public API. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ings recorded) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove #ifdef HAVE_BAUTH/HAVE_DAUTH/HAVE_GNUTLS/HAVE_WEBSOCKET guards from public headers. Public methods are now declared unconditionally and return documented sentinels or throw feature_unavailable when the controlling build flag is undefined (DR-007/§7). Add webserver::features() returning a struct of compile-time feature booleans. create_webserver::use_ssl(true) on a non-TLS build and register_ws_resource on a non-WebSocket build now throw feature_unavailable at the documented points. Acceptance: grep '#if(def)? HAVE_(BAUTH|DAUTH|GNUTLS|WEBSOCKET)' src/httpserver/*.hpp returns no matches; new tests webserver_features_test, webserver_ws_unavailable_test pass; the consumer fixture (test/consumer_fixture.cpp) compiles unchanged across configurations. Refs: PRD-FLG-REQ-001..004, DR-007. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t digest auth) and housekeeping (status Done in _index.md) - Add HAVE_DAUTH guard so digest_auth(true) on a build without digest support throws feature_unavailable. - Guard http_request::check_digest_auth against null underlying connection (test-request path). - Add test/unit/webserver_dauth_unavailable_test.cpp covering the new throw site. - Update specs/tasks/_index.md: TASK-034 status -> Done. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Capture the iter-2 validation review findings that were intentionally left unworked — the iter-2 pass approved across all 8 active agents and none of the remaining findings were blocking. Persisted via the standard persist-unworked-findings.js helper.
…es() (PRD-FLG-REQ-001..005)
Mirror TASK-023's ownership pattern on the websocket-registration
surface (PRD-HDL-REQ-003, PRD-HDL-REQ-005, DR-010).
Public API changes (webserver.hpp):
- Removed: bool register_ws_resource(string, websocket_handler*).
- Added: void register_ws_resource(string, unique_ptr<T>) where
T : websocket_handler (header-inline shim that constructs a
shared_ptr and forwards to the canonical overload).
- Added: void register_ws_resource(string, shared_ptr<websocket_handler>).
- Added: void unregister_ws_resource(string).
- All three throw feature_unavailable("websocket", "HAVE_WEBSOCKET")
on a HAVE_WEBSOCKET-off build (consistent with TASK-034).
Internal changes (webserver_impl.hpp, webserver.cpp):
- registered_ws_handlers map value type: websocket_handler* -> shared_ptr.
- ws_upgrade_data::handler: websocket_handler* -> shared_ptr; the
dispatch path takes a shared_ptr copy under the shared lock before
releasing it, so the handler is kept alive across the MHD upgrade
callback even if unregister_ws_resource races to drop the slot
mid-upgrade (mirrors the TASK-023 TOCTOU fix on the HTTP side).
- upgrade_handler now owns ws_upgrade_data via unique_ptr for the
duration of the session loop instead of `delete data` immediately
after extraction; this is what keeps the shared_ptr reference alive
through on_open / on_message / on_close.
- register_ws_resource throws std::invalid_argument on duplicate
registration (v1 silently overwrote via operator[]); this matches
the rest of the v2.0 registration surface.
Tests:
- New: test/unit/webserver_register_ws_smartptr_test.cpp. Compile-time
signature contract (unique_ptr / shared_ptr overloads exist, raw-
pointer overload absent via SFINAE void_t, unregister_ws_resource
exists) plus HAVE_WEBSOCKET-on runtime ownership tests (unique_ptr
dtor on webserver destruction, shared_ptr caller-keeps-alive,
throw-on-null, throw-on-duplicate, unregister-drops-slot, unregister-
missing-is-noop).
- Updated: test/unit/webserver_ws_unavailable_test.cpp. Three sub-tests
on a HAVE_WEBSOCKET-off build cover register_ws_resource(unique_ptr),
register_ws_resource(shared_ptr), and unregister_ws_resource; all
three must throw feature_unavailable naming both "websocket" and
"HAVE_WEBSOCKET".
- Updated: test/consumer_fixture.cpp:touch_ws exercises both new
overloads plus unregister_ws_resource so the consumer-compile gate
covers the new surface.
Callsite migration:
- examples/websocket_echo.cpp: now uses make_unique<echo_handler>().
Verified locally (macOS, HAVE_WEBSOCKET-off): make check 47/47 pass
on both --enable-debug and release. The HAVE_WEBSOCKET-on runtime
suite is exercised by the CI matrix (verify-build.yml).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-REQ-003, PRD-HDL-REQ-005, PRD-FLG-REQ-002)
Wire the new handler-return-by-value contract end-to-end through the
dispatch path. http_resource::render_* virtuals now return http_response
by value (DR-004, PRD-RSP-REQ-007); detail::lambda_resource::invoke_
drops the v1 make_shared<http_response> shim and forwards the prvalue
directly; webserver_impl::finalize_answer's dispatch site moves the
returned value into modded_request::response_ — a new
std::optional<http_response> slot that anchors the deferred body's
lifetime to ~modded_request() (and therefore to MHD's
request_completed), satisfying DR-010 verbatim.
Synth-response helpers (not_found_page, method_not_allowed_page,
internal_error_page, run_internal_error_handler_safely) return
http_response by value. materialize_response and
get_raw_response_with_fallback take http_response* / modded_request*
through the optional. detail::empty_render and src/http_resource.cpp
are removed; the default render() inlines a default-constructed
http_response{} (status_code_ == -1 sentinel) routed through
internal_error_page as before.
The auth_handler_ptr signature (shared_ptr<http_response>) is
intentionally preserved — its migration is out of TASK-036 scope; the
dispatch path dereferences and moves into the optional.
TASK-035's websocket_handler shared_ptr / unique_ptr overloads are
also preserved verbatim — they are a separate API surface (DR-010
Option 1 for websocket handler ownership).
Tests:
- test/unit/http_resource_test.cpp: ten static_asserts pin the
by-value return type of every render_* virtual at compile time.
- test/integ/deferred.cpp: three new acceptance tests cover
AC-1 (on_get lambda by-value -> 200/body), AC-2 (http_resource
subclass render_get by-value -> 200/body), and AC-3
(destruction_sentinel inside the producer's captures is destroyed
exactly when request_completed fires, proving the producer state
outlives every MHD trampoline invocation).
- All 47 tests in the suite pass.
Related: PRD-HDL-REQ-001, PRD-RSP-REQ-007, DR-004, DR-010, §5.3
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-001, PRD-RSP-REQ-007) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires a two-lane GitHub Actions matrix that locks in the "same consumer
source compiles against every HAVE_* combination" invariant introduced
by TASK-034. PRD-FLG-REQ-001 / arch §9 testing item 2.
Changes:
- test/consumer_fixture.cpp: extend the existing TASK-034 fixture to
pin every remaining TLS cert accessor (get_client_cert_{issuer_dn,
cn,fingerprint_sha256,not_before,not_after}, has_client_certificate,
is_client_cert_verified), the DAUTH check_digest_auth /
check_digest_auth_digest declarations via member-function pointers
(so the OFF lane links without invoking a method that would throw
on a !HAVE_DAUTH build), and the positive-`true` form of the three
feature-flag setters use_ssl / basic_auth / digest_auth on
create_webserver (again as member-function pointers).
- .github/workflows/verify-build.yml: add two matrix entries,
flag-invariance-on (stock libmicrohttpd + gnutls) and
flag-invariance-off (libmicrohttpd rebuilt with --disable-bauth
--disable-dauth --disable-websockets and gnutls install step
skipped so all four HAVE_* auto-detect to off). Each lane runs
`make consumer_fixture`; broad `make check`/cppcheck are skipped
since the gate is compile+link-only. Adds two verification steps
asserting (a) libmicrohttpd has the BAUTH/DAUTH symbols stripped
and libmicrohttpd_ws is absent, and (b) libhttpserver's configure
reported every HAVE_* as 'no' in the OFF lane.
- test/Makefile.am: cross-reference TASK-037 in the consumer_fixture
banner.
- specs/tasks/_index.md, specs/tasks/M6-release/TASK-037.md: status
Not Started -> Done; action items checked off.
Local RED proof (not committed): wrapping get_client_cert_dn() in
#ifdef HAVE_GNUTLS and compiling consumer_fixture.cpp with -UHAVE_GNUTLS
fails with "no member named 'get_client_cert_dn'", confirming the gate
catches the documented regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…56 pin libmicrohttpd downloads), correctness (make -C test consumer_fixture so CI builds in build/test/), and coverage (pin remaining unconditional TLS credential-material setters in consumer_fixture). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rch §9 testing item 2)
…tizer-flag fix
Adds test/unit/http_response_move_sanitizer_test.cpp — the v2.0
sanitizer canary for http_response move ops (DR-005, AR-004,
PRD-RSP-REQ-001 / PRD-RSP-REQ-007). Complements the existing whitebox
SBO test (http_response_sbo_test.cpp) by:
* Driving all four move-assign cases plus both move-ctor cases via
factory-constructed responses (string / empty / file), exercising
the public API rather than placement-new'ing bodies through the
SBO friend hook.
* Covering the heap-fallback branch in adopt_body_from /
destroy_body with a synthetic >64 B body subclass (fat_body) —
no production body currently exceeds the SBO budget, so this is
the first test that genuinely exercises the heap-pointer-swap
path. fat_body is placed through the same friend hook the SBO
test uses; no production-API widening.
* Pinning the moved-from invariant contract: a moved-from
http_response is destructible, accessor-safe (get_status / kind
/ get_header / get_headers().size() etc. are well-defined), and
re-assignable (a fresh response can be move-assigned into it).
* Adding a file_body move-ctor case so the hand-written fd-ownership
transfer in file_body is sanitizer-exercised.
Each cycle is non-tautological — verified by injecting deliberate
bugs into adopt_body_from (force inline branch on heap path) and
destroy_body (skip ~body on heap path) and confirming the
corresponding heap-path assertions and dtor-counter checks fired.
Implementation restored after RED verification.
Also fixes a long-standing typo in .github/workflows/verify-build.yml
(lines 656-660): `CXXLAGS` -> `CXXFLAGS` across the asan / msan /
lsan / tsan / ubsan lanes, plus a duplicated `export export` on the
ubsan line. Before this fix, autoconf did not honour the C++ flag
(CXXLAGS is not a recognized variable), so the sanitizer runtime
libs were linked but C++ TUs were compiled without -fsanitize=...
The matrix names were unchanged but the lanes were no-ops on the
library code. The fix is invisible at the matrix-surface level; any
sanitizer findings in unrelated code after this commit are
pre-existing issues the CI was silently letting through, not
regressions introduced here.
Local verification:
* make check (debug build, all 48 testsuite entries): PASS
* ASan rebuild + http_response_move_sanitizer: 60/60 checks PASS
* UBSan rebuild + http_response_move_sanitizer: 60/60 checks PASS
* cpplint: clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ings (1 major, 27 minor). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s (PRD-RSP-REQ-001, PRD-RSP-REQ-007, arch §9 testing item 3)
…p_resource)
Adds a `make bench` target (separate from `make check`) that runs two
microbenchmarks verifying the two PRD §3.6 numeric acceptance criteria:
* bench_get_headers -- PRD-REQ-REQ-001: v2.0 get_headers() is
>=10x faster than v1's. Measured: ~230x
on the maintainer reference host.
* bench_sizeof_http_resource -- PRD-REQ-REQ-003: sizeof(http_resource)
shrunk by ~the empty std::map footprint
after the method_set refactor. Pure
compile-time static_assert.
The v1 baseline literals live in test/v1_baseline/v1_constants.hpp,
sampled once on master (d8b055e) and committed with the host metadata
that produced them. test/v1_baseline/measure_v1_sizes.cpp and
measure_v1_get_headers.cpp ship in EXTRA_DIST as the re-measurement
recipe; they are not built by `make bench` or `make check`. The full
methodology (warmup, 10x1M timing loop with steady_clock + asm-volatile
sink, libstdc++ vs libc++ caveats) is documented in
test/PERFORMANCE.md, alongside the rationale for keeping bench out of
the sanitizer-gated CI matrix.
Notes on the static_assert algebra: TASK-039's literal task wording
suggests `sizeof(http_resource) <= V1_SIZEOF - V1_MAP_SIZEOF`. That
formula fails on every stdlib because v2.0 added a small method_set
member to replace the map. The committed assertion encodes the
mathematically correct contract -- "the removed map saved at least
its empty footprint, less the new method_set field" -- and a tighter
strict-shrinkage check (sizeof(http_resource) < V1_HTTP_RESOURCE_SIZEOF)
as belt-and-suspenders. The divergence from the task wording is
called out in PERFORMANCE.md "Why not the literal task formulation".
Verified:
- make check: 48/48 PASS, no bench binaries invoked.
- make bench (release -O3): bench_sizeof_http_resource passes static_assert;
bench_get_headers reports ratio=226-403x (well above 10x threshold).
- make bench (debug -O0 -Werror -Wextra -pedantic): clean compile, ratio=134x.
- cpplint: 0 errors on all new files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ready fixed)
Major (performance):
- commit_handlers_to_shim: pass handler by value, copy into N-1 slots,
move into last slot — eliminates one extra heap allocation per
multi-method registration (performance-reviewer-iter1-1)
- Updated declaration in webserver_impl_dispatch.hpp to match
Cosmetic / documentation fixes:
- on_methods_: add null-byte path guard (CWE-20, security-iter1-23)
- on_methods_: add security comment documenting exact-only semantics in
single_resource mode (CWE-284, security-iter1-22)
- route(method_set,...): add comment explaining no count_ guard needed
and documenting count_-only set edge case (code-quality-iter1-6)
- header_func: add rfind starts_with comment (code-simplifier-iter1-16)
- header_func: replace std::string(kAllowPrefix).size() with strlen()
(code-simplifier-iter1-17)
Test additions:
- route_delete_serves_delete_request: exercise DELETE beyond GET/POST
(code-quality-iter1-7, PORT+15)
- route_duplicate_method_path_throws_for_post: method-agnostic conflict
check (code-quality-iter1-9, PORT+16)
- route_root_path_serves_get_request: root path sanity (test-iter1-30,
PORT+17)
- route_method_set_count_sentinel_only_behavior: pin current behaviour
for method_set{}.set(count_) (test-iter1-29, PORT+18)
- Rename route_only_allows_registered_method ->
route_get_returns_405_with_allow_header_for_post_request
(test-quality-iter1-27)
Already-fixed items (verified, no action taken):
- explicit constructor (arch-iter1-2)
- method_set::empty() predicate (code-quality-iter1-3)
- for_each_requested_method helper (code-quality-iter1-4/5/14/15)
- is_new_entry naming (code-simplifier-iter1-12/13)
- assert+unconditional guard in lambda_resource::invoke_
(security-iter1-21)
- TASK-036 comment in lambda_resource (performance-iter1-18)
Deferred: TIME_WAIT convention (8), ws.start() assertion (10),
curl error handling (11), rvalue handler overload (19), http_endpoint
caching (20), TASK-029 renames (24/25), TASK-034/035 ifdef (26),
multi-assert atomicity test split (28).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All items checked: 1 major fixed, 12 minors already fixed by prior tasks, 11 minors fixed in this pass, 6 minors deferred (pre-existing TASK-029/034/035 items, project conventions, API-width concerns). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tic_assert Remove TASK-021 ticket-prefix annotations from permanent source and test comments (modded_request.hpp, webserver_impl_dispatch.hpp, webserver_request.cpp, webserver_routes.cpp, method_utils.hpp, http_method.hpp, http_resource.hpp, webserver_route_test.cpp, http_resource_test.cpp, basic.cpp). Replace the TASK-021-acceptance / PRD-REQ-REQ-002/003 block comment above the http_resource static_assert with a plain, self-contained rationale. Mark minor items 4-7 in the 1st-pass review file (items 4 and 7 deferred as superseded by 2nd-pass; items 5 and 6 fixed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Initialize modded_request::callback member pointer to nullptr by default; previously uninitialized, which is UB even though the unrecognized-method path never invokes it (is_allowed returns false for unknown strings, so the else branch executes instead). - Remove render_only_resource_methods_allowed test: it duplicates the nine is_allowed assertions already covered by is_allowed_known_methods on the same base-class constructor path. is_allowed_known_methods (using simple_resource) is kept as the canonical check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Second-pass review of TASK-021 (webserver/method_set bitmask era). Most items referenced TASK-021 worktree code that TASK-027/036/048 already superseded in feature/v2.0: - 26 items marked [x]: already resolved by later refactors (no code remaining from the specific TASK-021 forms referenced) - 2 items marked [x]: actively fixed in fix/task-021-2nd-review-cleanup branch (callback nullptr init; redundant test removal) - 2 items marked [-]: deferred (render_GET naming per arch §4.4 needs separate task; Allow-header caching per review itself is optional) - 1 item marked [-]: kept disallow_all_methods for isolated diagnostics Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
# Conflicts: # src/httpserver/details/modded_request.hpp # test/unit/http_resource_test.cpp
…files Sweeper agents verified each unchecked finding against the current code on feature/v2.0 and added a *Status:* line (Resolved / False positive / deferred) where one was missing. Coverage: - 22 review files swept (task-007/008/010/011/012/013/019/020/022/023/028/ 029/030/036/040/042/047/049 plus three already fully dispositioned) - ~330 items dispositioned this pass - 0 remaining unchecked items without a *Status:* / *Fixed:* / *Deferred:* marker across the whole specs/unworked_review_issues/ directory Notable verified-fixed clusters: TASK-011/012/013 closed most of TASK-010's http_response findings; TASK-046/048/049 closed most of TASK-047's hook-bus findings; TASK-027 cache + ban-system work closed most of TASK-019/020/030. Notable deferrals worth a follow-up pass (queued for Pass 2): - task-019 #22: A09 password plaintext in http_request operator<< - task-010 #23/#24: input-validation gaps in http_response file/pipe factories - task-036 #37: v2 3-tier route table (TASK-027) built but never wired into dispatch hot path - task-036 #38: auth_handler_ptr migration to optional<http_response> - task-040 #58–#61: hardcoded creds + reflected XSS + path traversal in example code (users will copy these) - task-047 #3/#4/#5: hook_handle::remove() switch refactor + fire_hooks unification + curl helper extraction No code changes in this commit — only review-file dispositions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the highest-signal deferred findings surfaced by Pass 1: examples/ — security fixes copied by users (CWE-798, CWE-79, CWE-22): - basic_authentication.cpp: read BASIC_USER/BASIC_PASS from env; bail if unset; capture into the lambda. Removes hardcoded "myuser"/"mypass". - digest_authentication.cpp: read DIGEST_PASS from env; bail if unset. - file_upload.cpp: add html_escape() helper and escape every user-controlled field (key, filename, fs path, content-type, transfer encoding) before writing into the HTML table. - file_upload_with_callback.cpp: html_escape() the filename in the HTML body and add is_safe_filename() guard (rejects empty/./../slash/ backslash/NUL) before joining with permanent_dir. test/REGRESSION.md §4 — prose drift: - The pinned overlap test now asserts `*hp == first` (deterministic first-wins) but the prose still said "could be either one". Updated to match the actual assertion and remove the v1-era hedge. Closes task-040 #58 #59 #60 #61 and task-028 #9 #25 in the unworked review issues tracker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
44 of 45 deferred TASK-009 findings were scoped to TASK-011/012/013, which have since merged. Verified each against the current code and closed: - TASK-013 follow-ups (final, v1-compat header removal, virtual *_response method removal, MHD forward-decl cleanup, friends-private, AC/static_assert in TASK-013.md): http_response is final at http_response.hpp:74; empty/ deferred/file/string/iovec/pipe/basic_auth/digest_auth_response.hpp gone. - TASK-011 follow-ups: get_header/footer/cookie are nodiscard const string_view. - TASK-012 follow-ups: with_header/footer/cookie have & and && overloads returning http_response& / http_response&&. - namespace details → detail consistent across src/, test/, docs/. - security: callback null-deref guarded by 405 short-circuit; upload filenames sanitized via http_utils::sanitize_upload_filename. One item genuinely still open: #35 (deferred_body std::function SBO threshold doc + optional void* producer overload) — low-priority performance polish, no follow-up task currently owns it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final cleanup pass over the unworked review issues tracker: - Flipped 7 checkboxes whose *Status:* already indicated Resolved / Accepted / No-action (task-028 #9/#25, task-031 #25/#27/#28/#29/#35). - Converted 74 clearly-cosmetic deferrals (naming preferences, idiom choices, comment trim suggestions, "consider renaming" notes) to explicit *Status:* wontfix. Kept the checkbox as [ ] so they remain visible in the open list but are no longer in the actionable backlog. Final state of specs/unworked_review_issues/: - 1974 total findings across 37 review files - 1578 closed [x] / [~] (80%) - 322 still-open deferred (actionable backlog) - 74 wontfix (cosmetic, explicit close) - 0 items missing a *Status:* line The 322 actionable deferrals skew toward substantive backlog: missing tests, missing input validation, perf hot paths, refactor candidates, and spec/architecture drift. The full list of "real engineering work worth a follow-up pass" surfaced by Passes 1-3 is preserved in each review file's *Status:* lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the substantive deferrals surfaced by the four-pass review-backlog sweep into a single planning doc, formatted to match specs/tasks/M*/TASK-*.md so each entry can be split into its own task file when work starts. - TASK-053 Wire lookup_v2() into dispatch hot path (L, GA-blocker) - TASK-054 Migrate auth_handler_ptr to optional<http_response> (M, GA-blocker) - TASK-055 DR-009 revision: default error body must not surface e.what() (M, GA-blocker, CWE-209) - TASK-056 Hash-DoS hardening + prefix-route disambiguation (M, GA-blocker, CWE-407) - TASK-057 Redact credentials in http_request::operator<< (S, GA-blocker, A09:2021) - TASK-058 Hot-path allocation pass (L, post-v2.0 polish) - TASK-059 sha256-pin PMD analyzer in CI (S, GA-blocker) Each entry has: goal, action items, dependencies, acceptance criteria, related findings (back-references to specs/unworked_review_issues/), and related decisions. Execution-order section recommends a 3-4 week sequencing for a single engineer with TASK-057 and TASK-059 as Friday-afternoon warm-ups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls in two commits authored on the orphan fix/task-007-review-cleanup worktree at /private/tmp/task-007-review-cleanup that the Pass 1 review sweeper flagged as missing from feature/v2.0: - 477a06f TASK-007 review cleanup: address cosmetic and minor behavioral findings (Makefile.am hygiene comments + sed-vs-awk + .PHONY, verify-build YAML, TASK-007/TASK-020 AC grep pattern, header_hygiene_test MSYS2 guard). - dff19e5 TASK-007 review: broaden HYGIENE_STAMP deps to include Makefile.am and consumer TU. Both commits were authored 2026-05-27 by Sebastiano Merlino with Claude Sonnet 4.6 — the user's own work, just stranded on the fix branch. # Conflicts: # Makefile.am
Flips the canonical auth handler typedef from
std::function<std::shared_ptr<http_response>(const http_request&)>
to
std::function<std::optional<http_response>(const http_request&)>,
completing the DR-009 / PRD-RSP-REQ-007 value-typed response rollout
onto the last remaining heap-allocating slot on the public API.
One-build escape hatch:
- compat::auth_handler_v1_ptr keeps the v1 shared_ptr-returning std::function
shape (with a [[deprecated]] attribute).
- compat::adapt_legacy_auth(legacy) wraps the v1 callable into the new
optional shape (nullptr -> nullopt; non-null -> moved-from optional).
- A [[deprecated]] create_webserver::auth_handler(compat::auth_handler_v1_ptr)
overload accepts the legacy shape and routes through adapt_legacy_auth.
Both deprecation sites cite TASK-054 and point at the new typedef.
Dispatch path: the before_handler auth-alias hook in webserver_aliases.cpp
now consumes std::optional<http_response> directly. Removes one heap
allocation per authenticated request (the shared_ptr control block);
small responses still ride the http_response SBO with zero further allocs.
Tests:
- test/unit/auth_handler_optional_signature_test.cpp NEW: static_assert
the new typedef shape; end-to-end pin nullopt-allows + engaged-rejects
via curl; hook-count invariant (+1 on before_handler).
- test/unit/auth_handler_legacy_shim_test.cpp NEW: pin the v1 typedef
alias shape, the deprecated setter overload, hook-count invariant via
the shim, and verbatim status/header forwarding through
compat::adapt_legacy_auth (proves response state is moved, not lost).
TU-scope #pragma GCC diagnostic ignored "-Wdeprecated-declarations".
- Migrated:
test/unit/hooks_alias_count_test.cpp (auth lambdas -> optional shape)
test/unit/webserver_register_path_prefix_test.cpp (reject_auth)
test/unit/create_webserver_test.cpp (builder_auth_handler)
test/integ/authentication.cpp (centralized_auth_handler)
Docs:
- README.md: setter blurb + worked example switched to optional shape.
- RELEASE_NOTES.md: added "auth handler" entry under "What changed
semantically" + rename-pair row in the v1->v2 table.
- specs/architecture/04-components/create-webserver.md: documented the
new signature, the compat alias, and the one-build deprecation window.
- src/httpserver/create_webserver.hpp: Doxygen on the setter rewritten;
TODO at lines 92-99 removed.
- examples/centralized_authentication.cpp: drops <memory>/make_shared,
returns std::optional<http_response> directly.
Verification:
- All TASK-054 new + migrated tests PASS (auth_handler_optional_signature,
auth_handler_legacy_shim, hooks_alias_count, webserver_register_path_prefix,
create_webserver, authentication).
- 73 PASS / 0 NEW FAIL in the broader test suite. 3 pre-existing baseline
segfaults (lookup_pipeline, route_table_concurrency, routing_regression)
and 6 pre-existing compile failures (basic, http_resource,
header_hygiene_iovec, iovec_entry, hook_api_shape,
hooks_per_route_resource_destroyed_first) reproduce on feature/v2.0
HEAD without these changes.
- scripts/check-examples.sh, check-readme.sh, check-release-notes.sh: OK.
- cpplint clean on new + modified files.
Note on the task action-item line: the spec says "Update the auth
short-circuit path in webserver_dispatch.cpp" -- the actual call site
post-TASK-048 is in webserver_aliases.cpp::install_default_alias_hooks_
(verified by grep -- webserver_dispatch.cpp does not reference
auth_handler). The fix is in webserver_aliases.cpp.
Check all six action-item checkboxes in the TASK-054 section of v2-deferred-backlog-plan.md and add **Status:** Done, reflecting that the auth_handler_ptr optional migration is fully implemented. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrates auth_handler_ptr from shared_ptr<http_response> to optional<http_response>, removing the last shared_ptr<http_response> on the public API (PRD-RSP-REQ-007, DR-009). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DR-009 Revision 1: the default internal-error response body is now the fixed string "Internal Server Error" (constants::INTERNAL_SERVER_ERROR). The originating exception's e.what() text is no longer surfaced on the wire by default — closes the CWE-209 information-disclosure surface flagged by task-031 #3, task-031 #4, and task-036 #44. The verbatim message is still: - forwarded to the configured internal_error_handler; - logged via the configured log_error callback (log_dispatch_error). A new builder flag, create_webserver::expose_exception_messages(bool), restores the v1 / pre-revision message-in-body behaviour for development. Default is false (security-fix default). Tests: - basic.cpp: split dr009_runtime_error_message_surfaces_in_default_body and dr009_non_std_exception_yields_unknown_exception_in_default_body into four tests covering both the fixed-body default and the verbose-mode opt-in (PORT+80/+83 reused for the default-body half, PORT+88/+89 for the opt-in half). - basic.cpp: basic_suite::set_up opts the shared ws into expose_exception_messages(true) so the legacy tests that pre-date the revision and probe the message-forwarding path (exception_forces_500, untyped_error_forces_500, file_serving_resource_missing, file_serving_resource_dir, long_error_message) keep their original assertion intent. - basic.cpp: dr009_feature_unavailable_lands_as_generic_500 opts in for the same reason (its intent is "feature_unavailable's what() flows through the message-forwarding path to the body"). - create_webserver_test.cpp: new builder_expose_exception_messages_toggle. Docs: - DR-009.md: appended "Revision 1 (2026-05-29)" subsection. - 05-cross-cutting.md §5.2: contract points 1 and 2 amended to note the fixed-body default and the opt-in flag. - webserver.hpp class-level Doxygen block: point 2 amended. - create_webserver.hpp internal_error_handler Doxygen: amended. - create_webserver.hpp expose_exception_messages: new Doxygen with CWE-209 @warning. - webserver_impl_dispatch.hpp log_dispatch_error: Doxygen note that the verbatim message reaches the log regardless of the flag. - webserver_error_pages.cpp log_dispatch_error: matching body comment. - README.md "Error propagation": updated points 1 and 2 + worked example callout; new builder bullet under "Custom error handlers". - RELEASE_NOTES.md "What changed semantically": new bullet documents the behaviour change; "Error propagation" section bullet extended. Acceptance criteria: - default body no longer surfaces e.what() (dr009_default_body_is_fixed_string) - opt-in restores v1 behaviour (dr009_verbose_body_surfaces_message_when_opted_in) - log path unchanged (dr009_runtime_error_logged_via_error_logger) - builder toggle smoke-tested (builder_expose_exception_messages_toggle) - all 97 basic tests pass (291 checks, 0 failures); create_webserver unit suite passes (131 checks, 0 failures). Note on the deferred clause "with the request id (if any) appended": the codebase has no request-id concept today; adding one is out of scope for a security-fix revision and the fixed-body alone satisfies CWE-209. DR-009 Revision 1 "Consequences" documents this trade-off. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
17 minor items deferred from the multi-agent validation pass on TASK-055 (sanitize default 500 body / DR-009 Revision 1). All critical and major findings were addressed in the implementation commit; remaining items are documentation/cosmetic polish that fits a future hygiene sweep. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two security findings on the v2 routing table deferred during the
TASK-027 cleanup, fixed together because both land in the same code
area and share the same testing surface.
1. CWE-407 hash-flooding immunity (radix tree child container).
src/httpserver/detail/radix_tree.hpp swaps the per-segment children
container of every radix_node from
std::unordered_map<std::string, std::unique_ptr<radix_node>>
to
std::map<std::string, std::unique_ptr<radix_node>, std::less<>>.
Rationale: URL path segments are attacker-controlled, and neither
libc++ nor libstdc++ seed std::hash<std::string> by default. A
crafted sibling-key corpus can degrade unordered_map::find from
O(1) amortized to O(n) per probe. std::map (red-black tree) gives
O(log n) worst case with no hashing in the loop. The transparent
comparator std::less<> lets the hot-path find() take a
std::string_view directly without constructing a temporary
std::string per probe (a small bonus win on the cache-miss path).
The plan considered three options (std::map, in-tree flat_map,
hash-randomized unordered_map). std::map wins on minimum-diff +
pointer stability + zero new code in detail/. Typical URL trees
branch shallowly (< 10 children per node), so the constant-factor
difference vs hashing is dominated by the per-segment string
compare either way.
2. Prefix-vs-exact terminus collision guard at registration time.
src/detail/webserver_routes.cpp (upsert_v2_radix_route,
insert_fresh_v2_entry) and src/detail/webserver_register.cpp
(register_v2_route) call a new helper
webserver_impl::reject_terminus_collision(key, want_is_prefix)
that throws std::invalid_argument BEFORE any mutation when a
register_path-then-register_prefix (or the reverse) lands on the
same canonical path. The route-cache key (method, path) cannot
discriminate the two kinds at lookup time, so the conflict is
rejected at the source rather than silently shadowing one or the
other.
A new radix_tree primitive radix_tree<T>::has_terminus_at(path,
is_prefix) supports the guard: it returns true iff the EXACT node
reached by tokenizing `path` carries a terminus of the requested
kind (no fallback to prefix ancestors, no wildcard descent —
pattern-exact equality).
register_impl_ and on_methods_ wrap the v2 call in a try/catch that
rolls back the v1-tier inserts on throw, so the documented
atomicity contract ("a failed registration leaves the table
exactly as it was") still holds across the v1+v2 dual-write window
that v2.0 keeps for backward compatibility.
Tests:
- test/unit/routing_regression_test.cpp: six new pin-tests cover
every combination of {register_path, register_prefix, on_get} on
the same path with opposite polarity:
* register_exact_after_prefix_does_not_collide
* register_prefix_after_exact_does_not_collide
* register_path_after_prefix_does_not_collide
* register_prefix_after_path_does_not_collide
* parameterized_exact_after_parameterized_prefix_does_not_collide
* parameterized_prefix_after_parameterized_exact_does_not_collide
Each pins both halves of the contract: the second registration
throws AND the original entry survives intact.
- test/unit/webserver_register_path_prefix_test.cpp: paired pin-test
register_path_and_register_prefix_on_same_path_collide for the
public class-based surface; existing
unregister_resource_removes_both_path_and_prefix_registrations
and unregister_path_leaves_prefix_registration_intact updated to
use DISTINCT paths (the pre-TASK-056 same-path setup is now
forbidden state by contract).
- test/integ/basic.cpp: family_endpoints and duplicate_endpoints
updated to the same model.
- test/integ/ws_start_stop.cpp: register_duplicate_resource_throws
updated.
- test/integ/threadsafety_stress.cpp: new sub-test C
adversarial_segments_registration_no_latency_spike hammers the
registration path with 15 000 sibling segments per parent (union
of plan options β + γ — 32-byte strings with 24-byte shared
prefix and 8-byte high-entropy tail) across 3 parents under 4
writer threads. Asserts p99 < 10 × warmup-median (the
deterministic encoding of the task's "no latency spikes > 10×
baseline" criterion). On a modern host the corpus completes in
well under 1 s with p99/warmup_median ratio < 2×.
Drive-by fixes (needed to keep the suite green with the new tests
exercising lookup_v2 paths that the v1 surface did not hit):
- src/httpserver/detail/route_cache.hpp:
empty-cache early-out in route_cache::find_promote_for_lookup.
libc++ default-constructed unordered_map has bucket_count() == 0;
calling cbegin(0)/cend(0) on it dereferences a null bucket-list
pointer (UB). Triggered by any test that calls lookup_v2 before
a cache insert. Same fix lives on TASK-053; this hunk becomes
a benign duplicate if TASK-053 merges first.
- src/detail/webserver_dispatch.cpp:
stop moving result.entry / result.captured_params into the
cache_value on the lookup_v2 miss path — the caller consumes
`result` after the function returns, so the move-out left it
reading a moved-from variant. Same fix on TASK-053.
Documentation:
- specs/architecture/04-components/route-table.md §4.7 amended
with: (a) container choice and rationale (CWE-407 immunity);
(b) the prefix-vs-exact collision-detection contract; (c) updated
"Future evolution" paragraph (flat_map is now the next fallback
if std::map probe cost ever dominates); (d) "Implementation
status" updated.
Acceptance criteria (from task):
- bench_route_lookup ≤ 2× regression on cache-miss radix path:
cannot be measured on this branch — bench_route_lookup.cpp lives
on TASK-053, not yet on feature/v2.0. The plan flagged this
explicitly. The gate will be re-checked once TASK-053 lands and
this branch is rebased onto it.
- new regression test passes: 6 new TASK-056 pin-tests pass
(routing_regression: 26 tests, 104 successes, 0 failures).
- 60 s adversarial-segment stress run completes without latency
spikes > 10× baseline: passes deterministically with the std::map
swap (p99/warmup_median observed < 2× on dev host).
- routing architecture doc reflects the new container: updated
(route-table.md §4.7; note that the task text said routing.md but
the actual doc filename is route-table.md per the rest of the
spec).
Pre-existing build issues encountered, NOT introduced by this task:
test/unit/http_resource_test.cpp, header_hygiene_iovec_test.cpp,
iovec_entry_test.cpp, hook_api_shape_test.cpp, and
hooks_per_route_resource_destroyed_first.cpp fail to compile on
feature/v2.0 with -Werror (private-ctor, missing set_up/tear_down,
unused-parameter, static_assert mismatch). These are not touched
by TASK-056 and have no diff vs feature/v2.0.
Naming note: the task text references upsert_v2_param_route; the
actual function is webserver_impl::upsert_v2_radix_route (renamed in
an earlier task). The guard is placed there + at insert_fresh_v2_entry
+ at register_v2_route to cover both registration entry points.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Housekeeper: - Mark all five TASK-056 action items complete ([x]) in v2-deferred-backlog-plan.md - Add 'Status: Done' to TASK-056 section (consistent with TASK-054/055 pattern) - Correct routing.md -> route-table.md filename in action item text test-quality-reviewer (major): - threadsafety_stress: add zero-floor guard (baseline = max(warmup_median, 1000ns)) so the latency gate never becomes 'p99 < 0' on sub-microsecond hosts - routing_regression_test: remove internal tier/is_prefix field assertions from all six collision tests; replace with observable-behaviour checks (does the surviving route resolve expected paths, does the failed registration leave no prefix terminus) performance-reviewer (major): - radix_tree: change insert/remove/has_terminus_at and tokenize() from std::string_view to const std::string& so callers holding a const std::string& avoid the extra copy through the string_view conversion when calling tokenize_url - webserver_dispatch: fix misleading comment on the cache-miss copy path; clarify that a full copy (not move) is required because result is returned by value and moving captured_params out would silently empty the return value Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The acceptance criterion requiring `bench_route_lookup` to show no
regression worse than 2x on the cache-miss radix path cannot be
verified in TASK-056 because `test/bench_route_lookup.cpp` does not
exist and `lookup_v2` is not yet wired into dispatch (that is
TASK-053's scope).
- Add an explicit DEFERRED note under TASK-056's acceptance criteria in
v2-deferred-backlog-plan.md recording the rationale and the handoff.
- Add a reminder under TASK-053's action items to verify the 2x budget
when creating bench_route_lookup.cpp.
- Update route-table.md to make the bench_route_lookup reference
prospective rather than dangling ("once TASK-053 lands").
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Validation loop produced 8 unworked majors and 40 minors after three iterations (10 critical/major findings fixed in iter 1, 1 in iter 2). The remaining majors are non-blocking judgment calls from the code-simplifier (dead alias variable, redundant collision-flag assignment in reject_terminus_collision) and minor follow-ups across agents. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address two code-simplifier majors carried in the unworked review report for TASK-056: - Remove the `opposite_is_prefix` alias variable; derive the `existing_kind` string from `want_is_prefix` directly with the inverse ternary. - Collapse the two-step `collision = true` if/else-if pattern in the `want_is_prefix=true` branch into a single boolean expression, making the symmetry with the `!want_is_prefix` branch visible and letting `collision` be `const`. No behaviour change. routing_regression (26/26, 99 checks) and webserver_register_path_prefix (19/19, 38 checks) still green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…guard Two security findings deferred during TASK-027 cleanup are now closed: - Switched the radix tree's per-segment child container from std::unordered_map to std::map (CWE-407 algorithmic-complexity DoS hardening; libc++ does not randomize unordered_map hashing). - Added an is_prefix_match guard at upsert_v2_param_route so prefix- vs-exact terminus collisions on the same canonical key are detected at registration time. bench_route_lookup verification is formally deferred to TASK-053.
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