Skip to content

Commit 43ece69

Browse files
etrclaude
andcommitted
refactor: give webserver_impl a single composition-root TU
Every webserver_impl translation unit now maps to exactly one class, and the one file that spanned both classes is gone. Previously webserver_impl's construction (the DR-014 collaborator/service wiring) lived inside webserver.cpp alongside the public webserver:: façade, and two more of its members sat in single-function files (webserver_dispatch.cpp = 8 SLOC, webserver_routes_upsert.cpp) that existed only as gate-era overflow. Introduce src/detail/webserver_impl.cpp as the composition-root TU: it holds the webserver_impl ctor/dtor (moved out of webserver.cpp) plus the small non-trampoline glue folded in from webserver_dispatch.cpp (serialize_allow_methods) and webserver_routes_upsert.cpp (prepare_or_create_lambda_shim / commit_handlers_to_shim + the for_each_requested_method helper). webserver.cpp is now purely the webserver façade (drops the now-unused <random>). The MHD trampolines stay in the adapter TUs (webserver_request/callbacks.cpp). Net: webserver_impl goes from 6 TUs to 3 (webserver_impl + request + callbacks); no file mixes webserver and webserver_impl anymore. Pure code move, no behavior change. webserver.cpp 190->148 SLOC, new webserver_impl.cpp 108. Local: builds, 113/113 tests pass, file-size + complexity + cpplint gates green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015tAodxYJMEY4VxCX4dk62e
1 parent 03b80bc commit 43ece69

5 files changed

Lines changed: 261 additions & 304 deletions

File tree

src/Makefile.am

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ 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/request_dispatcher.cpp detail/request_pipeline.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_dispatch.cpp detail/webserver_request.cpp detail/webserver_body_pipeline.cpp detail/webserver_aliases.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/request_pipeline.cpp detail/response_materializer.cpp detail/upload_pipeline.cpp detail/websocket_upgrader.cpp detail/webserver_impl.cpp detail/webserver_lifecycle.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_callbacks.cpp detail/webserver_request.cpp detail/webserver_body_pipeline.cpp detail/webserver_aliases.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>.

src/detail/webserver_dispatch.cpp

Lines changed: 0 additions & 45 deletions
This file was deleted.

src/detail/webserver_impl.cpp

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
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+
// webserver_impl.cpp -- the composition-root translation unit for
22+
// detail::webserver_impl.
23+
//
24+
// Holds the impl's construction/destruction (the DR-014 collaborator +
25+
// service wiring in the member-initialiser list, and the GnuTLS SNI-cache
26+
// teardown) plus the small non-trampoline member glue that has no better
27+
// home:
28+
// * serialize_allow_methods -- a thin test seam over
29+
// detail::format_allow_header (production dispatch calls
30+
// hrm->get_allow_header() directly);
31+
// * prepare_or_create_lambda_shim / commit_handlers_to_shim -- the
32+
// lambda_resource shim lifecycle for on_*/route registration (the
33+
// table probe + mutation live in the route_table collaborator; these
34+
// run inside the caller's routes_.lock_for_write() window).
35+
//
36+
// The MHD C-ABI trampolines live in the adapter TUs (webserver_request.cpp /
37+
// webserver_callbacks.cpp); the public webserver:: surface lives in
38+
// webserver.cpp and the webserver_*.cpp API-area files. Consolidated here
39+
// (from the former webserver.cpp ctor/dtor block, webserver_dispatch.cpp, and
40+
// webserver_routes_upsert.cpp) so that webserver_impl maps to a small set of
41+
// single-class TUs rather than a scatter of one-function files.
42+
43+
#include "httpserver/webserver.hpp"
44+
#include "httpserver/detail/webserver_impl.hpp"
45+
46+
#include <microhttpd.h>
47+
48+
#include <array>
49+
#include <cstdint>
50+
#include <functional>
51+
#include <memory>
52+
#include <random>
53+
#include <stdexcept>
54+
#include <string>
55+
#include <utility>
56+
57+
#include "httpserver/create_webserver.hpp"
58+
#include "httpserver/http_method.hpp"
59+
#include "httpserver/http_request.hpp"
60+
#include "httpserver/http_resource.hpp"
61+
#include "httpserver/http_response.hpp"
62+
#include "httpserver/detail/http_endpoint.hpp"
63+
#include "httpserver/detail/lambda_resource.hpp"
64+
#include "httpserver/detail/method_utils.hpp"
65+
#include "httpserver/detail/route_entry.hpp"
66+
67+
#ifdef HAVE_GNUTLS
68+
#include <gnutls/gnutls.h>
69+
#endif // HAVE_GNUTLS
70+
71+
namespace httpserver {
72+
73+
namespace {
74+
75+
// Iterate enum-declaration order (get, head, post, ...) over the bits
76+
// set in @p methods, invoking @p fn for each. Used by the on_methods_
77+
// pre-check loop and the commit loop; pulling the scaffolding into a
78+
// single helper dedupes the iteration boilerplate. The order matches
79+
// http_method enum-declaration order, which is also the serialization
80+
// order for the `Allow:` header.
81+
template <typename Fn>
82+
void for_each_requested_method(method_set methods, Fn&& fn) {
83+
for (std::uint8_t i = 0;
84+
i < static_cast<std::uint8_t>(http_method::count_); ++i) {
85+
auto m = static_cast<http_method>(i);
86+
if (methods.contains(m)) fn(m);
87+
}
88+
}
89+
90+
} // namespace
91+
92+
namespace detail {
93+
94+
// ----- webserver_impl construction / destruction -------------------------
95+
96+
// Seed `digest_opaque_` with 16 random bytes from
97+
// std::random_device, hex-encoded -> 32-char string. RFC 7616 §5.10:
98+
// opaque is an identifier, not a secret -- std::random_device gives the
99+
// "unpredictable enough that clients cannot guess server state"
100+
// property the RFC requires; the strong nonce HMAC keying is owned by
101+
// libmicrohttpd (MHD_OPTION_DIGEST_AUTH_RANDOM seed on the create_webserver).
102+
//
103+
// Note: std::random_device's fallback behaviour when no hardware/OS entropy
104+
// source is available is implementation-defined -- it may fall back to a
105+
// deterministic PRNG on some platforms/standard libraries. That does not
106+
// weaken security here: the RFC 7616 opaque is an identifier (not a
107+
// secret), and the anti-guessing guarantee for digest auth comes from
108+
// libmicrohttpd's HMAC-keyed nonce (MHD_OPTION_DIGEST_AUTH_RANDOM), not
109+
// from this value.
110+
#ifdef HAVE_DAUTH
111+
static std::string generate_random_hex_opaque_() {
112+
std::random_device rd;
113+
static constexpr char kHex[] = "0123456789abcdef";
114+
std::string out;
115+
out.reserve(32);
116+
for (int i = 0; i < 8; ++i) {
117+
uint32_t sample = static_cast<uint32_t>(rd());
118+
// Pack 4 bytes -> 8 hex chars per std::random_device sample.
119+
for (int b = 0; b < 4; ++b) {
120+
uint32_t octet = (sample >> (b * 8)) & 0xFFu;
121+
out.push_back(kHex[(octet >> 4) & 0xFu]);
122+
out.push_back(kHex[octet & 0xFu]);
123+
}
124+
}
125+
return out;
126+
}
127+
#endif // HAVE_DAUTH
128+
129+
webserver_impl::webserver_impl(webserver* parent, MHD_socket bind_socket_val)
130+
: parent(parent), daemon_(this, bind_socket_val),
131+
#ifdef HAVE_WEBSOCKET
132+
// Declared with ws_ (before the services block), so it must be
133+
// initialised before errors_ here to satisfy -Wreorder.
134+
ws_upgrader_(ws_),
135+
#endif // HAVE_WEBSOCKET
136+
errors_(parent->config), hooks_dispatch_(hooks_, parent->config),
137+
response_mat_(errors_, hooks_dispatch_, digest_opaque_, parent->config),
138+
upload_(parent->config),
139+
dispatcher_(routes_, hooks_dispatch_, errors_, response_mat_,
140+
#ifdef HAVE_WEBSOCKET
141+
ws_upgrader_,
142+
#endif // HAVE_WEBSOCKET
143+
parent->config),
144+
pipeline_(hooks_dispatch_, dispatcher_, parent->config) {
145+
// Guard against null parent: the dispatch helpers (not_found_page,
146+
// method_not_allowed_page, internal_error_page, etc.) read the const
147+
// config bag on `parent` and will dereference this pointer on every
148+
// request. The only valid call site is webserver::webserver, which
149+
// always passes `this` — a non-null pointer to the owning webserver.
150+
// (daemon_ was already constructed with owner=this above; it stores the
151+
// pointer but never dereferences parent config until start(). The
152+
// config-reading behavior services below -- errors_ and the ones added
153+
// in later DR-014 steps -- DO bind parent->config in this init list, so
154+
// a non-null parent is a hard construction precondition; the sole caller
155+
// webserver::webserver satisfies it by passing `this`.)
156+
if (parent == nullptr) {
157+
throw std::invalid_argument(
158+
"webserver_impl requires a non-null owning webserver pointer");
159+
}
160+
#ifdef HAVE_DAUTH
161+
digest_opaque_ = generate_random_hex_opaque_();
162+
#endif // HAVE_DAUTH
163+
}
164+
165+
webserver_impl::~webserver_impl() {
166+
#if defined(HAVE_GNUTLS) && defined(MHD_OPTION_HTTPS_CERT_CALLBACK)
167+
// Clean up cached SNI credentials
168+
for (auto& [name, creds] : sni_credentials_cache) {
169+
gnutls_certificate_free_credentials(creds);
170+
}
171+
sni_credentials_cache.clear();
172+
#endif // HAVE_GNUTLS && MHD_OPTION_HTTPS_CERT_CALLBACK
173+
}
174+
175+
// ----- allowed-method serialization (test seam) --------------------------
176+
177+
std::string webserver_impl::serialize_allow_methods(method_set allowed) const {
178+
return detail::format_allow_header(allowed);
179+
}
180+
181+
// ----- on_*/route lambda-shim registration POLICY ------------------------
182+
183+
// Caller must hold routes_.lock_for_write() (unique_lock). The shim returned
184+
// here is the SAME object that subsequent helpers (commit_handlers_to_shim,
185+
// upsert_v2_table_entry_locked_) mutate; holding the lock across the
186+
// whole prepare->commit->upsert sequence prevents a concurrent registration
187+
// from racing in between.
188+
//
189+
// The returned bool (written /*fresh=*/ below) is true iff the shim was
190+
// newly constructed because no entry existed at this path. on_methods_
191+
// receives it as `is_new_entry` and forwards it unchanged as the `fresh`
192+
// parameter of upsert_v2_table_entry_locked_ -- all three names denote
193+
// the same flag. The v2 route-table conflict probe + table mutation
194+
// (find_v2_entry_by_path_ / upsert_v2_table_entry_locked_ and the reject_*
195+
// guards) live in the route_table collaborator (src/detail/route_table.cpp);
196+
// this reaches them through impl's routes_ member.
197+
std::pair<std::shared_ptr<detail::lambda_resource>, bool>
198+
webserver_impl::prepare_or_create_lambda_shim(const detail::http_endpoint& idx,
199+
method_set methods) {
200+
const detail::route_entry* existing = routes_.find_v2_entry_by_path_(idx);
201+
if (existing == nullptr) {
202+
return {std::make_shared<detail::lambda_resource>(), /*fresh=*/true};
203+
}
204+
// Existing entry. route_entry::handler is a
205+
// shared_ptr<http_resource>. Dynamic-cast to
206+
// lambda_resource: if the cast misses, a class-based
207+
// register_path/register_prefix owns this path and we must throw.
208+
auto shim = std::dynamic_pointer_cast<detail::lambda_resource>(
209+
existing->handler);
210+
if (!shim) {
211+
throw std::invalid_argument(
212+
"A non-lambda http_resource is already registered at "
213+
"this path; on_*/route cannot share a path with "
214+
"register_path/register_prefix");
215+
}
216+
// Atomicity pre-check: every requested slot must be empty BEFORE
217+
// we mutate any of them.
218+
for_each_requested_method(methods, [&](http_method m) {
219+
if (shim->has_slot(m)) {
220+
throw std::invalid_argument(
221+
"A handler is already registered for one of the "
222+
"requested methods on this path");
223+
}
224+
});
225+
return {shim, /*fresh=*/false};
226+
}
227+
228+
void webserver_impl::commit_handlers_to_shim(detail::lambda_resource& shim,
229+
method_set methods,
230+
std::function<::httpserver::http_response(
231+
const ::httpserver::http_request&)> handler) {
232+
// Collect the set of requested methods in iteration order so the
233+
// loop below can identify the last one. Using a small inline array
234+
// avoids a heap allocation in the common case (N <= 9 methods).
235+
//
236+
// Move-into-last-slot optimisation:
237+
// all but the last slot receive a copy; the last slot is populated
238+
// by moving the handler, avoiding one extra heap allocation when the
239+
// std::function's capture is too large for the SBO buffer.
240+
std::array<http_method, static_cast<std::size_t>(http_method::count_)> buf{};
241+
std::size_t count = 0;
242+
for_each_requested_method(methods, [&](http_method m) {
243+
buf[count++] = m;
244+
});
245+
for (std::size_t i = 0; i < count; ++i) {
246+
if (i + 1 < count) {
247+
shim.set_slot(buf[i], handler); // copy
248+
} else {
249+
shim.set_slot(buf[i], std::move(handler)); // move into last slot
250+
}
251+
}
252+
}
253+
254+
} // namespace detail
255+
256+
} // namespace httpserver

0 commit comments

Comments
 (0)