Skip to content

Commit ef48ed2

Browse files
etrclaude
etr
and
claude
committed
refactor(dr-014): extract error_pages behavior service (leaf)
First behavior extraction of DR-014. Move the 404/405/500 synthesis logic (not_found_page, method_not_allowed_page, internal_error_page, run_internal_error_handler_safely) out of webserver_impl into a new detail::error_pages service that holds only a const webserver_config& (no parent back-pointer, no state, no locks). Extract log_dispatch_error as a shared free function in detail/dispatch_util.{hpp,cpp} so every error path can log without a dependency edge on error_pages. webserver_impl gains an error_pages errors_ member (bound to parent->config at construction); the former detail/webserver_error_pages.cpp becomes thin webserver_impl forwarders so existing in-class call sites compile unchanged during the migration (removed in the final slim step). Also make modded_request.hpp self-contained by forward-declaring http_resource (it forms a pointer-to-member and a weak_ptr on it) so the new dispatch-pipeline TUs can include it directly. 113/113 tests pass; cpplint/complexity/file-size gates green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c7b969f commit ef48ed2

9 files changed

Lines changed: 350 additions & 103 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/ip_access_control.cpp detail/ws_registry.cpp detail/hook_bus.cpp detail/route_table.cpp detail/daemon_lifecycle.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/ws_registry.cpp detail/hook_bus.cpp detail/route_table.cpp detail/daemon_lifecycle.cpp detail/dispatch_util.cpp detail/error_pages.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/ip_access_control.hpp httpserver/detail/ws_registry.hpp httpserver/detail/hook_bus.hpp httpserver/detail/route_table.hpp httpserver/detail/daemon_lifecycle.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/ws_registry.hpp httpserver/detail/hook_bus.hpp httpserver/detail/route_table.hpp httpserver/detail/daemon_lifecycle.hpp httpserver/detail/dispatch_util.hpp httpserver/detail/error_pages.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/dispatch_util.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
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/dispatch_util.hpp"
22+
23+
#include <string>
24+
#include <string_view>
25+
26+
#include "httpserver/create_webserver.hpp"
27+
28+
namespace httpserver {
29+
namespace detail {
30+
31+
void log_dispatch_error(const webserver_config& config,
32+
std::string_view msg) noexcept {
33+
if (config.log_error == nullptr) {
34+
return;
35+
}
36+
// A misbehaving user logger must not poison the catch from inside the
37+
// catch. Swallow any exception it throws; we have no recovery beyond
38+
// dropping the log line.
39+
try {
40+
config.log_error(std::string(msg));
41+
} catch (...) {
42+
// Intentionally suppressed.
43+
}
44+
}
45+
46+
} // namespace detail
47+
} // namespace httpserver

‎src/detail/error_pages.cpp‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
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+
// error_pages behavior service (DR-014 §4.11). Logic moved verbatim out
22+
// of the former detail/webserver_error_pages.cpp; that TU now holds only
23+
// the thin webserver_impl forwarders during the migration.
24+
25+
#include "httpserver/detail/error_pages.hpp"
26+
27+
#include <string>
28+
#include <string_view>
29+
30+
#include "httpserver/constants.hpp"
31+
#include "httpserver/create_webserver.hpp"
32+
#include "httpserver/http_request.hpp"
33+
#include "httpserver/http_response.hpp"
34+
#include "httpserver/http_utils.hpp"
35+
#include "httpserver/detail/dispatch_util.hpp"
36+
#include "httpserver/detail/modded_request.hpp"
37+
38+
namespace httpserver {
39+
40+
using httpserver::http::http_utils;
41+
42+
namespace detail {
43+
44+
http_response error_pages::not_found_page(modded_request* mr) const {
45+
if (config_.not_found_handler != nullptr) {
46+
return config_.not_found_handler(*mr->request);
47+
}
48+
return http_response::string(std::string{constants::NOT_FOUND_ERROR})
49+
.with_status(http_utils::http_not_found);
50+
}
51+
52+
http_response error_pages::method_not_allowed_page(modded_request* mr) const {
53+
if (config_.method_not_allowed_handler != nullptr) {
54+
return config_.method_not_allowed_handler(*mr->request);
55+
}
56+
return http_response::string(std::string{constants::METHOD_ERROR})
57+
.with_status(http_utils::http_method_not_allowed);
58+
}
59+
60+
http_response error_pages::internal_error_page(modded_request* mr,
61+
std::string_view msg,
62+
bool force_our) const {
63+
// The double-fault fallback. Used when the user-supplied
64+
// internal_error_handler itself threw or when the belt-and-suspenders
65+
// site after get_raw_response_with_fallback fires. The body is
66+
// intentionally empty and the message is intentionally ignored.
67+
if (force_our) {
68+
return http_response::empty()
69+
.with_status(http_utils::http_internal_server_error);
70+
}
71+
// Invoke the user handler with the originating message.
72+
if (config_.internal_error_handler != nullptr) {
73+
return config_.internal_error_handler(*mr->request, msg);
74+
}
75+
// The default body is the fixed string "Internal Server Error" to
76+
// avoid CWE-209 information disclosure of e.what() text (which
77+
// routinely embeds file paths, SQL fragments, internal identifiers,
78+
// attacker-influenced input). The originating message is still
79+
// surfaced via the configured log_error callback (see
80+
// log_dispatch_error). Application code that needs the v1 verbose body
81+
// (for development) must opt in via
82+
// create_webserver::expose_exception_messages(true).
83+
const auto status = http_utils::http_internal_server_error;
84+
if (config_.expose_exception_messages) {
85+
return http_response::string(std::string{msg}).with_status(status);
86+
}
87+
return http_response::string(
88+
std::string{constants::INTERNAL_SERVER_ERROR})
89+
.with_status(status);
90+
}
91+
92+
http_response error_pages::run_internal_error_handler_safely(
93+
modded_request* mr,
94+
std::string_view msg) const {
95+
try {
96+
return internal_error_page(mr, msg, /*force_our=*/false);
97+
} catch (...) {
98+
// The user handler itself threw. Log generically and return an
99+
// empty-body 500. No exception escapes from here.
100+
log_dispatch_error(config_,
101+
"internal_error_handler threw; "
102+
"sending hardcoded empty-body 500");
103+
return internal_error_page(mr, "", /*force_our=*/true);
104+
}
105+
}
106+
107+
} // namespace detail
108+
} // namespace httpserver

‎src/detail/webserver_error_pages.cpp‎

Lines changed: 22 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -18,127 +18,50 @@
1818
USA
1919
*/
2020

21-
// Error-page helpers, split out of detail/webserver_dispatch.cpp to
22-
// keep that TU under the per-file LOC ceiling (FILE_LOC_MAX in
23-
// scripts/check-file-size.sh).
24-
//
25-
// Five small functions live here: the three synth-response helpers
26-
// (not_found_page, method_not_allowed_page, internal_error_page), the
27-
// guarded user-logger forwarder (log_dispatch_error), and the
28-
// double-fault-safe wrapper around the user internal_error_handler
29-
// (run_internal_error_handler_safely). They are pure const helpers off
30-
// webserver_impl and share no state with the rest of the dispatch TU
31-
// beyond `parent->*` user handlers and `mr->request`.
21+
// Thin webserver_impl forwarders into the error_pages behavior service
22+
// and the log_dispatch_error free function (DR-014 §4.11). The real logic
23+
// moved to detail/error_pages.cpp and detail/dispatch_util.cpp. These
24+
// forwarders keep the existing in-class call sites
25+
// (not_found_page(mr) / log_dispatch_error(msg) / ...) compiling
26+
// unchanged during the migration; they are removed once every caller is
27+
// itself a service holding error_pages& / the config bag directly.
3228

3329
#include "httpserver/webserver.hpp"
3430
#include "httpserver/detail/webserver_impl.hpp"
3531

36-
#include <string>
3732
#include <string_view>
3833

39-
#include "httpserver/constants.hpp"
4034
#include "httpserver/create_webserver.hpp"
41-
#include "httpserver/detail/modded_request.hpp"
42-
#include "httpserver/http_request.hpp"
4335
#include "httpserver/http_response.hpp"
44-
#include "httpserver/http_utils.hpp"
36+
#include "httpserver/detail/dispatch_util.hpp"
4537

4638
namespace httpserver {
47-
48-
using httpserver::http::http_utils;
49-
5039
namespace detail {
5140

5241
http_response webserver_impl::not_found_page(detail::modded_request* mr) const {
53-
if (parent->config.not_found_handler != nullptr) {
54-
return parent->config.not_found_handler(*mr->request);
55-
}
56-
return http_response::string(std::string{constants::NOT_FOUND_ERROR})
57-
.with_status(http_utils::http_not_found);
42+
return errors_.not_found_page(mr);
5843
}
5944

60-
http_response webserver_impl::method_not_allowed_page(detail::modded_request* mr) const {
61-
if (parent->config.method_not_allowed_handler != nullptr) {
62-
return parent->config.method_not_allowed_handler(*mr->request);
63-
}
64-
return http_response::string(std::string{constants::METHOD_ERROR})
65-
.with_status(http_utils::http_method_not_allowed);
45+
http_response webserver_impl::method_not_allowed_page(
46+
detail::modded_request* mr) const {
47+
return errors_.method_not_allowed_page(mr);
6648
}
6749

68-
http_response webserver_impl::internal_error_page(
69-
detail::modded_request* mr,
70-
std::string_view msg,
71-
bool force_our) const {
72-
// The double-fault fallback. Used
73-
// when the user-supplied internal_error_handler itself threw or
74-
// when the belt-and-suspenders site after
75-
// get_raw_response_with_fallback fires. The body is intentionally
76-
// empty and the message is intentionally ignored.
77-
if (force_our) {
78-
return http_response::empty()
79-
.with_status(http_utils::http_internal_server_error);
80-
}
81-
// Invoke the user handler with the originating message.
82-
if (parent->config.internal_error_handler != nullptr) {
83-
return parent->config.internal_error_handler(*mr->request, msg);
84-
}
85-
// The default body is the fixed string
86-
// "Internal Server Error" to avoid CWE-209 information disclosure of
87-
// e.what() text (which routinely embeds file paths, SQL fragments,
88-
// internal identifiers, attacker-influenced input). The originating
89-
// message is still surfaced via the configured log_error callback
90-
// (see log_dispatch_error). Application code that needs the v1
91-
// verbose body (for development) must opt in via
92-
// create_webserver::expose_exception_messages(true).
93-
const auto status = http_utils::http_internal_server_error;
94-
if (parent->config.expose_exception_messages) {
95-
return http_response::string(std::string{msg}).with_status(status);
96-
}
97-
return http_response::string(
98-
std::string{constants::INTERNAL_SERVER_ERROR})
99-
.with_status(status);
50+
http_response webserver_impl::internal_error_page(detail::modded_request* mr,
51+
std::string_view msg,
52+
bool force_our) const {
53+
return errors_.internal_error_page(mr, msg, force_our);
10054
}
10155

102-
void webserver_impl::log_dispatch_error(std::string_view msg) const noexcept {
103-
if (parent->config.log_error == nullptr) {
104-
return;
105-
}
106-
// msg is forwarded VERBATIM regardless
107-
// of create_webserver::expose_exception_messages. The error log is
108-
// the canonical destination for the verbatim exception text; only
109-
// the HTTP response body path is sanitized.
110-
//
111-
// Framework contract (CWE-532): msg may contain e.what() text from
112-
// a handler exception, which could include sensitive data (DB connection
113-
// strings, file paths, user-supplied input that triggered the exception).
114-
// Application code should sanitize or wrap exceptions that might expose
115-
// sensitive information before re-throwing them.
116-
//
117-
// A misbehaving user logger must not poison the catch from inside
118-
// the catch. Swallow any exception it throws; we have no recovery
119-
// beyond dropping the log line.
120-
try {
121-
parent->config.log_error(std::string(msg));
122-
} catch (...) {
123-
// Intentionally suppressed.
124-
}
56+
http_response webserver_impl::run_internal_error_handler_safely(
57+
detail::modded_request* mr,
58+
std::string_view msg) const {
59+
return errors_.run_internal_error_handler_safely(mr, msg);
12560
}
12661

127-
http_response
128-
webserver_impl::run_internal_error_handler_safely(
129-
detail::modded_request* mr,
130-
std::string_view msg) const {
131-
try {
132-
return internal_error_page(mr, msg, /*force_our=*/false);
133-
} catch (...) {
134-
// The user handler itself threw. Log generically
135-
// and return an empty-body 500. No exception escapes from here.
136-
log_dispatch_error("internal_error_handler threw; "
137-
"sending hardcoded empty-body 500");
138-
return internal_error_page(mr, "", /*force_our=*/true);
139-
}
62+
void webserver_impl::log_dispatch_error(std::string_view msg) const noexcept {
63+
detail::log_dispatch_error(parent->config, msg);
14064
}
14165

14266
} // namespace detail
143-
14467
} // namespace httpserver

‎src/httpserver/detail/dispatch_util.hpp‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
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+
// Shared dispatch-path free functions (DR-014). These are pure helpers
22+
// with no instance state, so they live as free functions in
23+
// httpserver::detail rather than as methods on a service class. Keeping
24+
// log_dispatch_error here (rather than on error_pages) avoids forcing
25+
// every service's error path to take a dependency edge on error_pages.
26+
//
27+
// Internal header; only reachable when compiling libhttpserver.
28+
#if !defined(HTTPSERVER_COMPILATION)
29+
#error "dispatch_util.hpp is internal; only reachable when compiling libhttpserver."
30+
#endif
31+
32+
#ifndef SRC_HTTPSERVER_DETAIL_DISPATCH_UTIL_HPP_
33+
#define SRC_HTTPSERVER_DETAIL_DISPATCH_UTIL_HPP_
34+
35+
#include <string_view>
36+
37+
namespace httpserver {
38+
39+
struct webserver_config;
40+
41+
namespace detail {
42+
43+
// Forward @p msg VERBATIM to config.log_error if a logger is configured;
44+
// a no-op when none is. Swallows any exception thrown by the user logger
45+
// so a misbehaving logger cannot poison a catch from inside the catch
46+
// (noexcept). @p msg is forwarded unchanged regardless of
47+
// create_webserver::expose_exception_messages — the error log is the
48+
// canonical destination for verbatim exception text; only the HTTP
49+
// response body path is sanitized (CWE-209 / CWE-532; see error_pages).
50+
void log_dispatch_error(const webserver_config& config,
51+
std::string_view msg) noexcept;
52+
53+
} // namespace detail
54+
} // namespace httpserver
55+
56+
#endif // SRC_HTTPSERVER_DETAIL_DISPATCH_UTIL_HPP_

0 commit comments

Comments
 (0)