TASK-050: housekeeping — mark Done, persist review notes (0 critical, 0 major, 48 minor)

Apply selected post-review minor fixes:
- CWE-117: sanitize control chars in log_access alias path/method (and
  in examples/clf_access_log.cpp) to prevent log-line injection.
- Perf: skip steady_clock::now() in fire_response_sent_gated when only
  the log_access alias slot fires (alias does not read elapsed).
- Doxygen block on create_webserver::log_access() pointing users to
  add_hook(response_sent, ...) for structured ctx.
- Architecture doc: update hooks.md firing-site references to the new
  webserver_request.cpp / webserver_callbacks.cpp split.
- Test wiring: add -lmicrohttpd to hooks_log_access_alias_slot LDADD.

Persist full reviewer findings under specs/unworked_review_issues/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: etr

examples/clf_access_log.cpp

@@ -52,6 +52,7 @@
 #include <functional>
 #include <memory>
 #include <string>
+#include <string_view>
 
 #include <httpserver.hpp>
 
@@ -66,6 +67,19 @@ class hello_resource : public hs::http_resource {
     }
 };
 
+// SECURITY (CWE-117): sanitize a string_view for CLF output.
+// Replace ASCII control characters (< 0x20 or == 0x7F) with '-' to
+// prevent a client from injecting additional log lines by embedding
+// newlines or carriage-returns in the request path or method.
+std::string sanitize_clf(std::string_view sv) {
+    std::string out;
+    out.reserve(sv.size());
+    for (unsigned char c : sv) {
+        out += (c < 0x20 || c == 0x7f) ? '-' : static_cast<char>(c);
+    }
+    return out;
+}
+
 void emit_clf_line(const hs::response_sent_ctx& ctx) {
     std::time_t now = std::time(nullptr);
     char ts[32];
@@ -81,8 +95,9 @@ void emit_clf_line(const hs::response_sent_ctx& ctx) {
                      ctx.elapsed).count();
     std::string method, path;
     if (ctx.request != nullptr) {
-        method = std::string(ctx.request->get_method());
-        path = std::string(ctx.request->get_path());
+        // Sanitize before embedding in the CLF line to prevent log injection.
+        method = sanitize_clf(ctx.request->get_method());
+        path   = sanitize_clf(ctx.request->get_path());
     }
     std::printf("- - - [%s] \"%s %s HTTP/1.1\" %d %zu %lld\n",
                 ts,

specs/architecture/04-components/hooks.md

@@ -14,8 +14,8 @@
 | `before_handler` | `webserver.cpp:dispatch_resource_handler` — start | **yes** | yes |
 | `handler_exception` | `webserver.cpp:dispatch_resource_handler` — each catch arm | **yes** (maps exception to response) | yes |
 | `after_handler` | between handler return and `materialize_and_queue_response` | **yes** (replaces response) | yes |
-| `response_sent` | `webserver.cpp:materialize_and_queue_response` — post `MHD_queue_response` | no | yes |
-| `request_completed` | `webserver.cpp:request_completed` — NOTIFY_COMPLETED | no | yes |
+| `response_sent` | `webserver_request.cpp:materialize_and_queue_response` — post `MHD_queue_response` | no | yes |
+| `request_completed` | `webserver_callbacks.cpp:request_completed` — NOTIFY_COMPLETED | no | yes |
 | `connection_closed` | `webserver.cpp:connection_notify` — NOTIFY_CLOSED | no | no |
 
 **Implementation.** Each phase has its own `std::vector<std::function<...>>` in `webserver_impl`, guarded by a single `std::shared_mutex hook_table_mutex_`. A per-phase `std::atomic<bool> any_hooks_[hook_phase::count_]` flag short-circuits the dispatch site to a relaxed atomic load and a compare-with-zero when no subscribers exist — the only hook-related cost on the hot request path for a server with zero hooks registered.

specs/tasks/M5-routing-lifecycle/TASK-050.md

@@ -8,11 +8,11 @@
 Wire the three tail-end phases. `after_handler` is the post-handler short-circuit (it can REPLACE the in-flight response). `response_sent` is the observation point that finally has the response, status, byte-count, and timing — the data #281 and #69 have been asking for. `request_completed` is the unconditional final hook. Convert `log_access` into the documented alias.
 
 **Action Items:**
-- [ ] Fire `after_handler` between `dispatch_resource_handler` and `materialize_and_queue_response` in `webserver_impl::finalize_answer` (`webserver.cpp:2444` → `webserver.cpp:2449`). Context: `after_handler_ctx { const http_request& req; http_response& resp; }` — mutable response reference so a hook can `resp.with_header(...)` in place. Short-circuit semantics: a hook returning `hook_action::respond_with(r2)` REPLACES `mr->response_` with `r2` and skips remaining hooks at the phase. Subsequent hooks at later phases (`response_sent`, `request_completed`) still fire on the replaced response.
-- [ ] Fire `response_sent` immediately after the `MHD_queue_response` call in `webserver_impl::materialize_and_queue_response` (`webserver.cpp:2421`), BEFORE `MHD_destroy_response`. Context: `response_sent_ctx { const http_request& req; const http_response& resp; int status; std::size_t bytes_queued; std::chrono::nanoseconds elapsed; }`. `elapsed` is measured from `answer_to_connection`'s first invocation for this `mr` (timestamp captured in `modded_request` at TASK-045 time).
-- [ ] Fire `request_completed` from `webserver_impl::request_completed` (`webserver.cpp:1253`), BEFORE the `delete static_cast<detail::modded_request*>(*con_cls)` step (otherwise the context fields would dangle). Context: `request_completed_ctx { const http_request& req; const http_response* resp; bool succeeded; }`. `resp` is nullable — on early failures (`accept_decision` rejection, etc.) there may be no response object.
-- [ ] **Alias: `log_access(fn)`.** Internally register a `response_sent` hook that formats the request line as today and invokes `fn(line)`. Re-registration replaces. Doxygen notes it as an alias and recommends `add_hook(hook_phase::response_sent, ...)` for users who want the structured context (status, bytes, timing — what #281 and #69 actually asked for).
-- [ ] `examples/clf_access_log.cpp`: a Common-Log-Format access logger as a `response_sent` hook, demonstrating that the long-requested CLF / `time-taken` format is now writable in user code without a library change. Closes issues #281 and #69 from the user-facing-doc perspective.
+- [x] Fire `after_handler` between `dispatch_resource_handler` and `materialize_and_queue_response` in `webserver_impl::finalize_answer` (implemented in `src/detail/webserver_finalize.cpp:fire_after_handler_gated`, called from `src/detail/webserver_request.cpp:finalize_answer`). Context: `after_handler_ctx { const http_request& req; http_response& resp; }` — mutable response reference so a hook can `resp.with_header(...)` in place. Short-circuit semantics: a hook returning `hook_action::respond_with(r2)` REPLACES `mr->response_` with `r2` and skips remaining hooks at the phase. Subsequent hooks at later phases (`response_sent`, `request_completed`) still fire on the replaced response.
+- [x] Fire `response_sent` immediately after the `MHD_queue_response` call in `webserver_impl::materialize_and_queue_response` (`src/detail/webserver_request.cpp:materialize_and_queue_response`), BEFORE `MHD_destroy_response`. Context: `response_sent_ctx { const http_request& req; const http_response& resp; int status; std::size_t bytes_queued; std::chrono::nanoseconds elapsed; }`. `elapsed` is measured from `answer_to_connection`'s first invocation for this `mr` (timestamp captured in `modded_request` at TASK-045 time).
+- [x] Fire `request_completed` from `webserver_impl::request_completed` (`src/detail/webserver_callbacks.cpp`), BEFORE the `delete static_cast<detail::modded_request*>(*con_cls)` step (otherwise the context fields would dangle). Context: `request_completed_ctx { const http_request& req; const http_response* resp; bool succeeded; }`. `resp` is nullable — on early failures (`accept_decision` rejection, etc.) there may be no response object.
+- [x] **Alias: `log_access(fn)`.** Internally registers a `response_sent` hook via dedicated alias slot (`webserver_impl::log_access_alias_`) that formats the request line and invokes `fn(line)`. Re-registration replaces. Control characters are sanitized (CWE-117). Doxygen block added to `create_webserver::log_access()` noting it as an alias and recommending `add_hook(hook_phase::response_sent, ...)` for structured context (status, bytes, timing — what #281 and #69 actually asked for).
+- [x] `examples/clf_access_log.cpp`: a Common-Log-Format access logger as a `response_sent` hook, demonstrating that the long-requested CLF / `time-taken` format is now writable in user code without a library change. Closes issues #281 and #69 from the user-facing-doc perspective.
 
 **Dependencies:**
 - Blocked by: TASK-045
@@ -32,4 +32,4 @@ Wire the three tail-end phases. `after_handler` is the post-handler short-circui
 **Related Decisions:** DR-012, §4.10
 **Resolves:** Issues #281, #69
 
-**Status:** Not Started
+**Status:** Done

specs/tasks/_index.md

@@ -132,7 +132,7 @@ Nominally: **13 sequential tasks**, each S–XL. Most other tasks parallelize of
 | TASK-047 | Fire `request_received` and `body_chunk` (pre-handler short-circuit) | M5 | Done | TASK-045 |
 | TASK-048 | Fire `route_resolved` and `before_handler`; wire 404/405/auth aliases | M5 | Done | TASK-045, TASK-027, TASK-031 |
 | TASK-049 | Fire `handler_exception`; wire `internal_error_handler` alias | M5 | Done | TASK-045, TASK-031 |
-| TASK-050 | Fire `after_handler` (post-handler short-circuit), `response_sent`, `request_completed`; wire `log_access` alias | M5 | Not Started | TASK-045 |
+| TASK-050 | Fire `after_handler` (post-handler short-circuit), `response_sent`, `request_completed`; wire `log_access` alias | M5 | Done | TASK-045 |
 | TASK-051 | Per-route hooks (`http_resource::add_hook`) | M5 | Not Started | TASK-045, TASK-048, TASK-049, TASK-050 |
 | TASK-052 | Hook bus documentation, examples, benchmark, stress-test extension (touches back into TASK-040/041/042/043) | M5 | Not Started | TASK-045, TASK-046, TASK-047, TASK-048, TASK-049, TASK-050, TASK-051 |
 | TASK-037 | CI test for build-flag invariance | M6 | Done | TASK-034 |