Skip to content

Commit c70b4d9

Browse files
etrclaude
andcommitted
refactor: extract ip_access_control collaborator from webserver_impl
First step of the webserver_impl decomposition. The deny/allow IP sets and their two mutexes were one of five independent state clusters living directly on the webserver_impl god-object. Move them behind a dedicated detail::ip_access_control collaborator that owns the state, both shared_mutexes, and the wildcard-precedence insert invariant. webserver_impl now holds it as 'ip_access_control acl_;'. The four public setters (webserver::{deny,allow,remove_denied,remove_allowed}_ip) forward to it; policy_callback consults it via classify(), which reads both lists under their shared locks held together, preserving the pre-extraction consistent-snapshot semantics exactly. No public API or behavioural change; build-green, 109/109 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 685ea52 commit c70b4d9

7 files changed

Lines changed: 193 additions & 50 deletions

File tree

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/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.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/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.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
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/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/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/ip_access_control.cpp

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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+
#include "httpserver/detail/ip_access_control.hpp"
22+
23+
#include <mutex>
24+
#include <set>
25+
#include <shared_mutex>
26+
#include <string>
27+
#include <string_view>
28+
29+
#include "httpserver/ip_representation.hpp"
30+
31+
namespace httpserver {
32+
namespace detail {
33+
34+
using http::ip_representation;
35+
36+
void ip_access_control::insert_wildcard_aware(
37+
std::set<ip_representation>& list, const ip_representation& t_ip) {
38+
auto it = list.find(t_ip);
39+
if (it != list.end() && t_ip.weight() < it->weight()) {
40+
list.erase(it);
41+
}
42+
list.insert(t_ip);
43+
}
44+
45+
void ip_access_control::deny(std::string_view ip) {
46+
std::unique_lock deny_lock(deny_list_mutex_);
47+
insert_wildcard_aware(deny_list_, ip_representation{std::string{ip}});
48+
}
49+
50+
void ip_access_control::remove_denied(std::string_view ip) {
51+
std::unique_lock deny_lock(deny_list_mutex_);
52+
deny_list_.erase(ip_representation{std::string{ip}});
53+
}
54+
55+
void ip_access_control::allow(std::string_view ip) {
56+
std::unique_lock allow_lock(allow_list_mutex_);
57+
insert_wildcard_aware(allow_list_, ip_representation{std::string{ip}});
58+
}
59+
60+
void ip_access_control::remove_allowed(std::string_view ip) {
61+
std::unique_lock allow_lock(allow_list_mutex_);
62+
allow_list_.erase(ip_representation{std::string{ip}});
63+
}
64+
65+
ip_access_control::membership ip_access_control::classify(
66+
const ip_representation& peer) const {
67+
std::shared_lock deny_lock(deny_list_mutex_);
68+
std::shared_lock allow_lock(allow_list_mutex_);
69+
return {deny_list_.count(peer) != 0, allow_list_.count(peer) != 0};
70+
}
71+
72+
} // namespace detail
73+
} // namespace httpserver

src/detail/ip_representation.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ bool is_v4_mapped_prefix_octet_pair(uint8_t a, uint8_t b) {
362362
// case does not apply are bytes 10-11 folded into the scores and the
363363
// totals compared.
364364
//
365-
// insert_wildcard_aware in src/detail/webserver_lifecycle.cpp relies on
365+
// insert_wildcard_aware in src/detail/ip_access_control.cpp relies on
366366
// this equivalence: std::set::find must locate a stored entry that
367367
// merely OVERLAPS the new one (wildcard-subsumed, or v4-mapped vs
368368
// plain form) so the more-permissive entry can win.

src/detail/webserver_callbacks_lifecycle.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -225,16 +225,12 @@ MHD_Result webserver_impl::policy_callback(void *cls, const struct sockaddr* add
225225
std::optional<std::string_view> reason{};
226226

227227
if (ws->config.ip_access_control_enabled) {
228-
bool is_denied = false;
229-
bool is_allowed = false;
230-
{
231-
std::shared_lock deny_lock(impl->deny_list_mutex);
232-
std::shared_lock allow_lock(impl->allow_list_mutex);
233-
is_denied = impl->deny_list.count(ip_representation(addr));
234-
is_allowed = impl->allow_list.count(ip_representation(addr));
235-
} // release deny/allow locks before firing the user hook
228+
// Consistent deny/allow snapshot under the acl's own locks, released
229+
// before the user hook fires.
230+
const auto membership = impl->acl_.classify(ip_representation(addr));
236231
std::tie(accepted, reason) =
237-
classify_decision(ws->config.default_policy, is_denied, is_allowed);
232+
classify_decision(ws->config.default_policy,
233+
membership.denied, membership.allowed);
238234
}
239235

240236
const MHD_Result decision = accepted ? MHD_YES : MHD_NO;

src/detail/webserver_lifecycle.cpp

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -460,49 +460,23 @@ bool webserver::add_connection(int client_socket, const struct sockaddr* addr, u
460460
static_cast<socklen_t>(addrlen)) == MHD_YES;
461461
}
462462

463-
// insert_wildcard_aware: shared helper for deny_ip/allow_ip. Inserts
464-
// t_ip into @p list, preserving the invariant that a less-specific
465-
// (lower weight()) wildcard entry takes precedence over a more-specific
466-
// one. When the incoming entry is less specific than a stored match, the
467-
// stored entry is erased first so the wildcard wins; when it is equal or
468-
// more specific, std::set::insert is a no-op and the existing entry is
469-
// kept. The unconditional insert covers all three cases: (1) no existing
470-
// entry — insert; (2) equal/higher weight — no-op insert; (3) lower
471-
// weight — erase first, then insert. This works only because
472-
// ip_representation::operator< treats OVERLAPPING entries as equivalent:
473-
// octets masked out on either side are excluded from the comparison, so
474-
// a wildcard and any address it matches compare equal under std::set's
475-
// ordering. find(t_ip) may therefore return a DIFFERENT stored entry —
476-
// one the new entry subsumes or collides with — not just an exact
477-
// duplicate; that comparator invariant is what the erase-then-insert
478-
// relies on. Caller holds the list's mutex.
479-
static void insert_wildcard_aware(std::set<ip_representation>& list,
480-
const ip_representation& t_ip) {
481-
auto it = list.find(t_ip);
482-
if (it != list.end() && t_ip.weight() < it->weight()) {
483-
list.erase(it);
484-
}
485-
list.insert(t_ip);
486-
}
487-
463+
// The deny/allow IP setters forward to the ip_access_control collaborator
464+
// that owns the lists and their locks (and the wildcard-precedence insert
465+
// invariant). See httpserver/detail/ip_access_control.hpp.
488466
void webserver::deny_ip(std::string_view ip) {
489-
std::unique_lock deny_lock(impl_->deny_list_mutex);
490-
insert_wildcard_aware(impl_->deny_list, ip_representation{std::string{ip}});
467+
impl_->acl_.deny(ip);
491468
}
492469

493470
void webserver::remove_denied_ip(std::string_view ip) {
494-
std::unique_lock deny_lock(impl_->deny_list_mutex);
495-
impl_->deny_list.erase(ip_representation{std::string{ip}});
471+
impl_->acl_.remove_denied(ip);
496472
}
497473

498474
void webserver::allow_ip(std::string_view ip) {
499-
std::unique_lock allow_lock(impl_->allow_list_mutex);
500-
insert_wildcard_aware(impl_->allow_list, ip_representation{std::string{ip}});
475+
impl_->acl_.allow(ip);
501476
}
502477

503478
void webserver::remove_allowed_ip(std::string_view ip) {
504-
std::unique_lock allow_lock(impl_->allow_list_mutex);
505-
impl_->allow_list.erase(ip_representation{std::string{ip}});
479+
impl_->acl_.remove_allowed(ip);
506480
}
507481

508482
} // namespace httpserver
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
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+
// IP allow/deny access-control collaborator. Internal header; only
22+
// reachable when compiling libhttpserver translation units. NOT part of
23+
// the installed surface; consumers cannot reach it through the public
24+
// umbrella.
25+
#if !defined(HTTPSERVER_COMPILATION)
26+
#error "ip_access_control.hpp is internal; only reachable when compiling libhttpserver."
27+
#endif
28+
29+
#ifndef SRC_HTTPSERVER_DETAIL_IP_ACCESS_CONTROL_HPP_
30+
#define SRC_HTTPSERVER_DETAIL_IP_ACCESS_CONTROL_HPP_
31+
32+
#include <set>
33+
#include <shared_mutex>
34+
#include <string>
35+
#include <string_view>
36+
37+
#include "httpserver/ip_representation.hpp"
38+
39+
namespace httpserver {
40+
namespace detail {
41+
42+
// Owns the deny/allow IP sets and their locks — the whole of the
43+
// webserver's IP access-control state. Extracted from webserver_impl so
44+
// that responsibility lives behind one boundary: policy_callback consults
45+
// it via classify(); webserver::{deny,allow,remove_denied,remove_allowed}_ip
46+
// mutate it. No other state cluster on webserver_impl shares these mutexes.
47+
class ip_access_control {
48+
public:
49+
// Add @p ip (an IP literal or wildcard pattern) to the deny / allow
50+
// list, preserving the wildcard-precedence invariant (see
51+
// insert_wildcard_aware). remove_* erase the exact entry previously
52+
// added. All four take the relevant list's mutex internally.
53+
void deny(std::string_view ip);
54+
void remove_denied(std::string_view ip);
55+
void allow(std::string_view ip);
56+
void remove_allowed(std::string_view ip);
57+
58+
struct membership {
59+
bool denied = false;
60+
bool allowed = false;
61+
};
62+
63+
// Consistent deny/allow snapshot for @p peer. Both lists are read with
64+
// their shared locks held together, matching the pre-extraction
65+
// policy_callback semantics (no mutation can interleave between the two
66+
// reads).
67+
[[nodiscard]] membership classify(
68+
const http::ip_representation& peer) const;
69+
70+
private:
71+
// Insert @p t_ip into @p list, preserving the invariant that a
72+
// less-specific (lower weight()) wildcard entry takes precedence over a
73+
// more-specific one. When the incoming entry is less specific than a
74+
// stored match, the stored entry is erased first so the wildcard wins;
75+
// when it is equal or more specific, std::set::insert is a no-op and the
76+
// existing entry is kept. The unconditional insert covers all three
77+
// cases: (1) no existing entry — insert; (2) equal/higher weight — no-op
78+
// insert; (3) lower weight — erase first, then insert. This works only
79+
// because ip_representation::operator< treats OVERLAPPING entries as
80+
// equivalent: octets masked out on either side are excluded from the
81+
// comparison, so a wildcard and any address it matches compare equal
82+
// under std::set's ordering. find(t_ip) may therefore return a DIFFERENT
83+
// stored entry — one the new entry subsumes or collides with — not just
84+
// an exact duplicate; that comparator invariant is what the
85+
// erase-then-insert relies on. Caller holds the list's mutex.
86+
static void insert_wildcard_aware(std::set<http::ip_representation>& list,
87+
const http::ip_representation& t_ip);
88+
89+
mutable std::shared_mutex deny_list_mutex_;
90+
std::set<http::ip_representation> deny_list_;
91+
92+
mutable std::shared_mutex allow_list_mutex_;
93+
std::set<http::ip_representation> allow_list_;
94+
};
95+
96+
} // namespace detail
97+
} // namespace httpserver
98+
99+
#endif // SRC_HTTPSERVER_DETAIL_IP_ACCESS_CONTROL_HPP_

src/httpserver/detail/webserver_impl.hpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
#include "httpserver/hook_phase.hpp"
7373
#include "httpserver/detail/connection_state.hpp"
7474
#include "httpserver/detail/http_endpoint.hpp"
75+
#include "httpserver/detail/ip_access_control.hpp"
7576
#include "httpserver/detail/segment_trie.hpp"
7677
#include "httpserver/detail/route_cache.hpp"
7778
#include "httpserver/detail/route_entry.hpp"
@@ -372,11 +373,11 @@ class webserver_impl {
372373
std::vector<phase_entry<void(const ::httpserver::connection_close_ctx&)>>
373374
hooks_connection_closed_;
374375

375-
std::shared_mutex deny_list_mutex;
376-
std::set<http::ip_representation> deny_list;
377-
378-
std::shared_mutex allow_list_mutex;
379-
std::set<http::ip_representation> allow_list;
376+
// IP allow/deny access control (deny/allow sets + their mutexes) lives
377+
// behind this collaborator. policy_callback consults it via classify();
378+
// webserver::{deny,allow,remove_denied,remove_allowed}_ip mutate it. No
379+
// other state cluster on webserver_impl shares these mutexes.
380+
ip_access_control acl_;
380381

381382
#ifdef HAVE_WEBSOCKET
382383
// shared_ptr storage. The dispatch path

0 commit comments

Comments
 (0)