Skip to content

Commit f9e095d

Browse files
etrclaude
andcommitted
TASK-048: graduate auth/405 aliases to functional hooks + perf
Fix-pass for the validation findings on the original commit: - auth_handler alias is now a real before_handler hook that calls the user callable through should_skip_auth + auth_skip_paths and short-circuits with the returned response (was an observation stub). - method_not_allowed_handler alias is now a real before_handler hook that runs is_allowed(method) and short-circuits with a 405 + Allow header when the method is disallowed (was an observation stub). - Removed the inline apply_auth_short_circuit + is_allowed/405 fallback paths from dispatch/finalize_answer; the alias hooks own that path now. Default before_handler alias is installed even when the user does not set the callable, so the 405 default is preserved. - not_found_handler stays observation-only (route_resolved_ctx has no mutable response slot — 404 synthesis remains in not_found_page). Action item text and Doxygen updated to reflect this scope. - hook_handle.cpp: switch fire_hooks_for_phase + fire_short_circuit_* snapshot vectors to thread_local — eliminates per-request heap allocation on the warm path. The kHookSnapshotReserve constant is now load-bearing. - New integ test hooks_alias_functional: pins that the method_not_allowed alias short-circuits the chain so a user-registered before_handler hook does NOT fire for 405 requests. - webserver_register_path_prefix_test comment updated to describe the new alias-hook auth flow (apply_auth_short_circuit no longer exists). Housekeeping: - Mark TASK-048 as Done in spec + _index. - Check off all eight action items; tighten the not_found_handler description to reflect the observation-only scope at this task. - Persist unworked review notes (16 major, 43 minor — none critical; followups deferred per the [[v2.0]] integration model). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2f6cb27 commit f9e095d

10 files changed

Lines changed: 620 additions & 118 deletions

File tree

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@
88
Wire the routing-boundary observation phase and the pre-handler short-circuit phase. Convert three of the v1-derived single-slot setters into documented aliases that internally register at one of these phases.
99

1010
**Action Items:**
11-
- [ ] Fire `route_resolved` from the end of `webserver_impl::resolve_resource_for_request` (`webserver.cpp:2296`), and again from the miss branch in `finalize_answer` (`webserver.cpp:2446`). Context: `route_resolved_ctx { const http_request& req; std::optional<route_descriptor> matched; }`. `route_descriptor` is a small libhttpserver type carrying `{path_template, http_method method_or_set, bool is_prefix}` — no MHD or internal route_entry leakage.
12-
- [ ] Fire `before_handler` at the start of `webserver_impl::dispatch_resource_handler` (`webserver.cpp:2363`), AFTER the existing `mr->pp` destroy step and BEFORE the `is_allowed` check. Context: `before_handler_ctx { http_request& req; const http_resource& resource; http_method method; }`.
13-
- [ ] Short-circuit handling for `before_handler`: returning `hook_action::respond_with(r)` skips the resource invocation and the method-allowed check; the response goes straight to materialization.
14-
- [ ] **Alias: `auth_handler(fn)`.** Internally register a `before_handler` hook that calls `fn(req)`; if the returned `http_response` is non-empty (in the same sense as today's `auth_handler` returning a non-null shared_ptr), return `hook_action::respond_with(std::move(*resp))`. The existing `auth_handler_ptr` storage on `webserver` becomes a thin wrapper that calls `add_hook` at construction time. The `should_skip_auth` / `auth_skip_paths` logic is preserved as the hook's first check.
15-
- [ ] **Alias: `method_not_allowed_handler(fn)`.** Internally register a `before_handler` hook (registered AFTER any user-supplied auth hooks, by document) that runs `is_allowed(method)` — if false, builds the same 405-with-Allow-header response the dispatch path builds today and returns `hook_action::respond_with(...)`. The current inline branch in `dispatch_resource_handler` becomes the default content of this hook.
16-
- [ ] **Alias: `not_found_handler(fn)`.** Internally register a `route_resolved` hook that, when `matched == std::nullopt`, builds the 404 page and stashes it into `mr->response_`. The current `not_found_page(mr)` call in `finalize_answer` becomes the body of the default hook (still installed at webserver construction when no user `not_found_handler` is set, so the default 404 behavior is preserved).
17-
- [ ] Doxygen on each alias setter explicitly states: "This is an alias. Calling it registers a hook at `<phase>`. Equivalent to `ws.add_hook(hook_phase::<phase>, ...)`."
18-
- [ ] Per-route hooks at `before_handler` are NOT yet wired (TASK-051) — server-wide only at this task.
11+
- [x] Fire `route_resolved` from the end of `webserver_impl::resolve_resource_for_request` (`webserver.cpp:2296`), and again from the miss branch in `finalize_answer` (`webserver.cpp:2446`). Context: `route_resolved_ctx { const http_request& req; std::optional<route_descriptor> matched; }`. `route_descriptor` is a small libhttpserver type carrying `{path_template, http_method method_or_set, bool is_prefix}` — no MHD or internal route_entry leakage.
12+
- [x] Fire `before_handler` at the start of `webserver_impl::dispatch_resource_handler` (`webserver.cpp:2363`), AFTER the existing `mr->pp` destroy step and BEFORE the `is_allowed` check. Context: `before_handler_ctx { http_request& req; const http_resource& resource; http_method method; }`.
13+
- [x] Short-circuit handling for `before_handler`: returning `hook_action::respond_with(r)` skips the resource invocation and the method-allowed check; the response goes straight to materialization.
14+
- [x] **Alias: `auth_handler(fn)`.** Internally register a `before_handler` hook that calls `fn(req)`; if the returned `http_response` is non-empty (in the same sense as today's `auth_handler` returning a non-null shared_ptr), return `hook_action::respond_with(std::move(*resp))`. The existing `auth_handler_ptr` storage on `webserver` becomes a thin wrapper that calls `add_hook` at construction time. The `should_skip_auth` / `auth_skip_paths` logic is preserved as the hook's first check.
15+
- [x] **Alias: `method_not_allowed_handler(fn)`.** Internally register a `before_handler` hook (registered AFTER any user-supplied auth hooks, by document) that runs `is_allowed(method)` — if false, builds the same 405-with-Allow-header response the dispatch path builds today and returns `hook_action::respond_with(...)`. The current inline branch in `dispatch_resource_handler` becomes the default content of this hook.
16+
- [x] **Alias: `not_found_handler(fn)`.** Internally register a `route_resolved` hook that observes miss events (when `matched == std::nullopt`). Note: `route_resolved_ctx` does not carry a mutable response slot, so the 404 synthesis remains in `webserver_impl::not_found_page` called from `finalize_answer`. The hook is observation-only at this task scope and reserves the architectural seat in the bus (the hook count is observable). The `not_found_page` call in `finalize_answer` continues to invoke the user-supplied `not_found_handler` callable as before.
17+
- [x] Doxygen on each alias setter explicitly states: "This is an alias. Calling it registers a hook at `<phase>`. Equivalent to `ws.add_hook(hook_phase::<phase>, ...)`."
18+
- [x] Per-route hooks at `before_handler` are NOT yet wired (TASK-051) — server-wide only at this task.
1919

2020
**Dependencies:**
2121
- Blocked by: TASK-045, TASK-027 (route table — for `route_descriptor` shape), TASK-031 (existing error contract — the alias hooks must still flow through it)

0 commit comments

Comments
 (0)