Skip to content

Commit 91e9126

Browse files
committed
TASK-051: per-route hooks via http_resource::add_hook
Add a per-resource scope to the hook bus, restricted to the five post-route-resolution phases (before_handler, handler_exception, after_handler, response_sent, request_completed). Other phases throw std::invalid_argument naming the constraint. Storage uses a PIMPL (detail::resource_hook_table) reached through a single std::shared_ptr<> on http_resource (lazily allocated on first add_hook), so resources that never register a hook pay only the 16-byte pointer plus a null-check on the dispatch hot path. The sizeof cap on http_resource is bumped accordingly (vptr + shared_ptr + method_set + padding); bench_sizeof_http_resource and the in-header static_assert reflect the new cap. The per-route chain fires at each of the five applicable phases AFTER the server-wide chain and BEFORE checking for a server-wide short-circuit result -- if server-wide short-circuited, the per-route chain is skipped (response is already fixed). modded_request carries a weak_ptr<http_resource> populated in finalize_answer once the route resolves; fire_response_sent_gated and fire_request_completed_gated lock() it to fire the per-route chain after the server-wide one. If the resource was unregistered between dispatch and completion, lock() returns null and the per-route chain is naturally skipped. hook_handle gains a weak_ptr<detail::resource_hook_table> so per-route handles drain through the table's writer lock; if the resource is destroyed before the handle, remove() is a no-op. The handle size cap is bumped from 32 to 48 in the unit shape test. Lock order documented in §5.6: route_table_mutex_ -> resource hook_table_mutex_ -> server-wide hook_table_mutex_. Exercised by hooks_per_route_concurrent_registration under TSan. Carved out of webserver_request.cpp: - fire_before_handler_gated as a member on webserver_impl (definition in webserver_finalize.cpp alongside the other gated-fire helpers) so finalize_answer stays under CCN_MAX after the per-route branch doubles the local branch count. Carved out of hook_handle.cpp: - remove_per_route static helper for the per-route remove() branch so the host function stays at its pre-task CCN. Five new integ tests + the unit hook_api_shape pin extension cover: - invalid-phase rejection - server-wide-before-per-route ordering on response_sent - different early-413 size policies on two endpoints - resource-destroyed-before-handle no-op + no UAF (ASan) - concurrent registration on resource R from inside a handler on resource Q (TSan) Closes TASK-051.
1 parent 873359e commit 91e9126

21 files changed

Lines changed: 1954 additions & 153 deletions

src/Makefile.am

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ lib_LTLIBRARIES = libhttpserver.la
2525
# builds. The WS-off branch in websocket_handler.cpp provides stub
2626
# definitions (every member throws feature_unavailable except is_valid()
2727
# which returns false).
28-
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/http_request_impl.cpp detail/http_request_impl_tls.cpp detail/webserver_setup.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_finalize.cpp
28+
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp peer_address.cpp resource_hook_table.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/http_request_impl.cpp detail/http_request_impl_tls.cpp detail/webserver_setup.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_finalize.cpp
2929
# noinst_HEADERS: shipped in the tarball but NEVER installed under $prefix/include.
3030
# Detail headers (httpserver/detail/*.hpp) live here so they cannot leak to
3131
# downstream consumers — the public surface comes in through <httpserver.hpp>.
32-
noinst_HEADERS = httpserver/string_utilities.hpp httpserver/detail/modded_request.hpp httpserver/detail/http_endpoint.hpp httpserver/detail/body.hpp httpserver/detail/webserver_impl.hpp httpserver/detail/webserver_impl_dispatch.hpp httpserver/detail/connection_state.hpp httpserver/detail/http_request_impl.hpp httpserver/detail/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/radix_tree.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp gettext.h
32+
noinst_HEADERS = httpserver/string_utilities.hpp httpserver/detail/modded_request.hpp httpserver/detail/http_endpoint.hpp httpserver/detail/body.hpp httpserver/detail/webserver_impl.hpp httpserver/detail/webserver_impl_dispatch.hpp httpserver/detail/connection_state.hpp httpserver/detail/http_request_impl.hpp httpserver/detail/resource_hook_table.hpp httpserver/detail/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/radix_tree.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp gettext.h
3333
nobase_include_HEADERS = httpserver.hpp httpserver/body_kind.hpp httpserver/constants.hpp httpserver/create_webserver.hpp httpserver/create_test_request.hpp httpserver/webserver.hpp httpserver/webserver_routes.hpp httpserver/webserver_websocket.hpp httpserver/webserver_hooks.hpp httpserver/websocket_handler.hpp httpserver/http_utils.hpp httpserver/ip_representation.hpp httpserver/file_info.hpp httpserver/http_request.hpp httpserver/http_request_auth.hpp httpserver/http_response.hpp httpserver/http_resource.hpp httpserver/feature_unavailable.hpp httpserver/iovec_entry.hpp httpserver/http_arg_value.hpp httpserver/http_method.hpp httpserver/hook_phase.hpp httpserver/hook_action.hpp httpserver/hook_handle.hpp httpserver/hook_context.hpp
3434

3535
AM_CXXFLAGS += -fPIC -Wall

src/detail/webserver_dispatch.cpp

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
#include "httpserver/detail/http_endpoint.hpp"
6666
#include "httpserver/detail/lambda_resource.hpp"
6767
#include "httpserver/detail/modded_request.hpp"
68+
#include "httpserver/detail/resource_hook_table.hpp"
6869
#include "httpserver/http_request.hpp"
6970
#include "httpserver/http_resource.hpp"
7071
#include "httpserver/http_response.hpp"
@@ -379,31 +380,45 @@ void handle_dispatch_exception(
379380
webserver_impl* impl,
380381
detail::modded_request* mr,
381382
std::string_view message) {
382-
if (impl->any_hooks_[static_cast<std::size_t>(
383+
// TASK-051: per-route handler_exception. weak_ptr was set on mr in
384+
// finalize_answer before dispatch_resource_handler was called.
385+
auto res = mr->resource_weak_.lock();
386+
auto* rtable = res ? res->hook_table_raw_() : nullptr;
387+
const bool per_route = rtable != nullptr &&
388+
rtable->any_hooks(hook_phase::handler_exception);
389+
const bool server_chain =
390+
impl->any_hooks_[static_cast<std::size_t>(
383391
hook_phase::handler_exception)]
384392
.load(std::memory_order_relaxed) ||
385-
impl->handler_exception_alias_) {
386-
handler_exception_ctx ctx{
387-
/*request=*/mr->dhr.get(),
388-
/*exception=*/std::current_exception(),
389-
/*message=*/message};
390-
if (auto sc = impl->fire_handler_exception(ctx)) {
391-
mr->response_.emplace(std::move(*sc));
392-
return;
393+
impl->handler_exception_alias_;
394+
395+
if (server_chain || per_route) {
396+
handler_exception_ctx ctx{mr->dhr.get(),
397+
std::current_exception(), message};
398+
if (server_chain) {
399+
if (auto sc = impl->fire_handler_exception(ctx)) {
400+
mr->response_.emplace(std::move(*sc));
401+
return;
402+
}
403+
}
404+
if (per_route) {
405+
// Per-route chain runs AFTER server-wide. Same DR-009 §5.2
406+
// semantics: respond_with() short-circuits the chain.
407+
if (auto sc = rtable->fire_handler_exception(ctx,
408+
[impl](std::string_view m) {
409+
impl->log_dispatch_error(std::string(m));
410+
})) {
411+
mr->response_.emplace(std::move(*sc));
412+
return;
413+
}
393414
}
394-
// DR-009 §5.2 point 4 extended: every hook (and the alias) ran
395-
// without producing a response -- emit the hardcoded empty-body
396-
// 500 directly. Do NOT re-enter run_internal_error_handler_safely
397-
// here; the alias slot has already invoked the user callable on
398-
// this request.
415+
// §5.2 point 4: every hook (and the alias) ran without a
416+
// response -- emit the hardcoded empty-body 500 directly.
399417
mr->response_.emplace(
400418
impl->internal_error_page(mr, "", /*force_our=*/true));
401419
return;
402420
}
403-
// Backwards-compat fast path: no handler_exception hooks AND no
404-
// alias wired (the v1 builder did not call internal_error_handler).
405-
// Use the existing safe-call site so the unset-alias default body
406-
// (which surfaces the message) still applies.
421+
// Backwards-compat fast path: no hook chain at all.
407422
mr->response_.emplace(
408423
impl->run_internal_error_handler_safely(mr, message));
409424
}

src/detail/webserver_finalize.cpp

Lines changed: 149 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,89 @@
4545

4646
#include <chrono>
4747
#include <cstddef>
48+
#include <memory>
49+
#include <optional>
50+
#include <string>
51+
#include <string_view>
4852
#include <utility>
4953

5054
#include "httpserver/detail/body.hpp"
5155
#include "httpserver/detail/modded_request.hpp"
56+
#include "httpserver/detail/resource_hook_table.hpp"
5257
#include "httpserver/hook_action.hpp"
5358
#include "httpserver/hook_context.hpp"
5459
#include "httpserver/hook_phase.hpp"
5560
#include "httpserver/http_request.hpp"
61+
#include "httpserver/http_resource.hpp"
5662
#include "httpserver/http_response.hpp"
5763

5864
namespace httpserver {
5965
namespace detail {
6066

67+
namespace {
68+
69+
// TASK-051: helper to fetch the per-route hook table (if any) from
70+
// the request's resource_weak_ slot. Holds the shared_ptr<http_resource>
71+
// alive in `res_out` so the caller can keep the table pointer valid.
72+
// Returns nullptr when no per-route table exists for this request.
73+
resource_hook_table* per_route_table(detail::modded_request* mr,
74+
std::shared_ptr<http_resource>& res_out) {
75+
if (mr->resource_weak_.expired()) return nullptr;
76+
res_out = mr->resource_weak_.lock();
77+
if (!res_out) return nullptr;
78+
return res_out->hook_table_raw_();
79+
}
80+
81+
} // namespace
82+
83+
// TASK-051: gated fire of before_handler, server-wide AND per-route,
84+
// with short-circuit detection. Carved out of finalize_answer to keep
85+
// the host function under CCN_MAX after TASK-051's per-route extension
86+
// roughly doubled the local branch count. Returns true iff either chain
87+
// short-circuited (mr->response_ has already been emplaced; caller must
88+
// route straight to materialize_and_queue_response). False means both
89+
// chains passed (or both gates were closed) and dispatch should proceed.
90+
bool webserver_impl::fire_before_handler_gated(
91+
detail::modded_request* mr,
92+
const std::shared_ptr<http_resource>& hrm) {
93+
const bool server_gate =
94+
any_hooks_[static_cast<std::size_t>(hook_phase::before_handler)]
95+
.load(std::memory_order_relaxed);
96+
auto* per_route_table = hrm->hook_table_raw_();
97+
const bool per_route_gate = per_route_table != nullptr &&
98+
per_route_table->any_hooks(hook_phase::before_handler);
99+
if (!server_gate && !per_route_gate) return false;
100+
101+
std::optional<route_descriptor> desc;
102+
if (!mr->matched_path_template.empty()) {
103+
desc = route_descriptor{
104+
/*path_template=*/std::string_view{mr->matched_path_template},
105+
/*methods=*/hrm->get_allowed_methods(),
106+
/*is_prefix=*/mr->matched_is_prefix};
107+
}
108+
before_handler_ctx ctx{
109+
/*request=*/mr->dhr.get(),
110+
/*matched=*/std::move(desc),
111+
/*method=*/mr->method_enum,
112+
/*resource=*/hrm.get()};
113+
if (server_gate) {
114+
if (auto sc = fire_before_handler(ctx)) {
115+
mr->response_.emplace(std::move(*sc));
116+
return true;
117+
}
118+
}
119+
if (per_route_gate) {
120+
if (auto sc = per_route_table->fire_before_handler(ctx,
121+
[this](std::string_view m) {
122+
log_dispatch_error(std::string(m));
123+
})) {
124+
mr->response_.emplace(std::move(*sc));
125+
return true;
126+
}
127+
}
128+
return false;
129+
}
130+
61131
// TASK-050: gated fire of after_handler. Fires between
62132
// dispatch_resource_handler (which populates mr->response_) and
63133
// materialize_and_queue_response. A hook returning respond_with(...)
@@ -70,21 +140,37 @@ namespace detail {
70140
// The pre-handler short-circuit branch (mr->skip_handler) is handled
71141
// upstream in finalize_answer and never reaches this site.
72142
void webserver_impl::fire_after_handler_gated(detail::modded_request* mr) {
73-
if (!any_hooks_[static_cast<std::size_t>(hook_phase::after_handler)]
74-
.load(std::memory_order_relaxed)) {
75-
return;
76-
}
143+
const bool server_gate =
144+
any_hooks_[static_cast<std::size_t>(hook_phase::after_handler)]
145+
.load(std::memory_order_relaxed);
146+
std::shared_ptr<http_resource> res;
147+
auto* rtable = per_route_table(mr, res);
148+
const bool route_gate = rtable != nullptr &&
149+
rtable->any_hooks(hook_phase::after_handler);
150+
151+
if (!server_gate && !route_gate) return;
77152
if (!mr->response_) return; // defensive: never fire without a response
78-
after_handler_ctx ctx{
79-
/*request=*/mr->dhr.get(),
80-
/*response=*/&*mr->response_,
81-
};
82-
if (auto sc = fire_after_handler(ctx)) {
83-
// Short-circuit: REPLACE mr->response_ with the hook's response.
84-
// emplace() destroys the old http_response in mr->response_
85-
// first (DR-010), tearing down any deferred-producer captures
86-
// here rather than later in ~modded_request.
87-
mr->response_.emplace(std::move(*sc));
153+
154+
after_handler_ctx ctx{mr->dhr.get(), &*mr->response_};
155+
if (server_gate) {
156+
if (auto sc = fire_after_handler(ctx)) {
157+
// Short-circuit: REPLACE mr->response_ (DR-010 -- emplace
158+
// destroys the old response, releasing any deferred captures
159+
// here rather than later in ~modded_request). Per-route
160+
// chain ALSO sees the replaced response, so refresh ctx.
161+
mr->response_.emplace(std::move(*sc));
162+
ctx.response = &*mr->response_;
163+
}
164+
}
165+
// TASK-051: per-route chain AFTER the server-wide chain. Same
166+
// short-circuit semantics (respond_with replaces the response).
167+
if (route_gate) {
168+
if (auto sc = rtable->fire_after_handler(ctx,
169+
[this](std::string_view m) {
170+
log_dispatch_error(std::string(m));
171+
})) {
172+
mr->response_.emplace(std::move(*sc));
173+
}
88174
}
89175
}
90176

@@ -96,34 +182,35 @@ void webserver_impl::fire_after_handler_gated(detail::modded_request* mr) {
96182
// size from http_response::body_->size(); for deferred/pipe bodies this
97183
// is 0 and consumers should fall back to the Content-Length header.
98184
void webserver_impl::fire_response_sent_gated(detail::modded_request* mr) {
99-
const bool user_hooks_present =
185+
const bool server_gate =
100186
any_hooks_[static_cast<std::size_t>(hook_phase::response_sent)]
101187
.load(std::memory_order_relaxed);
102-
if (!user_hooks_present && !log_access_alias_) return;
103-
if (!mr->response_) return; // defensive
188+
std::shared_ptr<http_resource> res;
189+
auto* rtable = per_route_table(mr, res);
190+
const bool route_gate = rtable != nullptr &&
191+
rtable->any_hooks(hook_phase::response_sent);
104192

105-
std::size_t bytes = 0;
106-
if (mr->response_->body_ != nullptr) {
107-
bytes = mr->response_->body_->size();
108-
}
109-
// Only compute elapsed when user-registered response_sent hooks are
110-
// present and will read the field. If only the log_access alias slot
111-
// fires, elapsed is unused by the alias lambda (which reads only path
112-
// and method), so we skip the steady_clock::now() call (typically
113-
// 20-50 ns on Linux/macOS via VDSO) to avoid per-request overhead
114-
// when the server is configured with log_access but no add_hook hooks.
115-
const auto elapsed = user_hooks_present
193+
if (!server_gate && !route_gate && !log_access_alias_) return;
194+
if (!mr->response_) return;
195+
196+
const std::size_t bytes = (mr->response_->body_ != nullptr)
197+
? mr->response_->body_->size() : 0;
198+
// elapsed is consumed by user hooks (server-wide + per-route), not
199+
// by the log_access alias. Skip the steady_clock::now() call when
200+
// only the alias slot is going to fire.
201+
const auto elapsed = (server_gate || route_gate)
116202
? std::chrono::duration_cast<std::chrono::nanoseconds>(
117203
std::chrono::steady_clock::now() - mr->start_time)
118204
: std::chrono::nanoseconds::zero();
119-
response_sent_ctx ctx{
120-
/*request=*/mr->dhr.get(),
121-
/*response=*/&*mr->response_,
122-
/*status=*/mr->response_->get_status(),
123-
/*bytes_queued=*/bytes,
124-
/*elapsed=*/elapsed,
125-
};
205+
response_sent_ctx ctx{mr->dhr.get(), &*mr->response_,
206+
mr->response_->get_status(), bytes, elapsed};
126207
fire_response_sent(ctx);
208+
// TASK-051: per-route chain AFTER server-wide + its alias slot.
209+
// response_sent is observation-only; no short-circuit logic.
210+
if (route_gate) {
211+
rtable->fire_response_sent(ctx,
212+
[this](std::string_view m) { log_dispatch_error(std::string(m)); });
213+
}
127214
}
128215

129216
// TASK-050: gated fire of request_completed. Fires from
@@ -139,8 +226,25 @@ void webserver_impl::fire_response_sent_gated(detail::modded_request* mr) {
139226
void webserver_impl::fire_request_completed_gated(
140227
detail::modded_request* mr,
141228
enum MHD_RequestTerminationCode toe) {
142-
if (!any_hooks_[static_cast<std::size_t>(hook_phase::request_completed)]
143-
.load(std::memory_order_relaxed)) {
229+
const bool server_gate =
230+
any_hooks_[static_cast<std::size_t>(hook_phase::request_completed)]
231+
.load(std::memory_order_relaxed);
232+
233+
// TASK-051: per-route gate. lock() may return null if the resource
234+
// was unregistered between dispatch and completion (per the action
235+
// item contract: skip the per-route chain in that case).
236+
std::shared_ptr<http_resource> res;
237+
detail::resource_hook_table* rtable = nullptr;
238+
if (!mr->resource_weak_.expired()) {
239+
res = mr->resource_weak_.lock();
240+
if (res) {
241+
rtable = res->hook_table_raw_();
242+
}
243+
}
244+
const bool per_route_present = rtable != nullptr &&
245+
rtable->any_hooks(hook_phase::request_completed);
246+
247+
if (!server_gate && !per_route_present) {
144248
return;
145249
}
146250
const http_response* resp_ptr =
@@ -157,7 +261,14 @@ void webserver_impl::fire_request_completed_gated(
157261
/*succeeded=*/(toe == MHD_REQUEST_TERMINATED_COMPLETED_OK),
158262
/*duration=*/std::chrono::steady_clock::now() - mr->start_time,
159263
};
160-
fire_request_completed(ctx);
264+
if (server_gate) {
265+
fire_request_completed(ctx);
266+
}
267+
// TASK-051: per-route chain fires AFTER the server-wide chain.
268+
if (per_route_present) {
269+
rtable->fire_request_completed(ctx,
270+
[this](std::string_view m) { log_dispatch_error(std::string(m)); });
271+
}
161272
}
162273

163274
} // namespace detail

src/detail/webserver_request.cpp

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
#include "httpserver/detail/http_endpoint.hpp"
6868
#include "httpserver/detail/lambda_resource.hpp"
6969
#include "httpserver/detail/modded_request.hpp"
70+
#include "httpserver/detail/resource_hook_table.hpp"
7071
#include "httpserver/http_request.hpp"
7172
#include "httpserver/http_resource.hpp"
7273
#include "httpserver/http_response.hpp"
@@ -303,32 +304,29 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection,
303304
std::shared_ptr<http_resource> hrm;
304305
bool found = resolve_resource_for_request(mr, hrm);
305306

307+
// TASK-051: capture the resolved resource on the request so the
308+
// tail-end firing helpers (fire_response_sent_gated /
309+
// fire_request_completed_gated) can fire the per-route hook chain
310+
// after the server-wide one. weak_ptr does not keep the resource
311+
// alive; the local `hrm` shared_ptr does that for the duration of
312+
// finalize_answer, after which mr->response_ no longer references
313+
// the resource directly.
314+
if (found) {
315+
mr->resource_weak_ = hrm;
316+
}
317+
306318
fire_route_resolved_gated(this, mr, found, hrm);
307319

308-
// TASK-048: fire before_handler from finalize_answer (not from inside
309-
// dispatch_resource_handler). This ensures auth and method-not-allowed
310-
// alias hooks run as part of the unified before_handler chain, with
311-
// the auth alias as the first hook (registered in
312-
// install_default_alias_hooks_). The gate preserves zero-cost-when-unused.
313-
if (found &&
314-
any_hooks_[static_cast<std::size_t>(hook_phase::before_handler)]
315-
.load(std::memory_order_relaxed)) {
316-
std::optional<route_descriptor> desc;
317-
if (!mr->matched_path_template.empty()) {
318-
desc = route_descriptor{
319-
/*path_template=*/std::string_view{mr->matched_path_template},
320-
/*methods=*/hrm->get_allowed_methods(),
321-
/*is_prefix=*/mr->matched_is_prefix};
322-
}
323-
before_handler_ctx ctx{
324-
/*request=*/mr->dhr.get(),
325-
/*matched=*/std::move(desc),
326-
/*method=*/mr->method_enum,
327-
/*resource=*/hrm.get()};
328-
if (auto sc = fire_before_handler(ctx)) {
329-
mr->response_.emplace(std::move(*sc));
330-
return materialize_and_queue_response(connection, mr);
331-
}
320+
// TASK-048 / TASK-051: fire before_handler from finalize_answer (not
321+
// from inside dispatch_resource_handler). This ensures auth and
322+
// method-not-allowed alias hooks run as part of the unified
323+
// before_handler chain, with the auth alias as the first hook
324+
// (registered in install_default_alias_hooks_). Per-route firing is
325+
// included by the helper (see fire_before_handler_gated). If either
326+
// chain short-circuited, mr->response_ is already populated and we
327+
// route straight to materialize.
328+
if (found && fire_before_handler_gated(mr, hrm)) {
329+
return materialize_and_queue_response(connection, mr);
332330
}
333331

334332
if (found) {

0 commit comments

Comments
 (0)