v2.0: Modernization (M1-M6, 44 tasks)#374
Draft
etr wants to merge 508 commits into
Draft
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #374 +/- ##
==========================================
+ Coverage 68.03% 68.72% +0.68%
==========================================
Files 34 64 +30
Lines 1730 4057 +2327
Branches 697 1489 +792
==========================================
+ Hits 1177 2788 +1611
- Misses 80 357 +277
- Partials 473 912 +439
... and 21 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>
Major (both already addressed by earlier tasks, now marked):
- Item 1: redundant constants.hpp include in http_utils.cpp — already gone
- Item 2: explicit std::string{} wrapping in webserver.cpp — already gone
Applied in this batch:
- src/webserver.cpp: remove redundant #include "httpserver/constants.hpp"
(transitively available via create_webserver.hpp; item 25)
- src/httpserver/create_webserver.hpp: uint16_t → std::uint16_t for _port
field, constructor, and port() setters (item 5)
- src/httpserver/constants.hpp: trim verbose per-constant comments to single
focused lines; condense namespace comment; remove internal ticket refs
(items 8-10, 19-21); fix inaccurate "non-negative by construction" claim
(item 22); document METHOD_ERROR name-preservation rationale inline (11, 23)
- specs/architecture/05-cross-cutting.md: add constants.hpp to §5.5 header
layout table (item 4)
- specs/tasks/M1-foundation/TASK-006.md: clarify acceptance criterion grep
scope — exclude include guards, COMPARATOR, platform shims (items 27, 31)
- test/unit/constants_test.cpp: strengthen comment explaining intentional
static_assert / runtime duplication (items 15, 35)
Deferred: items 14/16/26/34 (framework-required empty set_up/tear_down),
28 (string_response string_view overload — separate task), 30 (sub-namespace
API decision), 32 (COMPARATOR/platform-shim cleanup — future task),
33 (negative-compile test — build system change).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All 26 items in specs/unworked_review_issues/2026-04-30_233954_task-001.md now have [x] with Status annotations: Fixed in this batch (329aad9): items 1, 2, 4, 9, 12, 15 - Item 1: IWYU clone switched to --depth 1 --single-branch, SHA logged, TODO added - Item 2: curl SHA logging + TODO added; libmicrohttpd already pinned (TASK-037) - Item 4: ChangeLog updated gcc >= 10 -> gcc >= 11 - Item 9: configure.ac comment added about AX_CXX_COMPILE_STDCXX injection - Item 12: CI matrix drop comments rewritten with "(were here)" placement hint - Item 15: README.CentOS-7 updated to gcc >= 11 Already addressed before 329aad9: items 3, 6, 7, 8, 13, 18, 19, 23, 24 - Items 3/6/7/18: performance/lint matrix already at gcc-14 - Item 8: noext/mandatory recommendation said "no change needed" - Item 13: CXXLAGS typo fixed in TASK-038 - Item 19: permissions block added in TASK-037 - Items 23/24: recommendations said "no action required" Deferred (no action possible/needed in this batch): items 5, 10, 11, 14, 16, 17, 20, 21, 22, 25, 26 - Items 5/17: pre-existing AM_CFLAGS pattern, explicitly out of scope - Item 10/16: README formatting/structure change (minor cosmetic, low priority) - Item 11: upstream vendored m4 macro; modifying testbody causes drift - Item 14/26: IWYU hardcoded -std=c++20 literal; non-trivial to derive at CI time - Item 20: GitHub Actions SHA pinning requires Renovate/Dependabot follow-up - Item 21: pre-existing CXXFLAGS quoting pattern; low risk on GitHub Actions - Item 22: CHECKSUMS sidecar for m4 vendor file; follow-up supply-chain task - Items 25: MinGW version check nice-to-have; AX_CXX_COMPILE_STDCXX gates it
…ew tests) Header changes (http_method.hpp): - Rephrase "consteval-friendly" comment → "constant-expression" (item 7) - Add inline comment to operator&(method, method) documenting singleton AND semantics — non-empty only when a == b (item 8) - Tighten static_assert from <= 32 to < 32; add UB explanation comment (items 9, 10, 27 — prevents potential shift-by-32 UB as enum grows) - Update method_bit() comment to accurately describe the invariant (item 10) Test file changes (http_method_test.cpp): - Tighten AC #2 static_assert bound to < 32 to match header (items 15, 22) - Add CRTP explanation comment to set_up/tear_down stubs (items 12, 20, 21) - Add test 14: compound_assign_method_set_rhs — exercises |=, &=, ^= with method_set RHS in isolation (item 14 / finding 33) - Add test 15: mixed_set_method_operators_are_commutative — covers (http_method op method_set) and (method_set op http_method) commutativity for |, &, ^ (finding 34) - Add test 16: algebraic_identity_and_annihilator_laws — s|empty==s, s&full==s, s^s==empty, s|full==full (finding 35) All 16 runtime tests pass (80 checks, 0 failures). Static asserts compile clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 major + 20 minor items marked [x] fixed; 12 minor items marked deferred. Fixes applied in fix/task-005-review-cleanup branch (2 commits). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
21 of 52 items marked [x] (addressed by 9db77d0): majors 6, 7 and minors 14, 19-22, 24-28, 34, 36, 40, 42, 47-52. Remaining 31 items marked deferred with reasons. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mark all 59 findings in the 2026-05-25_210827_task-048.md review file: Fixed by bbb1c3a (this batch): - findings 2, 3, 4, 13, 14, 15, 16 (all 7 remaining majors) - findings 24, 25, 27, 45, 46, 49, 56, 58 (8 minors) Already addressed by prior commits: - findings 5-12: TASK-048.md action items already [x] (2f6cb27/f9e095d) - finding 1: auth graduated to real hook in f9e095d; bbb1c3a added enforcement comment - finding 41: before_handler moved to finalize_answer in f9e095d, eliminating duplicate get_allowed_methods() call - finding 44: kHookSnapshotReserve removed in TASK-049 (hook_handle.cpp documents this) - finding 48: CWE-209 WARNING added to internal_error_page in 7973dc2 (TASK-036 review) - finding 50: fix-pass changes committed in f9e095d Deferred (minors, not addressed): - findings 17-23, 26, 28-35, 36-40, 42-43, 47, 51-55, 57, 59 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Stray changes from TASK-007 agent dispatch that landed in main worktree working tree (multi-agent worktree coordination artifact). Captures the agent's review-file markings and test-quality improvements.
# Conflicts: # src/detail/webserver_aliases.cpp
Major fixes: - Finding 1: add block comment explaining \ballow_ip\b word-boundary rationale in RENAME_PAIRS (code was already correct) - Finding 2: add hook_action to RELEASE_NOTES.md (five short-circuit- capable hook phases return hook_action) and to REQUIRED_V2_TOKENS; fix divergence from check-readme.sh; update comment Minor fixes: - Finding 3: rewrite Makefile.am check-local comment to list all pre-install static checks explicitly - Finding 4: add link to specs/architecture/13-documentation.md in RELEASE_NOTES.md "See also" section - Finding 5: add \bHAVE_BAUTH\b and \bHAVE_DAUTH\b to REQUIRED_V1_TOKENS - Finding 6: trim check-release-notes Makefile comment to single-line cross-reference - Finding 7: replace all [ \t] with [[:space:]] for POSIX portability in grep and awk patterns - Finding 8: add inline comment on disallow_ip explaining no \b needed - Findings 10/13: unify check_tokens_present error path through fail(); add fail-fast comment - Finding 11: add A2 comment acknowledging intentional partial no_* coverage - Finding 12: document case-insensitive matching in extract_section_body - Finding 16: add comment that REQUIRED_SECTIONS must remain static - Finding 17: add ERE-metacharacter warning above RENAME_PAIRS - Finding 18: document awk -v limitation in extract_section_body - Finding 19: expand container-getter list with get_path_pieces/get_files - Finding 20: add shoutCAST() preservation note below What's renamed table - Finding 21: add \bregister_resource\b to REQUIRED_V1_TOKENS - Finding 22: add \bhttpserver::constants\b to REQUIRED_V2_TOKENS - Finding 23: add threading and error propagation to REQUIRED_SECTIONS - Finding 24: add explicit CRLF guard after A1 existence check Already-fixed (no change needed): 9 (markdownlint 2>/dev/null), 25 (DEFAULT_WS_TIMEOUT) Deferred: 14 (low-priority param reorder), 15 (performance, no practical impact) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
23 of 25 issues addressed in fix/task-042-review-cleanup. Deferred: 14 (low-priority param reorder), 15 (performance/no practical impact). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- lambda_resource::invoke_(): add assert(slot) + NDEBUG-safe 500 fallback (CWE-617: assert compiled away; empty std::function would throw std::bad_function_call inside MHD callback = UB / crash in release) - lambda_resource: update render_* override comment to accurately describe that 7 overrides match 7 on_* entry points; render_connect/render_trace intentionally not overridden; is_allowed gate prevents dispatch reaching them - webserver_routes.cpp: rename fresh -> is_new_entry in on_methods_(); add scoped-block comment explaining lock-ordering rationale; add comment about !family arm equivalence in single_resource guard - test/unit/webserver_on_methods_test.cpp: replace std::string(kAllowPrefix).size() with std::strlen(kAllowPrefix) to avoid temporary string allocation - specs/architecture/04-components/route-table.md: add Implementation status note documenting TASK-025/TASK-027 staging - specs/architecture/11-decisions/DR-004.md: add "First realised in" note - specs/unworked_review_issues/2026-05-10_191459_task-025.md: mark all 40 items with [x]/[ ] inline status after review Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…seded) Cross-reference 2026-05-10_224500_task-027.md against 2026-05-10_230348_task-027.md: all 10 items (6 major, 4 minor) were addressed in the second-pass review or are explicitly deferred there. Mark every item [x] with the corresponding 2nd-pass item number and its disposition (fixed commit or deferred rationale). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…routes + http_utils + websocket
All findings are minor (0 critical, 0 major); they describe documentation nits in the new comment block and pre-existing out-of-diff inconsistencies in webserver_request.cpp comments. No behavioural concerns. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Documents the architectural rationale for keeping the no-op MHD_OPTION_UNESCAPE_CALLBACK registration — needed to suppress MHD's internal %HH decode so the per-connection PMR arena (TASK-072) and any user-registered unescaper hook own decoding end-to-end. The v0.99 bug framing is demoted to a historical footnote with a verified version cutoff (libmicrohttpd 0.9.64, 2019-06-09). No behavioural change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the #ifdef DEBUG guard around the raw request-body stdout
dump in src/detail/webserver_body_pipeline.cpp with a runtime opt-in
keyed on LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY. Default behaviour is
now silent on RELEASE *and* DEBUG builds alike -- the env var is the
only gate, so a debug build accidentally shipped to production cannot
leak credentials/PII unless the operator explicitly opted in.
When the env var is observed at webserver::start(), the library emits
a single SECURITY WARNING line to stderr (and to the user's log_error
callback when wired) naming the env var, the credential/PII risk, and
the docs pointer. Process-wide std::atomic<bool> flag means multiple
webservers in the same process produce exactly one stderr emission.
The runtime gate is cached in a function-local static (Magic Static)
so getenv() is read at most once per process; subsequent setenv()
calls are intentionally ignored (debug-knob semantics).
New documentation:
- docs/debug-env-vars.md - canonical home for LIBHTTPSERVER_*
debug knobs; describes effect, risk, startup signal, disable
procedure, and the spot-check that release-with-DDEBUG still
behaves correctly. Includes a checklist for future debug knobs.
- specs/architecture/10-observability.md cross-references the new
doc.
New tests (test/integ/):
- debug_dump_request_body_unset_test.cpp - asserts silence on both
stdout and stderr when the env var is unset; passes identically
on DEBUG and RELEASE builds.
- debug_dump_request_body_set_test.cpp - asserts the body dump
and one-shot SECURITY WARNING fire when the env var is set, and
pins the process-wide idempotence with a second webserver.
The two binaries are split because the function-local-static cache
means the first observation in the process locks in for the rest;
splitting gives each scenario a deterministic fresh process.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… issues Mark TASK-074 status as Done in the task file and milestone index, and record the 28 review findings (6 major, 22 minor) that were not actioned during this task so they can be picked up by a follow-up cleanup pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Extract the readiness helper introduced in TASK-049 from
hooks_handler_exception_chain.cpp into a new shared header
test/integ/server_ready.hpp (namespace httpserver_test) and migrate the
22 sibling hooks_*.cpp files (plus the source file) from the racy
std::this_thread::sleep_for(50ms) pattern to the deterministic
httpserver_test::wait_for_server_ready(PORT) call.
Probe strategy: the shared helper uses CURLOPT_CONNECT_ONLY (TCP-only).
This matters because a HEAD-probe variant — naive port of the
exception_chain inline helper — would have fired the HTTP-level hook
phases (request_received, route_resolved, before_handler,
after_handler, response_sent, request_completed) on the server under
test, breaking exact-count assertions in
hooks_response_sent_carries_status_bytes_timing,
hooks_per_route_order, hooks_route_resolved_miss_and_hit and
hooks_request_completed_fires_on_early_failure. The TCP-only probe
also works on banned-IP tests (hooks_accept_decision_banned,
hooks_accept_decision_throwing banned sub-case) because the kernel
completes the three-way handshake before MHD's policy_callback runs.
Also consolidates hooks_not_found_alias_test.cpp's near-duplicate
wait_for_server helper to the shared header (it was the source of the
TCP-only design adopted here).
Add scripts/check-server-ready-helper.sh, a bash + grep/awk regression
gate that scans test/integ/hooks_*.cpp for any bare server-ready
sleep_for call and exits 1 if found. The gate honours a per-line
'// NON-READINESS-SLEEP: <reason>' escape hatch for legitimate
non-readiness sleeps; the self-test (scripts/test_check_server_ready_helper.sh,
5 cases, modelled on test_check_warning_suppressions.sh) verifies the
detection logic, including the empty-watched-files edge case that the
${arr[@]+...} idiom guards against. The gate is wired into
check-local in Makefile.am (alongside lint-warning-suppressions), so
it runs on every CI lane via `make check`.
Removes the now-unused <thread> and <chrono> includes from every
migrated TU except hooks_per_route_concurrent_registration.cpp (which
still uses std::thread for its worker pool).
Verified locally: all 102 integ tests pass; the gate self-test
reports 5/5; the gate against the migrated tree returns 0
violations (against the pre-migration tree it returned 24, matching
the audit). Doxygen check-doxygen is failing on this branch
pre-existing (unrelated to TASK-075).
Closes TASK-075 (M7 - v2 Cleanup).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 major (test-quality-reviewer flags scripts/test_check_server_ready_helper.sh as unreachable from `make check` because it is neither in EXTRA_DIST nor wired into a check-local target) and 18 minor findings deferred for future cleanup. See specs/unworked_review_issues/2026-06-09_103302_task-075.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`test/integ/ws_start_stop.cpp` carried 33 `LT_CHECK_EQ(1, 1)` statements
that simulated a "skip" inside `try { ws.start } catch { ... return; }`
or `if (!has_gnutls_cli()) { ... return; }` blocks. On hosts without
GnuTLS or `gnutls-cli`, the entire TLS suite reduced to passing
tautological assertions — a build that silently lost TLS support would
still report green CI.
Added `LT_SKIP` / `LT_SKIP_IF` macros and `skip_unattended`
exception class to `test/littletest.hpp`. `test_runner` now tracks a
`skip_counter`, prints `-> N skipped`, and returns 77 (Automake's SKIP
code) when a binary's only outcomes are skips. Two new gates pin the
plumbing: `lint-no-tautological-asserts` (forbids regressing the
pattern in ws_start_stop.cpp) and `lint-littletest-skip-exit-code`
(asserts the exit-77 / mixed-pass+skip-0 semantics). Both wired into
`check-local`.
Replaced all 33 occurrences with one of:
(a) `LT_SKIP_IF(true, "TLS start failed: ...")` in the 11 TLS-start
catches; reports SKIP rather than tautological PASS.
(b) Typed `catch (const std::exception&)` + `is_psk_unsupported_error`
substring match in the 8 PSK catches: SKIP on
libmicrohttpd-built-without-PSK; `throw;` (FAIL) otherwise so
implementation-broken cases surface.
(c) Strengthened 4 tail-position liveness sentinels: 3 ×
`LT_CHECK_EQ(ws.is_running(), false)`; 1 ×
`LT_CHECK_NEQ(cmd_exit, 127)` (capturing `system()`'s result
instead of discarding it).
(d) Deleted 3 redundant tail-position tautological assertions.
The five `if (!has_gnutls_cli())` env-gates now use
`LT_SKIP_IF(!has_gnutls_cli(), "gnutls-cli binary not in PATH")`.
Wired CI: new `tls-no-cli` lane installs `libgnutls28-dev` WITHOUT
`gnutls-bin`; post-test verification step greps
`build/test/ws_start_stop.log` for >=5 `[SKIP]` markers and zero
`[CHECK/ASSERT FAILURE]` from the five `psk_connection_*` tests. A
sibling canary on the baseline `classic` lane asserts those tests do
NOT report SKIP when `gnutls-bin` IS present, catching detection
regressions.
Acceptance criteria:
* `grep -nE 'LT_CHECK_EQ\(1, *1\)' test/integ/ws_start_stop.cpp`
returns no matches.
* Local run with PATH masked to drop gnutls-cli: ws_start_stop emits
5 SKIPs (the psk_connection_* tests) and exits 0 with 112
successes + 0 failures preserved. With gnutls-cli on PATH the
binary runs all 50 tests (117 successes, 0 failures, 0 skips).
* `make check` in test/ stays green at 103/103 PASS.
Files:
* test/littletest.hpp — SKIP severity, exception class, runner
counter + exit-77 plumbing.
* test/integ/ws_start_stop.cpp — 33 replacements +
`is_psk_unsupported_error` helper.
* test/unit/littletest_skip_semantics_test.cpp — TDD entry-point
sentinel pinning the runtime semantics.
* scripts/check-no-tautological-asserts.sh,
scripts/test_check_littletest_skip_exit_code.sh — new gates,
wired into check-local.
* .github/workflows/verify-build.yml — `tls-no-cli` matrix lane +
verification steps.
* RELEASE_NOTES.md — "Test infrastructure" section on the new
skip semantics.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Apply iter-1 validation-fixer changes for the three majors raised by test-quality-reviewer: - Remove unreachable dead assertions in littletest_skip_semantics_test. - Tighten is_psk_unsupported_error coverage via direct unit assertions. - Replace silent always-green branches in IPv6/dual-stack success paths with LT_ASSERT_EQ on curl results. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Capture the residual minors and unfixed code-simplifier majors from the validation loop. No critical findings; 2 majors and 26 minors logged for future cleanup sweeps. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces ~25 LT_CHECK_EQ(1,1) tautologies in ws_start_stop.cpp with LT_SKIP / LT_SKIP_IF (new littletest macros, Automake exit-77 semantics) or typed std::exception catches that distinguish "config unsupported" from "config broken". Adds tls-no-cli CI matrix lane and a regression gate (lint-no-tautological-asserts) wired into make check-local. Includes iter-1 review-cleanup fixes (test-quality-reviewer majors) and 28 persisted unworked findings. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three whole-suite or near-whole-suite `#ifndef _WINDOWS` / `#ifndef DARWIN` blocks in test/integ/ were silently removing the library's Windows / Darwin portability claim from CI without any commentary on why. This change: - Adds `scripts/check-skip-rationales.sh` (lint) + `test_check_skip_rationales.sh` (10-case fixture) that enforce a `// reason:` comment within 5 lines of every `#ifndef _WINDOWS`, `#ifndef DARWIN`, or `#if !defined(_WINDOWS)` block under `test/integ/`. `#ifdef _WINDOWS` (additive coverage) is deliberately not gated. - Annotates every existing skip site (`threaded.cpp` x4, `ws_start_stop.cpp` wide skip and `custom_socket`, `authentication.cpp` digest-auth block, and `connection_state_body_residue_test.cpp`) with the required comment. - Adds `test/PORTABILITY.md` recording symptom / root cause / restoration plan per skip site; the `// reason:` lines point at the relevant section. - Adds a `ws_start_stop_suite::windows_smoke` test under `#ifdef _WINDOWS` to restore a single non-TLS HTTP GET round-trip on the MinGW64 / MSYS lanes — recovering the most valuable bit of Windows coverage without inheriting the TLS / IPv6 / SNI / PSK flake tracked in PORTABILITY.md. - Wires the lint into `verify-build.yml`'s existing `lint` lane and adds a Windows-lane assertion that grep's the test log for `windows_smoke` so a future build-flag tweak that suppresses the `#ifdef _WINDOWS` branch fails CI explicitly rather than silently erasing coverage. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…iew findings Flip TASK-077 status to Done in TASK-077.md and _index.md, check off the five action items, and log the 27 minor findings (0 critical, 0 major) surfaced by the validation loop for future cleanup sweeps. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…m skips Adds `scripts/check-skip-rationales.sh` lint (with a 10-case fixture) that fails CI if a `#ifndef _WINDOWS` / `#ifndef DARWIN` / `#if !defined(_WINDOWS)` block under `test/integ/` lacks a `// reason:` comment within 5 lines. Annotates every existing skip site, adds `test/PORTABILITY.md` (symptom / root cause / restoration plan per skip), and restores a single Windows non-TLS HTTP smoke test (`ws_start_stop_suite::windows_smoke`) to recover the most valuable bit of Windows coverage. Wires the lint into the verify-build.yml lint lane and adds a Windows-lane assertion that grep's the test log for `windows_smoke` so a future build-flag tweak that silently suppresses the `#ifdef _WINDOWS` branch fails CI. Includes the status flip to Done and 27 persisted minor review findings (0 critical, 0 major) for future cleanup sweeps. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eg test
Two HIGH audit findings in test/integ/basic.cpp:
(a) Two `/* ... */`-commented CONNECT-method test bodies in
basic_suite::complete and basic_suite::only_render, dating to
commit bd39cb4 (Nov 2017, "Avoid tests getting stack on CONNECT
requests"). The hang was a libcurl client-side artifact -
curl_easy_perform with CURLOPT_CUSTOMREQUEST "CONNECT" waits for
a tunnel that never comes from a plain HTTP server. Server-side
CONNECT dispatch IS exercised: webserver_request.cpp's methods[]
table maps the CONNECT wire token to render_connect, and the
render_connect signature is pinned by
test/unit/http_resource_test.cpp. Delete the dead bodies and
record the rationale as REGRESSION.md divergence #6.
(b) basic_suite::validator_builder only asserted "server boots with a
validator callback set" - the validator hook has been unwired
from the dispatch path since commit 9163a4f (Jan 2013,
"Eliminated unescaper and validator delegates"). The equivalent
compile-time pin already lives in
test/unit/create_webserver_test.cpp::builder_validator
(LT_CHECK_NOTHROW on the builder call). Delete the integ test
and note in RELEASE_NOTES.md that v2's request_received /
accept_decision hooks are the documented replacement for the
URL-veto use case the original validator was intended for.
No public surface change. The validator() builder method,
validator_ptr typedef, and get_request_validator() accessor remain
as inert v1 surface (separate task if anyone wants to actually
remove them). The basic test count drops by 1 (validator_builder
removed, 97 -> 96); the complete and only_render tests pass
unchanged. Full make check: 103/103 PASS.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
All 9 are minor (0 critical, 0 major) — captured for future cleanup sweeps, none block merge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…or integ test Deletes two `/* ... */`-commented CONNECT-method test bodies in `test/integ/basic.cpp` (`basic_suite::complete` and `basic_suite::only_render`, dating to bd39cb4, Nov 2017) — the hang was a libcurl client-side artifact, not a server bug. Server-side CONNECT dispatch remains pinned by `test/unit/http_resource_test.cpp::render_connect_returns_by_value` and recorded as REGRESSION.md divergence #6. Also deletes `basic_suite::validator_builder`, which only asserted "server boots with a validator callback set" — the validator hook has been unwired from the dispatch path since 9163a4f (Jan 2013). Equivalent compile-time coverage already lives in `test/unit/create_webserver_test.cpp::builder_validator`. RELEASE_NOTES records that v2's `request_received` / `accept_decision` hooks are the documented replacement for the URL-veto use case. No public surface change. Basic integ test count drops by 1 (97 -> 96); full `make check` 103/103 PASS. Includes the status flip to Done and 9 persisted minor review findings (0 critical, 0 major) for future cleanup sweeps. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Six v2 digest-auth integ tests (digest_auth[_wrong_pass],
digest_auth_with_ha1_{md5,sha256}[_wrong_pass], digest_user_cache_with_auth)
were previously observationally indistinguishable from static-challenge
pins because libcurl's CURLAUTH_DIGEST owned the nonce/opaque computation.
That hid the HA1-precomputed AC: libcurl always recomputes HA1 from the
cleartext password it was given, so "server validates against configured
HA1, not against re-derived MD5/SHA-256 of cleartext" was unobservable.
Add a header-only RFC 7616 client helper at test/integ/digest_client.hpp
with inline public-domain MD5 (RFC 1321) and SHA-256 (FIPS 180-4), a
WWW-Authenticate parser, and cleartext + precomputed-HA1 response-compute
paths. Pin the helper in test/unit/digest_client_self_test.cpp against
the FIPS canonical "abc"/empty vectors and the RFC 7616 §3.9.1 worked
example for both MD5 (8ca523f5...) and SHA-256 (753927fa...).
Convert each of the six tests to a two-round flow: round 1 captures the
challenge via CURLOPT_HEADERFUNCTION, round 2 ships a hand-built
`Authorization: Digest ...` header. Wrong-password variants now assert
401 on the SECOND request, not on the initial challenge. HA1 variants
sign with the configured 16/32-byte HA1 directly -- cleartext never
leaves the test, so a 200 response proves the server validates against
the configured HA1. The wrong-HA1 negative variants (signing with
md5/sha256("user:realm:totallywrong")) strengthen the proof.
Migrate digest_user_cache_resource from the legacy `unauthorized("Digest",
"testrealm", "FAIL")` static overload to the RFC 7616 digest_challenge
factory so the handshake can complete; digest_user_cache_with_auth now
asserts "USER:testuser" (reaching the cache-hit code path).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…teg tests Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…t 41 unworked review findings Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…0x p95 Restores meaningful regression bite on the adversarial_segments registration latency gate (sub-test C of threadsafety_stress) without flaking on CI scheduler noise. Stabilisation stack applied: * Per-thread sample buffers eliminate the previous shared std::mutex-guarded std::vector<int64_t>::push_back from the timing window. Lock-wait jitter from prior iterations no longer leaks into subsequent samples' cache lines. * New HTTPSERVER_STRESS_PIN_CPU env knob (Linux only) pins the four writer threads to a single CPU via pthread_setaffinity_np. Counter- intuitive but correct: writers serialise on route_table_mutex_ anyway, and single-CPU placement eliminates cross-CPU cache misses on radix-tree node memory. Off by default; macOS / Windows: no-op. * New HTTPSERVER_STRESS_REPEATS env knob drives N back-to-back sampling rounds per test invocation, printing one [STATS] line per round. Used to build per-lane noise-floor CDFs without needing to rerun the full make-check N times. Capped at 200. When N>1 the gate is checked against the worst-observed p95 across all rounds. Gate redesign: * Statistic switched from p99 to p95. p99 = top 150 samples on a 15 000-op run; a single 1 ms OS-scheduler preemption spike against a ~10 us median gives a 100x ratio that is purely environmental. p95 = top 750 samples and is robust against single spikes while still catching algorithmic regressions (an O(n) traversal at 15k items would shift the entire upper quartile). p99 is still printed in the [STATS] line as a forensic diagnostic. * Threshold ratio lowered from 100x to 20x. Even with the stabilisation stack in place, the dominant noise floor on a quiet Apple Silicon laptop is legitimate route_table_mutex_ contention, not OS noise. 10x is infeasible without rewriting the registration lock strategy (out of scope). 20x gives ~50% headroom over the worst observed local 10-round sweep (13.4x), is still 5x tighter than the pre-TASK-080 gate, and restores real bite against algorithmic regressions. Documentation: * test/PERFORMANCE.md gains a "Methodology - threadsafety_stress adversarial_segments latency gate" section with the gate table, the per-statistic noise rationale (why p95), the per-ratio rationale (why 20x), and the HTTPSERVER_STRESS_REPEATS workflow for re-measuring on a flaky CI lane. * Inline test comment updated to point at the methodology section and record both design choices (statistic switch, threshold choice) with their measurement evidence. Acceptance criterion notes: * The TASK-080 "no flake in 50 CI runs across the matrix" criterion cannot be enforced at PR-time. Proxy used: local 10-round sweep (worst p95 ratio 13.4x against the 20x gate, ~50% headroom). Post-merge monitoring window covered in PERFORMANCE.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ild-debug/ - Flip Status from Backlog to Done in TASK-080.md - Tick all four action items; annotate item 3 noting 10× was infeasible (gate set at 20× p95 with measurement data in test/PERFORMANCE.md) - Flip TASK-080 row in _index.md from Backlog to Done - Add build-debug/ to .gitignore (developer noise-floor measurement dir) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dd rounds_ran gate Three review findings addressed: 1. Off-by-one in percentile index (performance-reviewer-iter1-1): Replace float-truncation `sorted[n * 0.95]` with ceiling integer arithmetic `sorted[min((n*95+99)/100, n-1)]` for p95, p99, p999. Prevents systematic sub-percentile samples on edge-case corpus sizes. 2. Document round-robin merge precondition (test-quality-reviewer-iter1-1, test-quality-reviewer-iter1-3, performance-reviewer-iter1-2): Add inline comments explaining: (a) the early-stop skew risk when per-thread buffers have wildly different sizes, and (b) that the "first quarter = warmup" is a statistical approximation under concurrent startup, not a wall-clock guarantee. Notes that the approximation is conservative (inflated warmup_median makes the gate stricter, not looser). 3. Explicit gate when rounds_ran == 0 (test-quality-reviewer-iter1-4): Unconditionally assert `LT_CHECK(rounds_ran > 0)` and emit a [WARNING] log with actionable instructions when the latency gate was never exercised, replacing the previous silent pass. All 104 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ector Six action items, six cycles: 1. New webserver_ws_available_test.cpp: HAVE_WEBSOCKET-on companion to webserver_ws_unavailable_test (which is empty on classic / flag-invariance-on lanes). Pins features().websocket==true and the throw-type contrast: null unique_ptr/shared_ptr → invalid_argument (NOT feature_unavailable). Body is empty on HAVE_WEBSOCKET-off lanes where the off-path contracts live in the unavailable TU. 2. New webserver_dauth_available_test.cpp: same paired pattern for HAVE_DAUTH. Pins features().digest_auth==true and that digest_auth(true)/digest_auth(false) construct cleanly. Empty body on HAVE_DAUTH-off (flag-invariance-off + Windows). 3. webserver_register_ws_smartptr_test: added HAVE_WEBSOCKET-off runtime block (was previously runtime-empty on off lanes — only the compile-time signature asserts fired). Pins features().websocket ==false and null unique_ptr → feature_unavailable (not invalid_argument — the throw-type contrast against the HAVE_WEBSOCKET-on contract). 4. http_request_operator_stream_test: split credential-redaction into HAVE_BAUTH-on (existing) and HAVE_BAUTH-off (new) variants. The off variant pins that get_user/get_pass return empty (admin/hunter2 never leak) AND that Authorization-class header and cookie redaction are independent of HAVE_BAUTH (the dumpers are unconditional). 5. body / http_response_factories / iovec_entry Windows pipe/iovec gates: documented the gap in test/PORTABILITY.md under a new "Skipped in test/unit/" section (branch B per the plan). Each #ifndef _WIN32 block gets a // reason: comment plus a PORTABILITY.md entry; the iovec_entry POSIX-iovec gate is structurally unfixable (no Windows equivalent), and pipe_body/file_body Windows ports are flagged as a future follow-up. MHD_IoVec bridge test is unconditional and covers the actual production cast path. 6. header_hygiene_test pthread detector: deleted (DELETE-PATH per the plan's investigation gate). Both libc++ and libstdc++-with-threads unconditionally pull <pthread.h> in from any STL container header. The libhttpserver public surface uses STL containers, so the detector cannot fire on any supported CI lane without rewriting the public surface to drop std::string / std::vector / std::map. The Makefile.am HEADER_HYGIENE_FORBIDDEN comment is updated and a RELEASE_NOTES.md entry under "Test infrastructure" records the deletion rationale. The remaining hygiene sentinels (microhttpd, gnutls, sys/socket, sys/uio) still fire on every lane. Acceptance criteria: - AC-1 (every unit suite exercises non-trivial code on at least one CI lane): satisfied by the paired ws/dauth on-path TUs + the new smartptr off-path block + the new operator_stream off-path block. - AC-2 (pthread detector works or is deleted): deleted with rationale. - AC-3 (Windows lane runs body/response/iovec variants OR PORTABILITY.md records the gap): PORTABILITY.md records all three gaps with Symptom / Root cause / Restoration plan. - Typecheck passes: verified by cross-flag compile of each touched TU under HAVE_*=on and HAVE_*=off. - Tests pass: all touched unit tests exit 0; no regressions in the rest of the unit suite. (The 15 integ FAILs in local make check are the pre-existing thread_safety / port-8080 flake on feature/v2.0, unrelated to TASK-081.) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ad detector Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integration branch for the v2.0 modernization effort. Tasks land here individually (one merge commit per task) so the full v2.0 ships as a single reviewable PR.
This PR will remain draft until all milestones are complete.
Milestones
Specs live under
specs/(product_specs, architecture, tasks).Merged tasks
Test plan
Per-task validation runs through the groundwork validation loop on each task branch before merging here. Pre-merge of v2.0 to
master:./configure && makeclean on macOS (Apple Clang) and Linux (recent GCC)make checkgreen-std=c++(11|14|17)regressions in tree🤖 Generated with Claude Code