Skip to content

Commit ee04989

Browse files
etrclaude
andcommitted
refactor(dr-014): extract request_dispatcher behavior service
Move the routing + auth + handler-invocation stage off webserver_impl into detail::request_dispatcher: finalize_answer (the orchestrator), resolve_resource_for_request, dispatch_resource_handler, the handler_exception helper, and the fire_route_resolved_gated helper. It holds route_table& (lookup), hook_dispatcher& (all firing/gating), error_pages& (404/405/500), response_materializer& (queueing), websocket_upgrader& (HAVE_WEBSOCKET), and const webserver_config& (logging); owns no state. complete_request (still on webserver_impl, moving to request_pipeline next) now hands off to dispatcher_.finalize_answer directly — no webserver_impl forwarder. should_skip_auth (auth alias + tests) and serialize_allow_methods (bench/test seam) stay on webserver_impl. Supporting changes: - hook_dispatcher gains has_hooks_for / has_handler_exception_alias query forwarders (dispatch-side gating). - request_dispatcher is granted http_request friendship (route-param replay via set_arg). - webserver_impl::try_handle_websocket_upgrade is now orphaned (the dispatcher calls ws_upgrader_.try_handle directly), so it and the empty webserver_websocket.cpp forwarder TU are removed. - The HAVE_WEBSOCKET probe is factored into a try_ws_upgrade helper so the #ifdef stays out of finalize_answer's CCN count (lizard counts it). 113/113 tests pass; cpplint/complexity/file-size gates green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 58e3399 commit ee04989

12 files changed

Lines changed: 490 additions & 492 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 webserver_add_hook.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp http_response_factories.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp peer_address.cpp resource_hook_table.cpp cookie.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/ip_access_control.cpp detail/ws_registry.cpp detail/hook_bus.cpp detail/route_table.cpp detail/daemon_lifecycle.cpp detail/dispatch_util.cpp detail/error_pages.cpp detail/hook_dispatcher.cpp detail/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.cpp detail/response_materializer.cpp detail/upload_pipeline.cpp detail/websocket_upgrader.cpp detail/webserver_lifecycle.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_routes_upsert.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_response_queue.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_hook_firing.cpp detail/hook_phase_dispatch.cpp
28+
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp webserver_add_hook.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp http_response_factories.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp peer_address.cpp resource_hook_table.cpp cookie.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/ip_access_control.cpp detail/ws_registry.cpp detail/hook_bus.cpp detail/route_table.cpp detail/daemon_lifecycle.cpp detail/dispatch_util.cpp detail/error_pages.cpp detail/hook_dispatcher.cpp detail/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.cpp detail/request_dispatcher.cpp detail/response_materializer.cpp detail/upload_pipeline.cpp detail/websocket_upgrader.cpp detail/webserver_lifecycle.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_routes_upsert.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_response_queue.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_hook_firing.cpp detail/hook_phase_dispatch.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/ip_access_control.hpp httpserver/detail/ws_registry.hpp httpserver/detail/hook_bus.hpp httpserver/detail/route_table.hpp httpserver/detail/daemon_lifecycle.hpp httpserver/detail/dispatch_util.hpp httpserver/detail/error_pages.hpp httpserver/detail/hook_dispatcher.hpp httpserver/detail/response_materializer.hpp httpserver/detail/upload_pipeline.hpp httpserver/detail/websocket_upgrader.hpp httpserver/detail/secure_zero.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/segment_trie.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp httpserver/detail/unescape_helpers.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/ip_access_control.hpp httpserver/detail/ws_registry.hpp httpserver/detail/hook_bus.hpp httpserver/detail/route_table.hpp httpserver/detail/daemon_lifecycle.hpp httpserver/detail/dispatch_util.hpp httpserver/detail/error_pages.hpp httpserver/detail/hook_dispatcher.hpp httpserver/detail/request_dispatcher.hpp httpserver/detail/response_materializer.hpp httpserver/detail/upload_pipeline.hpp httpserver/detail/websocket_upgrader.hpp httpserver/detail/secure_zero.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/segment_trie.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp httpserver/detail/unescape_helpers.hpp gettext.h
3333
nobase_include_HEADERS = httpserver.hpp httpserver/body_kind.hpp httpserver/cookie.hpp httpserver/constants.hpp httpserver/create_webserver.hpp httpserver/create_webserver_setters.hpp httpserver/create_test_request.hpp httpserver/webserver.hpp httpserver/webserver_routes.hpp httpserver/webserver_runtime.hpp httpserver/webserver_websocket.hpp httpserver/webserver_hooks.hpp httpserver/websocket_handler.hpp httpserver/http_utils.hpp httpserver/http_utils_helpers.hpp httpserver/ip_representation.hpp httpserver/file_info.hpp httpserver/http_request.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/hook_dispatcher.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@
5252
namespace httpserver {
5353
namespace detail {
5454

55+
bool hook_dispatcher::has_hooks_for(hook_phase p) const noexcept {
56+
return hooks_.has_hooks_for(p);
57+
}
58+
59+
bool hook_dispatcher::has_handler_exception_alias() const noexcept {
60+
return hooks_.has_handler_exception_alias();
61+
}
62+
5563
namespace {
5664

5765
// Fetch the per-route hook table (if any) from the request's

src/detail/request_dispatcher.cpp

Lines changed: 311 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,311 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2026 Sebastiano Merlino
4+
5+
This library is free software; you can redistribute it and/or
6+
modify it under the terms of the GNU Lesser General Public
7+
License as published by the Free Software Foundation; either
8+
version 2.1 of the License, or (at your option) any later version.
9+
10+
This library is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public
16+
License along with this library; if not, write to the Free Software
17+
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
18+
USA
19+
*/
20+
21+
// request_dispatcher behavior service (DR-014 §4.11). finalize_answer +
22+
// resolve_resource_for_request moved verbatim out of
23+
// detail/webserver_request.cpp; dispatch_resource_handler +
24+
// handle_dispatch_exception out of detail/webserver_dispatch.cpp; the
25+
// file-local fire_route_resolved_gated moved here too. Rewiring vs the
26+
// originals: lookup goes through routes_ (route_table), all hook firing +
27+
// gating through hooks_ (hook_dispatcher), 404/405/500 through errors_
28+
// (error_pages), queueing through materializer_ (response_materializer),
29+
// the websocket probe through ws_upgrader_ (HAVE_WEBSOCKET), and
30+
// log_dispatch_error is the free function over config_.
31+
32+
#include "httpserver/detail/request_dispatcher.hpp"
33+
34+
#include <microhttpd.h>
35+
36+
#include <memory>
37+
#include <optional>
38+
#include <string>
39+
#include <string_view>
40+
#include <utility>
41+
42+
#include "httpserver/create_webserver.hpp"
43+
#include "httpserver/hook_context.hpp"
44+
#include "httpserver/hook_phase.hpp"
45+
#include "httpserver/http_request.hpp"
46+
#include "httpserver/http_resource.hpp"
47+
#include "httpserver/http_response.hpp"
48+
#include "httpserver/http_utils.hpp"
49+
#include "httpserver/detail/dispatch_util.hpp"
50+
#include "httpserver/detail/error_pages.hpp"
51+
#include "httpserver/detail/hook_dispatcher.hpp"
52+
#include "httpserver/detail/modded_request.hpp"
53+
#include "httpserver/detail/resource_hook_table.hpp"
54+
#include "httpserver/detail/response_materializer.hpp"
55+
#include "httpserver/detail/route_table.hpp"
56+
#ifdef HAVE_WEBSOCKET
57+
#include "httpserver/detail/websocket_upgrader.hpp"
58+
#endif // HAVE_WEBSOCKET
59+
60+
namespace httpserver {
61+
62+
using httpserver::http::http_utils;
63+
64+
namespace detail {
65+
66+
bool request_dispatcher::resolve_resource_for_request(detail::modded_request* mr,
67+
std::shared_ptr<http_resource>& hrm) {
68+
// matched_path_template + matched_is_prefix feed the route_resolved and
69+
// before_handler hook ctxs. Skip the heap allocation when no hook in
70+
// either phase is registered.
71+
const bool need_path_template =
72+
hooks_.has_hooks_for(hook_phase::route_resolved) ||
73+
hooks_.has_hooks_for(hook_phase::before_handler);
74+
75+
// v2 lookup pipeline: cache -> exact -> radix -> regex.
76+
route_table::lookup_result result =
77+
routes_.lookup_v2(mr->method_enum, mr->standardized_url);
78+
if (!result.found) return false;
79+
80+
// Every writer of route_entry populates a non-null shared_ptr; a null
81+
// pointer here would indicate a future bug, so degrade to a not-found
82+
// miss defensively.
83+
if (result.entry.handler == nullptr) {
84+
return false;
85+
}
86+
hrm = result.entry.handler;
87+
88+
// Replay captured URL parameters into the request (per-name set_arg
89+
// matches v1 behaviour: duplicates with later wins).
90+
if (mr->request != nullptr) {
91+
for (const auto& [name, value] : result.captured_params) {
92+
mr->request->set_arg(name, value);
93+
}
94+
}
95+
96+
// Populate the hook ctx scratch slots when at least one hook is
97+
// registered for the phases that read them. v2 does not store the
98+
// matched URL template; fall back to standardized_url.
99+
if (need_path_template) {
100+
mr->matched_path_template = mr->standardized_url;
101+
mr->matched_is_prefix = result.entry.is_prefix;
102+
}
103+
return true;
104+
}
105+
106+
namespace {
107+
108+
// Shared body of the two dispatch_resource_handler catch arms. Either
109+
// routes the dispatch-thrown exception through the handler_exception hook
110+
// chain (when any user hooks are registered or the internal_error_handler
111+
// alias slot is wired) or falls back to run_internal_error_handler_safely.
112+
// Extracted to keep dispatch_resource_handler under the CCN bar.
113+
void handle_dispatch_exception(hook_dispatcher& hooks, error_pages& errors,
114+
const webserver_config& config, detail::modded_request* mr,
115+
std::string_view message) {
116+
// Per-route handler_exception: the weak_ptr was set on mr in
117+
// finalize_answer before dispatch_resource_handler was called. res keeps
118+
// the resource alive while rtable is in use.
119+
auto res = mr->resource_weak_.lock();
120+
auto* rtable = res ? res->hook_table_raw_() : nullptr;
121+
const bool per_route = rtable != nullptr &&
122+
rtable->any_hooks(hook_phase::handler_exception);
123+
const bool server_chain =
124+
hooks.has_hooks_for(hook_phase::handler_exception) ||
125+
hooks.has_handler_exception_alias();
126+
127+
if (server_chain || per_route) {
128+
// Capture the live exception_ptr before constructing the ctx so the
129+
// side-effectful call is separated from the struct literal.
130+
auto current_exc = std::current_exception();
131+
handler_exception_ctx ctx{mr->request.get(), current_exc, message};
132+
if (server_chain) {
133+
if (auto sc = hooks.fire_handler_exception(ctx)) {
134+
mr->response.emplace(std::move(*sc));
135+
return;
136+
}
137+
}
138+
if (per_route) {
139+
// Per-route chain runs AFTER server-wide. Same semantics:
140+
// respond_with() short-circuits the chain.
141+
if (auto sc = rtable->fire_handler_exception(ctx,
142+
[&config](std::string_view m) {
143+
log_dispatch_error(config, m);
144+
})) {
145+
mr->response.emplace(std::move(*sc));
146+
return;
147+
}
148+
}
149+
// Every hook (and the alias) ran without a response -- emit the
150+
// hardcoded empty-body 500 directly.
151+
mr->response.emplace(
152+
errors.internal_error_page(mr, "", /*force_our=*/true));
153+
return;
154+
}
155+
// Backwards-compat fast path: no hook chain at all.
156+
mr->response.emplace(
157+
errors.run_internal_error_handler_safely(mr, message));
158+
}
159+
160+
} // namespace
161+
162+
void request_dispatcher::dispatch_resource_handler(detail::modded_request* mr,
163+
const std::shared_ptr<http_resource>& hrm) {
164+
try {
165+
if (mr->pp != nullptr) {
166+
MHD_destroy_post_processor(mr->pp);
167+
mr->pp = nullptr;
168+
}
169+
// before_handler fires from finalize_answer, so auth and
170+
// method-not-allowed alias hooks run as part of the unified
171+
// before_handler chain before this is called; the is_allowed check
172+
// below is the default (no-hook) 405 fallback.
173+
if (hrm->is_allowed(mr->method_enum)) {
174+
// Pointer-to-member dispatch returns http_response by value; the
175+
// prvalue is moved into the per-connection optional anchor.
176+
mr->response.emplace(((*hrm).*(mr->callback))(*mr->request));
177+
if (mr->response->get_status() == -1) {
178+
// Handler returned the default-sentinel response. Route
179+
// through the safe internal-error path.
180+
mr->response.emplace(errors_.run_internal_error_handler_safely(
181+
mr, "handler returned null response"));
182+
}
183+
return;
184+
}
185+
// Method not allowed: emit the Allow header from the resource's
186+
// lazily-cached value.
187+
mr->response.emplace(errors_.method_not_allowed_page(mr));
188+
const std::string& header_value = hrm->get_allow_header();
189+
if (!header_value.empty()) {
190+
mr->response->with_header(http_utils::http_header_allow, header_value);
191+
}
192+
} catch (const std::exception& e) {
193+
// Handler threw std::exception -> handler_exception chain (with the
194+
// internal_error_handler alias as last-position fallback). Only build
195+
// the heap string when a log_error callback is wired.
196+
if (config_.log_error) {
197+
log_dispatch_error(config_,
198+
std::string("dispatch: handler threw std::exception: ")
199+
.append(e.what()));
200+
}
201+
handle_dispatch_exception(hooks_, errors_, config_, mr,
202+
std::string_view{e.what()});
203+
} catch (...) {
204+
// Handler threw non-std::exception. Same flow, sentinel message.
205+
log_dispatch_error(config_, "dispatch: handler threw unknown exception");
206+
handle_dispatch_exception(hooks_, errors_, config_, mr,
207+
std::string_view{"unknown exception"});
208+
}
209+
}
210+
211+
namespace {
212+
213+
void fire_route_resolved_gated(hook_dispatcher& hooks,
214+
detail::modded_request* mr, bool found,
215+
const std::shared_ptr<http_resource>& hrm) {
216+
if (!hooks.has_hooks_for(hook_phase::route_resolved)) {
217+
return;
218+
}
219+
std::optional<route_descriptor> desc;
220+
if (found && hrm) {
221+
desc = route_descriptor{
222+
/*path_template=*/std::string_view{mr->matched_path_template},
223+
/*methods=*/hrm->get_allowed_methods(),
224+
/*is_prefix=*/mr->matched_is_prefix};
225+
}
226+
route_resolved_ctx ctx{
227+
/*request=*/mr->request.get(),
228+
/*matched=*/std::move(desc),
229+
/*resource=*/hrm ? hrm.get() : nullptr};
230+
hooks.fire_route_resolved(ctx);
231+
}
232+
233+
} // namespace
234+
235+
std::optional<MHD_Result> request_dispatcher::try_ws_upgrade(
236+
MHD_Connection* connection, detail::modded_request* mr) {
237+
#ifdef HAVE_WEBSOCKET
238+
return ws_upgrader_.try_handle(connection, mr);
239+
#else
240+
(void)connection;
241+
(void)mr;
242+
return std::nullopt;
243+
#endif // HAVE_WEBSOCKET
244+
}
245+
246+
MHD_Result request_dispatcher::finalize_answer(MHD_Connection* connection,
247+
detail::modded_request* mr) {
248+
if (auto ws_result = try_ws_upgrade(connection, mr)) {
249+
return *ws_result;
250+
}
251+
252+
// A pre-handler short-circuit hook (request_received or body_chunk)
253+
// already populated mr->response. Skip route lookup, auth, and dispatch
254+
// -- go straight to the response queue. after_handler is NOT fired on
255+
// this path (no handler ran); response_sent fires unconditionally in
256+
// materialize_and_queue_response.
257+
if (mr->skip_handler) {
258+
return materializer_.materialize_and_queue_response(connection, mr,
259+
nullptr);
260+
}
261+
262+
// Hold a shared_ptr copy across dispatch so a concurrent
263+
// unregister_resource cannot free the resource mid-call.
264+
std::shared_ptr<http_resource> hrm;
265+
bool found = resolve_resource_for_request(mr, hrm);
266+
267+
if (found) {
268+
// Snapshot whether this resource carries a per-route hook table so
269+
// fire_request_completed_gated (fires after this shared_ptr is gone)
270+
// can gate its weak_ptr lock() on the common zero-per-route-hook path.
271+
mr->route_has_hook_table_ = (hrm->hook_table_raw_() != nullptr);
272+
273+
// Only stamp the weak_ptr when a later out-of-scope consumer can use
274+
// it (the MHD completion callback), i.e. iff this resource has a
275+
// per-route hook table OR a server-wide request_completed hook is
276+
// registered. Gating this drops 2 control-block atomics per matched
277+
// request on the common zero-hook path.
278+
if (mr->route_has_hook_table_ ||
279+
hooks_.has_hooks_for(hook_phase::request_completed)) {
280+
mr->resource_weak_ = hrm;
281+
}
282+
}
283+
284+
fire_route_resolved_gated(hooks_, mr, found, hrm);
285+
286+
// Fire before_handler from here (not inside dispatch_resource_handler) so
287+
// auth + method-not-allowed alias hooks run as part of the unified
288+
// before_handler chain (per-route firing included by the gate). If either
289+
// chain short-circuited, mr->response is already populated -> materialise.
290+
if (found && hooks_.fire_before_handler_gated(mr, hrm)) {
291+
return materializer_.materialize_and_queue_response(connection, mr,
292+
hrm.get());
293+
}
294+
295+
if (found) {
296+
dispatch_resource_handler(mr, hrm);
297+
} else if (!mr->response) {
298+
mr->response.emplace(errors_.not_found_page(mr));
299+
}
300+
301+
// after_handler fires between handler return (or 404 synthesis) and
302+
// materialise. hrm is null on the 404 path; both the gate and the
303+
// materialiser tolerate a null resource.
304+
hooks_.fire_after_handler_gated(mr, hrm.get());
305+
306+
return materializer_.materialize_and_queue_response(connection, mr,
307+
hrm.get());
308+
}
309+
310+
} // namespace detail
311+
} // namespace httpserver

0 commit comments

Comments
 (0)