Skip to content

Commit eea7163

Browse files
etrclaude
andcommitted
refactor: collapse webserver + webserver_impl to one class per file
With the façade/adapter carve-out in place, take webserver and webserver_impl to their ideal shape: every translation unit maps to exactly one class, and each class occupies as few files as its concerns require rather than being scattered to satisfy the old per-file line ceiling. webserver (public façade), 6 TUs -> 1: webserver_add_hook.cpp, webserver_lifecycle.cpp, webserver_register.cpp, webserver_routes.cpp, webserver_aliases.cpp -> all folded into webserver.cpp (the whole public API in one file, 552 SLOC — within the 600 carve-out). webserver_impl (MHD adapter), 2 TUs -> 1: webserver_request.cpp -> folded into webserver_callbacks.cpp (all C-ABI trampolines + the dispatch helpers in one adapter TU, 381 SLOC). webserver_impl's composition-root TU (detail/webserver_impl.cpp) is unchanged, so webserver_impl is now 2 single-class TUs (composition root + adapter). Net -6 source files. Pure code moves — no behavior change; no symbol collisions (only webserver_aliases had an anonymous namespace; the request/callbacks anon namespaces sit in different scopes). Local: builds, 113/113 tests pass, complexity + cpplint + file-size (with carve-out) gates green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015tAodxYJMEY4VxCX4dk62e
1 parent 84c5978 commit eea7163

9 files changed

Lines changed: 1248 additions & 1701 deletions

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_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
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_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_callbacks.cpp detail/webserver_body_pipeline.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_aliases.cpp

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

src/detail/webserver_callbacks.cpp

Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include <stdlib.h>
4545
#include <strings.h>
4646
#include <algorithm>
47+
#include <chrono>
4748
#include <cstring>
4849
#include <iosfwd>
4950
#include <iostream>
@@ -77,6 +78,8 @@
7778
#include "httpserver/string_utilities.hpp"
7879
#include "httpserver/detail/body.hpp"
7980
#include "httpserver/detail/connection_state.hpp"
81+
#include "httpserver/detail/path_normalize.hpp"
82+
#include "httpserver/detail/resource_hook_table.hpp"
8083

8184
#ifdef HAVE_GNUTLS
8285
#include <gnutls/gnutls.h>
@@ -576,4 +579,288 @@ MHD_Result webserver_impl::post_iterator(void *cls, enum MHD_ValueKind kind,
576579

577580
} // namespace detail
578581

582+
583+
584+
// ===== webserver_request.cpp (answer_to_connection + dispatch
585+
// helpers: resolve_method_callback / should_skip_auth / normalize_path)
586+
// ============================================================
587+
588+
589+
590+
namespace detail {
591+
592+
namespace {
593+
594+
// NOTE: the caller (should_skip_auth) must receive an already-unescaped
595+
// path (i.e., no %XX sequences remain). libhttpserver's base_unescaper()
596+
// (called in answer_to_connection) runs before should_skip_auth, so this
597+
// invariant is satisfied on the dispatch path. Double slashes (//) and
598+
// trailing slashes are collapsed automatically: empty segments between
599+
// consecutive '/' separators are skipped, producing the same result as a
600+
// single '/'.
601+
//
602+
// Path-normalization chain (per request, in order):
603+
// 1. http_utils::standardize_url (answer_to_connection) collapses
604+
// duplicate '/' runs and strips a trailing '/'.
605+
// 2. normalize_path (below), applied to the standardized URL,
606+
// resolves dot-segments ("." / "..") into a canonical absolute
607+
// path; the result is stored as mr->standardized_url and is the
608+
// single path the rest of dispatch sees.
609+
// 3. canonicalize_lookup_path, inside lookup_v2
610+
// (webserver_dispatch.cpp), canonicalizes slashes on the lookup
611+
// key (leading '/' ensured, trailing '/' stripped) so lookups hit
612+
// the same keys registration stored.
613+
// should_skip_auth re-runs normalize_path on its input (idempotent on
614+
// the already-normalized dispatch path), so the auth-skip decision and
615+
// the route lookup always agree on the same canonical path. A past
616+
// auth-bypass fix (dot-segment mismatch between auth and routing,
617+
// commit a3e53f3) depends on this agreement -- do not let the two
618+
// views diverge.
619+
//
620+
// Single pass, no per-segment heap allocation: each retained segment is
621+
// appended straight into the output buffer, and a stack of segment start
622+
// offsets lets ".." pop the previous segment by truncating the buffer
623+
// back to that offset. This runs on every request (the auth-bypass
624+
// canonicalisation, commit a3e53f3), so it avoids the vector<std::string>
625+
// of owning segments the earlier tokenize-and-rebuild form allocated.
626+
std::string normalize_path(std::string_view path) {
627+
std::string out;
628+
out.reserve(path.size() + 1);
629+
out.push_back('/');
630+
// Offsets into `out` where each retained segment begins, recorded
631+
// just BEFORE its leading separator so ".." can drop the whole "/seg"
632+
// by resizing back to the recorded offset.
633+
std::vector<std::string::size_type> seg_marks;
634+
std::string::size_type start = 0;
635+
if (!path.empty() && path[0] == '/') start = 1;
636+
while (start < path.size()) {
637+
auto end = path.find('/', start);
638+
if (end == std::string::npos) end = path.size();
639+
std::string_view seg = path.substr(start, end - start);
640+
start = end + 1;
641+
if (seg.empty() || seg == ".") continue;
642+
if (seg == "..") {
643+
if (!seg_marks.empty()) {
644+
out.resize(seg_marks.back());
645+
seg_marks.pop_back();
646+
}
647+
continue;
648+
}
649+
seg_marks.push_back(out.size());
650+
if (out.size() > 1) out.push_back('/');
651+
out.append(seg.data(), seg.size());
652+
}
653+
return out;
654+
}
655+
656+
} // namespace
657+
658+
// Pre-normalize each auth_skip_paths entry once at
659+
// webserver construction time. Entries ending in "/*" keep their
660+
// wildcard suffix; the prefix before the wildcard is normalized.
661+
// Callers (webserver::webserver) pass the raw config-bag list and
662+
// store the result on the webserver instance as a sibling to the
663+
// original `auth_skip_paths` list. Without this pre-normalization
664+
// the skip list would be matched verbatim against a normalized
665+
// request path, so non-canonical entries (e.g. "/public/",
666+
// "/a/../b") would silently never match.
667+
//
668+
// Entries containing '%' are rejected with
669+
// std::invalid_argument. Skip-path entries must be provided in
670+
// decoded form (the same form as the request path after
671+
// libhttpserver's base_unescaper() runs). A '%'-encoded entry would
672+
// never match a decoded request path and would silently bypass auth
673+
// for no route -- a misconfiguration hazard caught early here.
674+
std::vector<std::string> normalize_auth_skip_paths(
675+
const std::vector<std::string>& raw) {
676+
std::vector<std::string> out;
677+
out.reserve(raw.size());
678+
for (const auto& entry : raw) {
679+
// Reject percent-encoded entries: skip-path entries must be
680+
// provided in decoded form. A '%' in the entry indicates a
681+
// URL-encoded sequence that would never match the decoded
682+
// request path produced by libhttpserver's base_unescaper().
683+
if (entry.find('%') != std::string::npos) {
684+
throw std::invalid_argument(
685+
"auth_skip_paths entry contains a percent-encoded "
686+
"sequence ('" + entry + "'). "
687+
"Skip-path entries must be provided in decoded form "
688+
"(e.g. '/public/test', not '/public%2Ftest').");
689+
}
690+
// Wildcard suffix: strip the trailing "/*", normalize the
691+
// prefix, then re-append "/*". The special case "/*" (size
692+
// == 2) means "match every path" and is stored as-is so
693+
// should_skip_auth can recognise it with the >= 2 guard.
694+
if (entry.size() >= 2 && entry.back() == '*' &&
695+
entry[entry.size() - 2] == '/') {
696+
if (entry.size() == 2) {
697+
// "/*" -- global wildcard: matches every path.
698+
out.push_back("/*");
699+
} else {
700+
std::string prefix = entry.substr(0, entry.size() - 2);
701+
std::string normalized_prefix = normalize_path(prefix);
702+
if (normalized_prefix == "/") {
703+
// Prefix collapsed to root -- treat as "/*".
704+
out.push_back("/*");
705+
} else {
706+
out.push_back(normalized_prefix + "/*");
707+
}
708+
}
709+
continue;
710+
}
711+
out.push_back(normalize_path(entry));
712+
}
713+
return out;
714+
}
715+
716+
bool webserver_impl::should_skip_auth(std::string_view path) const {
717+
// Empty-list early-out. Servers with no
718+
// auth_skip_paths configured pay zero normalization cost. This
719+
// is the production-typical case for any server whose auth
720+
// surface either covers every route or has no auth_handler at
721+
// all.
722+
if (parent->auth_skip_paths_normalized.empty()) {
723+
return false;
724+
}
725+
726+
// Compare against the pre-normalized list (built
727+
// once at construction time) instead of re-normalizing skip-list
728+
// entries on every request. The per-request normalize_path call
729+
// on @p path remains -- the inbound URL is per-request data and
730+
// cannot be pre-normalized.
731+
std::string normalized = normalize_path(path);
732+
733+
for (const auto& skip_path : parent->auth_skip_paths_normalized) {
734+
if (skip_path == normalized) return true;
735+
// Support wildcard suffix (e.g., "/public/*").
736+
// Use >= 2 (not > 2) so the global
737+
// wildcard "/*" (size == 2) is handled. When skip_path is "/*"
738+
// the prefix is "/" and every normalized path starts with "/",
739+
// so we return true immediately for any request.
740+
if (skip_path.size() >= 2 && skip_path.back() == '*' &&
741+
skip_path[skip_path.size() - 2] == '/') {
742+
std::string_view prefix(skip_path.data(), skip_path.size() - 1);
743+
if (normalized.compare(0, prefix.size(), prefix.data(),
744+
prefix.size()) == 0) {
745+
return true;
746+
}
747+
}
748+
}
749+
return false;
750+
}
751+
752+
// requests_answer_first_step and requests_answer_second_step
753+
// live in detail/webserver_body_pipeline.cpp to keep this TU under the
754+
// 500-LOC ceiling (FILE_LOC_MAX in scripts/check-file-size.sh).
755+
756+
// finalize_answer, resolve_resource_for_request, dispatch_resource_handler,
757+
// and fire_route_resolved_gated moved to the request_dispatcher behavior
758+
// service; requests_answer_first_step / requests_answer_second_step /
759+
// complete_request moved to the request_pipeline behavior service (both
760+
// DR-014 §4.11). answer_to_connection (below) stays a webserver_impl static
761+
// MHD trampoline: it does the per-request setup (start_time, standardized_url,
762+
// method callback) and forwards into impl_->pipeline_.
763+
764+
void webserver_impl::resolve_method_callback(const char* method,
765+
detail::modded_request* mr) {
766+
// Case-sensitive per RFC 7230 §3.1.1: HTTP method is case-sensitive.
767+
// Also record the enum form once so finalize_answer can call
768+
// hrm->is_allowed without re-scanning the wire string.
769+
// Unrecognised methods leave mr->method_enum at the default
770+
// (count_), so is_allowed(count_) returns false and the request
771+
// takes the 405 path. mr->callback is left at nullptr (its
772+
// default-initializer value) for unrecognised methods; the 405 guard
773+
// in dispatch_resource_handler fires before it is ever invoked.
774+
//
775+
// Data-driven lookup table: a new HTTP method requires only one
776+
// row here (wire string, callback pointer, enum value, has_body
777+
// flag).
778+
using render_fn = http_response (http_resource::*)(const http_request&);
779+
struct method_entry {
780+
const char* wire;
781+
render_fn callback;
782+
http_method enum_val;
783+
bool has_body;
784+
};
785+
static const method_entry methods[] = {
786+
{ http_utils::http_method_get, &http_resource::render_get, http_method::get, false },
787+
{ http_utils::http_method_post, &http_resource::render_post, http_method::post, true },
788+
{ http_utils::http_method_put, &http_resource::render_put, http_method::put, true },
789+
{ http_utils::http_method_delete, &http_resource::render_delete, http_method::del, true },
790+
{ http_utils::http_method_patch, &http_resource::render_patch, http_method::patch, true },
791+
{ http_utils::http_method_head, &http_resource::render_head, http_method::head, false },
792+
{ http_utils::http_method_connect, &http_resource::render_connect, http_method::connect, false },
793+
{ http_utils::http_method_trace, &http_resource::render_trace, http_method::trace, false },
794+
{ http_utils::http_method_options, &http_resource::render_options, http_method::options, false },
795+
};
796+
for (const auto& e : methods) {
797+
if (0 == strcmp(method, e.wire)) {
798+
mr->callback = e.callback;
799+
mr->method_enum = e.enum_val;
800+
if (e.has_body) mr->has_body = true;
801+
return;
802+
}
803+
}
804+
// Unrecognised method: leave mr->callback == nullptr and
805+
// mr->method_enum == http_method::count_ (both set by modded_request
806+
// default initialiser); the 405 guard fires before callback is used.
807+
}
808+
809+
MHD_Result webserver_impl::answer_to_connection(void* cls, MHD_Connection* connection, const char* url, const char* method,
810+
const char* version, const char* upload_data, size_t* upload_data_size, void** con_cls) {
811+
auto* mr = static_cast<detail::modded_request*>(*con_cls);
812+
auto* impl = static_cast<webserver_impl*>(cls);
813+
814+
if (mr->request) {
815+
return impl->pipeline_.requests_answer_second_step(connection, method,
816+
version, upload_data, upload_data_size, mr);
817+
}
818+
819+
const MHD_ConnectionInfo* conninfo =
820+
MHD_get_connection_info(connection, MHD_CONNECTION_INFO_CONNECTION_FD);
821+
if (conninfo != nullptr && impl->parent->config.tcp_nodelay) {
822+
int yes = 1;
823+
setsockopt(conninfo->connect_fd, IPPROTO_TCP, TCP_NODELAY,
824+
reinterpret_cast<char*>(&yes), sizeof(int));
825+
}
826+
827+
// Anchor for response_sent.elapsed and
828+
// request_completed.duration. Captured here -- the earliest moment
829+
// for the request inside the dispatch path. uri_log runs earlier
830+
// but is also invoked on non-HTTP traffic (#371); answer_to_connection
831+
// is the first point where a real HTTP request is unambiguously
832+
// in flight.
833+
mr->start_time = std::chrono::steady_clock::now();
834+
// Hoist the parent-webserver back-pointer here (rather than in
835+
// complete_request) so the request_completed firing site
836+
// can reach impl_->any_hooks_ even on request_received short-circuit
837+
// paths that may not reach complete_request.
838+
mr->ws = impl->parent;
839+
840+
std::string t_url = url;
841+
base_unescaper(&t_url, impl->parent->config.unescaper);
842+
// SECURITY: collapse dot-segments ("." / "..") into the canonical
843+
// path here, at the single point where the routing/auth path is
844+
// derived. Both the route matcher (segment_trie::find via
845+
// mr->standardized_url) and should_skip_auth() must interpret the
846+
// path identically; should_skip_auth() runs the path through
847+
// normalize_path() (which pops ".."), but standardize_url() only
848+
// collapses duplicate '/' and a trailing '/'. Without this, a request
849+
// such as "/admin/../public/x" normalizes to "/public/x" for the
850+
// auth-skip check (auth skipped) yet the router still descends to the
851+
// "/admin" prefix/regex handler -- an authentication bypass. Applying
852+
// normalize_path() to the standardized URL makes the two views agree;
853+
// it is idempotent w.r.t. the normalize_path() call already in
854+
// should_skip_auth().
855+
mr->standardized_url = normalize_path(http_utils::standardize_url(t_url));
856+
mr->has_body = false;
857+
858+
// log_access is now a response_sent alias (see webserver_aliases.cpp).
859+
resolve_method_callback(method, mr);
860+
861+
return impl->pipeline_.requests_answer_first_step(connection, mr);
862+
}
863+
864+
} // namespace detail
865+
579866
} // namespace httpserver

0 commit comments

Comments
 (0)