Skip to content

Commit 8464996

Browse files
etrclaude
andcommitted
refactor: split webserver_impl.hpp (674 → 330)
Step 3 of the FILE_LOC_MAX ratchet. Two extractions: connection_state -> httpserver/detail/connection_state.hpp The per-MHD_Connection PMR arena anchor (initial_buffer_ + monotonic_buffer_resource + reset_arena()). Self-contained, no webserver_impl coupling. As a bonus, http_request.cpp can eventually narrow its #include from webserver_impl.hpp down to just connection_state.hpp, dropping a heavy header (which transitively pulls microhttpd / pthread / gnutls / regex) off http_request.cpp's compile-time footprint -- not done in this commit to keep the diff focused on the LOC ratchet. dispatch / start-helper / MHD-trampoline method declarations -> httpserver/detail/webserver_impl_dispatch.hpp The 280-line tail of method declarations on webserver_impl -- start-helper overloads (add_*_mhd_options, compose_*_flags), dispatch chain (requests_answer_first/second_step, finalize_answer + its CCN-10 sub-helpers), route-lookup / route-cache helpers, auth short-circuit, post-iterator helpers, MHD trampolines, GnuTLS PSK/SNI callbacks. Same in-class-body #include pattern introduced in TASK-step-2 for http_request_auth.hpp: the sibling header carries declarations only and gates itself behind SRC_HTTPSERVER_DETAIL_WEBSERVER_IMPL_HPP_INSIDE_CLASS_, which is #define'd/#undef'd around the include inside class webserver_impl body. Standalone inclusion produces a #error. webserver_impl.hpp ends up at 330 LOC (data members + lifecycle ctors + the in-class-body include for the dispatch surface). Behaviourally inert -- no ABI change, no public surface change. The order of declarations inside class webserver_impl is preserved verbatim because the sibling is included at the original textual location. FILE_LOC_MAX stays at 2700 -- webserver.cpp (2673) still pins it. Offender list down to four files. Verification: make check 51/51 PASS (includes hygiene, install-layout, doxygen, examples, readme, release-notes) ./scripts/check-file-size.sh PASS at FILE_LOC_MAX=2700 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3d5474e commit 8464996

5 files changed

Lines changed: 457 additions & 358 deletions

File tree

scripts/check-file-size.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
# src/http_request.cpp 1175
3535
# src/httpserver/webserver.hpp 845
3636
# src/http_utils.cpp 730
37-
# src/httpserver/detail/webserver_impl.hpp 674
3837
#
3938
# FILE_LOC_MAX is pinned by the largest unfixed file (webserver.cpp at
4039
# 2673), so it cannot drop until the top offender is decomposed. The

src/Makefile.am

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp fil
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/http_request_impl.hpp httpserver/detail/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/radix_tree.hpp httpserver/detail/route_cache.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/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/radix_tree.hpp httpserver/detail/route_cache.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/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
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
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+
// Internal detail header. Strict gate: reachable only from libhttpserver
22+
// translation units, never from the public umbrella.
23+
// cppcheck-suppress-file unusedStructMember
24+
#if !defined(HTTPSERVER_COMPILATION)
25+
#error "connection_state.hpp is internal; only reachable when compiling libhttpserver."
26+
#endif
27+
28+
#ifndef SRC_HTTPSERVER_DETAIL_CONNECTION_STATE_HPP_
29+
#define SRC_HTTPSERVER_DETAIL_CONNECTION_STATE_HPP_
30+
31+
#include <cstddef>
32+
#include <cstring>
33+
34+
#include <array>
35+
#include <memory_resource>
36+
37+
namespace httpserver {
38+
namespace detail {
39+
40+
// connection_state: per-MHD_Connection arena anchor.
41+
//
42+
// Owns a std::pmr::monotonic_buffer_resource over an embedded initial
43+
// buffer. The arena is allocated once per MHD connection (in
44+
// webserver_impl::connection_notify on MHD_CONNECTION_NOTIFY_STARTED)
45+
// and torn down on MHD_CONNECTION_NOTIFY_CLOSED. Between requests on a
46+
// keep-alive connection, request_completed calls arena_.release() to
47+
// rewind the bump pointer, so a second request reuses the same memory.
48+
//
49+
// Lifetime contract for views returned by http_request getters: they
50+
// remain valid until the request-completion callback fires for the
51+
// request they were derived from. Capturing them past the user
52+
// handler's return is undefined behavior. (See architecture doc
53+
// 04-components/http-request.md.)
54+
//
55+
// Initial-buffer sizing math (8 KiB):
56+
// - sizeof(http_request_impl) ~= 600-700 B with libstdc++/libc++
57+
// map/string layouts.
58+
// - A typical small GET populates ~1.5 KiB across header_view_map,
59+
// querystring, requestor_ip; a small POST with a few args ~2.5 KiB.
60+
// - Each std::pmr::map node (unescaped_args) is ~64-96 B on
61+
// libstdc++/libc++, so 5 headers/args already consume ~400-500 B
62+
// in tree nodes alone. 4 KiB was undersized for realistic requests
63+
// with moderate arg counts; 8 KiB matches the test's own generous
64+
// buffer and covers the common case without overflow to the upstream
65+
// heap. (performance-reviewer-iter1-1.)
66+
// - Overflow spills to the upstream resource (default = heap) silently
67+
// -- it is a correctness fall-through, not a hard limit.
68+
// - TODO(M5): expose ARENA_INITIAL_BYTES via create_webserver if/when
69+
// profiling shows tuning value.
70+
struct connection_state {
71+
static constexpr std::size_t ARENA_INITIAL_BYTES = 8192;
72+
73+
// The buffer aliases storage for any PMR-aware object the arena
74+
// hands out, so it must satisfy the strictest fundamental alignment.
75+
alignas(std::max_align_t) std::array<std::byte, ARENA_INITIAL_BYTES> initial_buffer_{};
76+
77+
// upstream defaults to new_delete_resource (= get_default_resource).
78+
// We pass it explicitly to make the contract obvious in source.
79+
std::pmr::monotonic_buffer_resource arena_{
80+
initial_buffer_.data(), initial_buffer_.size(),
81+
std::pmr::new_delete_resource()};
82+
83+
connection_state() = default;
84+
connection_state(const connection_state&) = delete;
85+
connection_state& operator=(const connection_state&) = delete;
86+
connection_state(connection_state&&) = delete;
87+
connection_state& operator=(connection_state&&) = delete;
88+
89+
// reset_arena(): release the bump pointer AND zero the initial buffer.
90+
//
91+
// The plain arena_.release() rewinds the bump pointer so the next
92+
// request reuses the same memory, but it does NOT clear the reclaimed
93+
// bytes. Credentials (username, password, digested_user) written into
94+
// the arena by a previous request would therefore linger in the buffer
95+
// until overwritten by the next request's lazy-cache population.
96+
// Explicit zeroing after release() closes that residual-credential
97+
// window. (security-reviewer-iter1-3, CWE-226.)
98+
//
99+
// Using std::memset here (rather than explicit_bzero / SecureZeroMemory)
100+
// is acceptable because the buffer is accessed again immediately by the
101+
// next request's arena allocation, preventing the compiler from
102+
// optimising the clear away as a dead store.
103+
void reset_arena() noexcept {
104+
arena_.release();
105+
std::memset(initial_buffer_.data(), 0, ARENA_INITIAL_BYTES);
106+
}
107+
};
108+
109+
} // namespace detail
110+
} // namespace httpserver
111+
112+
#endif // SRC_HTTPSERVER_DETAIL_CONNECTION_STATE_HPP_

0 commit comments

Comments
 (0)