Skip to content

Commit 374a203

Browse files
etrclaude
andcommitted
webserver: three near-bar refactors (register_impl_ / decode_websocket_buffer / answer_to_connection)
Three independent CCN-over-10 functions, bundled because each extraction is small. webserver::register_impl_ (CCN 14 -> 10): Extracted detail::webserver_impl::register_v2_route: a one-shot insert into the v2 3-tier route table with method_set::set_all() and no merge. Distinct from upsert_v2_table_entry (the on_*/route path with method-set merging). register_impl_ retains the v1 maps work and the input validation under registered_resources_mutex. decode_websocket_buffer (CCN 13 -> 4): Extracted dispatch_websocket_frame (the switch on the decode result) and handle_close_frame (RFC 6455 §5.5.1 close-payload parsing). decode_websocket_buffer is now just the recv-and-feed loop: MHD_websocket_decode + dispatch + break-on-zero-progress. webserver_impl::answer_to_connection (CCN 13 -> 4): Extracted resolve_method_callback: maps the wire-string HTTP method to mr->callback (pointer-to-member dispatch), mr->method_enum (is_allowed input), and mr->has_body (body-buffering branch). The 9-way strcmp chain stays in its own static method at CCN 10 (the bar), where it belongs as a single responsibility. Verified locally: full `make check`. scripts/check-complexity.sh CCN_MAX ratcheted 15 -> 13 (the new worst offender is webserver_impl::lookup_v2 caller-side wired into radix_tree::find at CCN 12). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2494c46 commit 374a203

3 files changed

Lines changed: 147 additions & 105 deletions

File tree

scripts/check-complexity.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
set -euo pipefail
2828

2929
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
30-
CCN_MAX="${CCN_MAX:-15}"
30+
CCN_MAX="${CCN_MAX:-13}"
3131

3232
# Prefer the standalone `lizard` entrypoint if it's on PATH; fall back to
3333
# `python3 -m lizard` which is what `pip install --user lizard` produces

src/httpserver/detail/webserver_impl.hpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,22 @@ class webserver_impl {
539539
const char* data,
540540
size_t size) const;
541541

542+
// Mirror a register_path / register_prefix call into the v2 3-tier
543+
// route table. Distinct from upsert_v2_table_entry (which is the
544+
// on_*/route path with methods merging): this is a one-shot insert
545+
// with method_set::set_all() and no merge. Takes route_table_mutex_
546+
// internally.
547+
void register_v2_route(const detail::http_endpoint& idx,
548+
std::shared_ptr<::httpserver::http_resource> res,
549+
bool family);
550+
551+
// Map a wire-string HTTP method to mr->callback (pointer-to-member
552+
// dispatch), mr->method_enum (for is_allowed checks), and mr->has_body
553+
// (for the body-buffering branch in requests_answer_first_step).
554+
// Unrecognised methods leave the defaults in place; finalize_answer
555+
// then routes through the 405 path.
556+
static void resolve_method_callback(const char* method, modded_request* mr);
557+
542558
MHD_Result complete_request(MHD_Connection* connection, modded_request* mr,
543559
const char* version, const char* method);
544560
struct MHD_Response* get_raw_response_with_fallback(modded_request* mr);

src/webserver.cpp

Lines changed: 130 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -413,43 +413,49 @@ void webserver::register_impl_(const std::string& resource,
413413
}
414414
registered_resources_lock.unlock();
415415

416-
// TASK-027: mirror into the v2 3-tier table. Tier placement via
417-
// classify_route_tier() (single source-of-truth):
416+
impl_->register_v2_route(idx, std::move(res), family);
417+
impl_->invalidate_route_cache();
418+
}
419+
420+
namespace detail {
421+
422+
void webserver_impl::register_v2_route(const detail::http_endpoint& idx,
423+
std::shared_ptr<http_resource> res, bool family) {
424+
// TASK-027: mirror a register_path / register_prefix call into the
425+
// v2 3-tier route table. Tier placement via classify_route_tier()
426+
// (single source-of-truth):
418427
// - family=true -> radix tree (prefix terminus).
419428
// - radix tier -> radix tree (exact terminus, wildcard nodes).
420429
// - regex tier -> regex_routes_ (pre-compiled at registration time).
421430
// - exact tier -> exact_routes_ hash map.
422-
{
423-
std::unique_lock table_lock(impl_->route_table_mutex_);
424-
detail::route_entry entry;
425-
entry.methods = method_set{}.set_all();
426-
entry.handler = res;
427-
entry.is_prefix = family;
428-
429-
if (family) {
430-
impl_->param_and_prefix_routes_.insert(
431-
idx.get_url_complete(), entry, /*is_prefix=*/true);
432-
} else {
433-
auto tier = classify_route_tier(idx);
434-
switch (tier.kind) {
435-
case route_tier_kind::radix:
436-
impl_->param_and_prefix_routes_.insert(
437-
idx.get_url_complete(), entry, /*is_prefix=*/false);
438-
break;
439-
case route_tier_kind::regex_:
440-
impl_->regex_routes_.push_back(
441-
{idx.get_url_complete(), std::move(*tier.re), entry});
442-
break;
443-
case route_tier_kind::exact:
444-
impl_->exact_routes_.emplace(idx.get_url_complete(), entry);
445-
break;
446-
}
447-
}
431+
std::unique_lock table_lock(route_table_mutex_);
432+
detail::route_entry entry;
433+
entry.methods = method_set{}.set_all();
434+
entry.handler = std::move(res);
435+
entry.is_prefix = family;
436+
if (family) {
437+
param_and_prefix_routes_.insert(idx.get_url_complete(), std::move(entry),
438+
/*is_prefix=*/true);
439+
return;
440+
}
441+
auto tier = classify_route_tier(idx);
442+
switch (tier.kind) {
443+
case route_tier_kind::radix:
444+
param_and_prefix_routes_.insert(idx.get_url_complete(), std::move(entry),
445+
/*is_prefix=*/false);
446+
break;
447+
case route_tier_kind::regex_:
448+
regex_routes_.push_back(
449+
{idx.get_url_complete(), std::move(*tier.re), std::move(entry)});
450+
break;
451+
case route_tier_kind::exact:
452+
exact_routes_.emplace(idx.get_url_complete(), std::move(entry));
453+
break;
448454
}
449-
450-
impl_->invalidate_route_cache();
451455
}
452456

457+
} // namespace detail
458+
453459
void webserver::register_path(const std::string& path,
454460
std::shared_ptr<http_resource> res) {
455461
register_impl_(path, std::move(res), /*family=*/false);
@@ -1627,6 +1633,64 @@ MHD_Result webserver_impl::post_iterator(void *cls, enum MHD_ValueKind kind,
16271633
} // namespace detail
16281634

16291635
#ifdef HAVE_WEBSOCKET
1636+
namespace {
1637+
1638+
// RFC 6455 §5.5.1: a CLOSE frame's payload starts with a 2-byte
1639+
// status code (default 1000 "normal closure") followed by an optional
1640+
// UTF-8 reason. Pulled out of dispatch_websocket_frame so the switch
1641+
// stays under the CCN bar.
1642+
void handle_close_frame(websocket_handler* handler, websocket_session& session,
1643+
const char* frame_data, size_t frame_len) {
1644+
uint16_t close_code = 1000;
1645+
std::string close_reason;
1646+
if (frame_len >= 2) {
1647+
close_code = static_cast<uint16_t>(
1648+
(static_cast<unsigned char>(frame_data[0]) << 8) |
1649+
static_cast<unsigned char>(frame_data[1]));
1650+
if (frame_len > 2) {
1651+
close_reason.assign(frame_data + 2, frame_len - 2);
1652+
}
1653+
}
1654+
handler->on_close(session, close_code, close_reason);
1655+
// Echo the close back and end the loop.
1656+
session.close(close_code, close_reason);
1657+
}
1658+
1659+
void dispatch_websocket_frame(int status, struct MHD_WebSocketStream* ws_stream,
1660+
websocket_handler* handler,
1661+
websocket_session& session,
1662+
char* frame_data, size_t frame_len) {
1663+
switch (status) {
1664+
case MHD_WEBSOCKET_STATUS_TEXT_FRAME:
1665+
handler->on_message(session, std::string_view(frame_data, frame_len));
1666+
MHD_websocket_free(ws_stream, frame_data);
1667+
break;
1668+
case MHD_WEBSOCKET_STATUS_BINARY_FRAME:
1669+
handler->on_binary(session, frame_data, frame_len);
1670+
MHD_websocket_free(ws_stream, frame_data);
1671+
break;
1672+
case MHD_WEBSOCKET_STATUS_PING_FRAME:
1673+
handler->on_ping(session, std::string_view(frame_data, frame_len));
1674+
MHD_websocket_free(ws_stream, frame_data);
1675+
break;
1676+
case MHD_WEBSOCKET_STATUS_CLOSE_FRAME:
1677+
handle_close_frame(handler, session, frame_data, frame_len);
1678+
MHD_websocket_free(ws_stream, frame_data);
1679+
break;
1680+
case MHD_WEBSOCKET_STATUS_OK:
1681+
// Need more data - go back to recv.
1682+
if (frame_data != nullptr) MHD_websocket_free(ws_stream, frame_data);
1683+
break;
1684+
default:
1685+
// Protocol error or unknown frame.
1686+
if (frame_data != nullptr) MHD_websocket_free(ws_stream, frame_data);
1687+
session.close(1002, "Protocol error");
1688+
break;
1689+
}
1690+
}
1691+
1692+
} // namespace
1693+
16301694
static void decode_websocket_buffer(struct MHD_WebSocketStream* ws_stream,
16311695
websocket_handler* handler,
16321696
websocket_session& session,
@@ -1643,51 +1707,9 @@ static void decode_websocket_buffer(struct MHD_WebSocketStream* ws_stream,
16431707
&frame_data,
16441708
&frame_len);
16451709
offset += step;
1646-
switch (status) {
1647-
case MHD_WEBSOCKET_STATUS_TEXT_FRAME:
1648-
handler->on_message(session, std::string_view(frame_data, frame_len));
1649-
MHD_websocket_free(ws_stream, frame_data);
1650-
break;
1651-
case MHD_WEBSOCKET_STATUS_BINARY_FRAME:
1652-
handler->on_binary(session, frame_data, frame_len);
1653-
MHD_websocket_free(ws_stream, frame_data);
1654-
break;
1655-
case MHD_WEBSOCKET_STATUS_PING_FRAME:
1656-
handler->on_ping(session, std::string_view(frame_data, frame_len));
1657-
MHD_websocket_free(ws_stream, frame_data);
1658-
break;
1659-
case MHD_WEBSOCKET_STATUS_CLOSE_FRAME: {
1660-
uint16_t close_code = 1000;
1661-
std::string close_reason;
1662-
if (frame_len >= 2) {
1663-
close_code = static_cast<uint16_t>(
1664-
(static_cast<unsigned char>(frame_data[0]) << 8) |
1665-
static_cast<unsigned char>(frame_data[1]));
1666-
if (frame_len > 2) {
1667-
close_reason.assign(frame_data + 2, frame_len - 2);
1668-
}
1669-
}
1670-
handler->on_close(session, close_code, close_reason);
1671-
MHD_websocket_free(ws_stream, frame_data);
1672-
// Send close response and end the loop
1673-
session.close(close_code, close_reason);
1674-
break;
1675-
}
1676-
case MHD_WEBSOCKET_STATUS_OK:
1677-
// Need more data - go back to recv
1678-
if (frame_data != nullptr) {
1679-
MHD_websocket_free(ws_stream, frame_data);
1680-
}
1681-
break;
1682-
default:
1683-
// Protocol error or unknown frame
1684-
if (frame_data != nullptr) {
1685-
MHD_websocket_free(ws_stream, frame_data);
1686-
}
1687-
session.close(1002, "Protocol error");
1688-
break;
1689-
}
1690-
// If decode consumed no bytes, we need more data
1710+
dispatch_websocket_frame(status, ws_stream, handler, session,
1711+
frame_data, frame_len);
1712+
// If decode consumed no bytes, we need more data.
16911713
if (step == 0) break;
16921714
}
16931715
}
@@ -2428,34 +2450,15 @@ MHD_Result webserver_impl::complete_request(MHD_Connection* connection, struct d
24282450
return finalize_answer(connection, mr);
24292451
}
24302452

2431-
MHD_Result webserver_impl::answer_to_connection(void* cls, MHD_Connection* connection, const char* url, const char* method,
2432-
const char* version, const char* upload_data, size_t* upload_data_size, void** con_cls) {
2433-
struct detail::modded_request* mr = static_cast<struct detail::modded_request*>(*con_cls);
2434-
auto* impl = static_cast<webserver_impl*>(cls);
2435-
2436-
if (mr->dhr) {
2437-
return impl->requests_answer_second_step(connection, method, version, upload_data, upload_data_size, mr);
2438-
}
2439-
2440-
const MHD_ConnectionInfo * conninfo = MHD_get_connection_info(connection, MHD_CONNECTION_INFO_CONNECTION_FD);
2441-
2442-
if (conninfo != nullptr && impl->parent->tcp_nodelay) {
2443-
int yes = 1;
2444-
setsockopt(conninfo->connect_fd, IPPROTO_TCP, TCP_NODELAY, reinterpret_cast<char*>(&yes), sizeof(int));
2445-
}
2446-
2447-
std::string t_url = url;
2448-
2449-
base_unescaper(&t_url, impl->parent->unescaper);
2450-
mr->standardized_url = http_utils::standardize_url(t_url);
2451-
2452-
mr->has_body = false;
2453-
2454-
webserver_impl::access_log(impl->parent, mr->complete_uri + " METHOD: " + method);
2455-
2453+
void webserver_impl::resolve_method_callback(const char* method,
2454+
detail::modded_request* mr) {
24562455
// Case-sensitive per RFC 7230 §3.1.1: HTTP method is case-sensitive.
24572456
// TASK-021: also record the enum form once so finalize_answer can
24582457
// call hrm->is_allowed without re-scanning the wire string.
2458+
// Unrecognised methods leave mr->method_enum at the default
2459+
// (count_), so is_allowed(count_) returns false and the request
2460+
// takes the 405 path. Pre-existing latent bug: mr->callback may
2461+
// also be left un-set here; see TASK-027 for the dispatch redesign.
24592462
if (0 == strcmp(method, http_utils::http_method_get)) {
24602463
mr->callback = &http_resource::render_get;
24612464
mr->method_enum = http_method::get;
@@ -2488,10 +2491,33 @@ MHD_Result webserver_impl::answer_to_connection(void* cls, MHD_Connection* conne
24882491
mr->callback = &http_resource::render_options;
24892492
mr->method_enum = http_method::options;
24902493
}
2491-
// Else: mr->method_enum stays at the default (count_), so
2492-
// is_allowed(count_) returns false and the request takes the 405
2493-
// path. Pre-existing latent bug: mr->callback may also be left
2494-
// un-set here; see TASK-027 for the dispatch redesign.
2494+
}
2495+
2496+
MHD_Result webserver_impl::answer_to_connection(void* cls, MHD_Connection* connection, const char* url, const char* method,
2497+
const char* version, const char* upload_data, size_t* upload_data_size, void** con_cls) {
2498+
auto* mr = static_cast<detail::modded_request*>(*con_cls);
2499+
auto* impl = static_cast<webserver_impl*>(cls);
2500+
2501+
if (mr->dhr) {
2502+
return impl->requests_answer_second_step(connection, method, version,
2503+
upload_data, upload_data_size, mr);
2504+
}
2505+
2506+
const MHD_ConnectionInfo* conninfo =
2507+
MHD_get_connection_info(connection, MHD_CONNECTION_INFO_CONNECTION_FD);
2508+
if (conninfo != nullptr && impl->parent->tcp_nodelay) {
2509+
int yes = 1;
2510+
setsockopt(conninfo->connect_fd, IPPROTO_TCP, TCP_NODELAY,
2511+
reinterpret_cast<char*>(&yes), sizeof(int));
2512+
}
2513+
2514+
std::string t_url = url;
2515+
base_unescaper(&t_url, impl->parent->unescaper);
2516+
mr->standardized_url = http_utils::standardize_url(t_url);
2517+
mr->has_body = false;
2518+
2519+
webserver_impl::access_log(impl->parent, mr->complete_uri + " METHOD: " + method);
2520+
resolve_method_callback(method, mr);
24952521

24962522
return impl->requests_answer_first_step(connection, mr);
24972523
}

0 commit comments

Comments
 (0)