mirror of
https://github.com/yhirose/cpp-httplib.git
synced 2026-10-18 11:09:39 +00:00
Stop a throwing user callback from terminating the server (#2564)
Server::process_request() wraps only routing() in a try/catch. Everything else the user supplies runs outside it: - the content provider, from write_response_core() - post_routing_handler_, error_handler_, logger_ - expect_100_continue_handler_ - a WebSocket handler, and pre_routing_handler_ on the upgrade path An exception from any of those unwinds out of process_and_close_socket() into the task queue, which calls the job without a catch, so it reaches the top of a pool thread and terminates the process. One handler that throws takes down every other connection the server is holding. Add Server::serve_guarded() and run the serving loop through it in both process_and_close_socket() overloads. The exception is not turned into a 500: by the time a content provider runs, the status line and headers are already on the wire, so there is nothing left to replace. Report it through the error logger as Error::UserCallbackException and drop the connection, which is what the peer observes regardless. Requests on other connections are unaffected, and the socket is still drained and closed - which unwinding used to skip on the non-SSL path, since drain_and_close_socket() sits after the call rather than in a scope guard. The error logger is a user callback too, so the report inside the guard is itself wrapped: a throwing logger must not be able to open the guard back up. Adds ServerExceptionTest: a throwing content provider, post-routing handler, WebSocket handler and error logger, plus the content provider case against SSLServer, each checking that a later request on a new connection still succeeds. Every test runs the server on a single worker thread, so a guard that catches the exception but still loses the thread shows up as the follow-up request never being served. Note that all of them abort the test binary without this change - which is the bug, but it means a regression here fails the run rather than one test.
This commit is contained in:
1 parent
ae417b405a
commit
2addb41089
2 files changed
+328
-19
No files matched your search
@@ -1834,6 +1834,7 @@ enum class Error {
|
||||
InvalidRangeHeader,
|
||||
UnsupportedContentEncoding,
|
||||
WebSocketHandshake,
|
||||
UserCallbackException,
|
||||
|
||||
// For internal use only
|
||||
SSLPeerCouldBeClosed_,
|
||||
@@ -2224,6 +2225,35 @@ protected:
|
||||
const std::function<void(Request &)> &setup_request,
|
||||
bool *websocket_upgraded = nullptr);
|
||||
|
||||
// Runs the per-connection serving loop and stops an exception thrown by a
|
||||
// user callback from escaping the worker thread.
|
||||
//
|
||||
// process_request() wraps only routing() in a try/catch. Content providers,
|
||||
// the post-routing, error, logging and expect-100 handlers and WebSocket
|
||||
// handlers all run outside it, and the task queue calls the job without a
|
||||
// catch, so an exception from any of those would terminate the process.
|
||||
//
|
||||
// No 500 is possible here: by the time a content provider runs, the status
|
||||
// line and headers are already on the wire. Report it through the error
|
||||
// logger and drop the connection, which is what the peer observes either
|
||||
// way. Other connections are unaffected.
|
||||
template <typename Serve> bool serve_guarded(Serve &&serve) const {
|
||||
#ifdef CPPHTTPLIB_NO_EXCEPTIONS
|
||||
return serve();
|
||||
#else
|
||||
try {
|
||||
return serve();
|
||||
} catch (...) {
|
||||
// The error logger is a user callback too, so it must not be able to
|
||||
// throw the guard back open.
|
||||
try {
|
||||
output_error_log(Error::UserCallbackException, nullptr);
|
||||
} catch (...) {}
|
||||
return false;
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
std::atomic<socket_t> svr_sock_{INVALID_SOCKET};
|
||||
|
||||
std::vector<std::string> trusted_proxies_;
|
||||
@@ -10740,6 +10770,7 @@ inline std::string to_string(const Error error) {
|
||||
case Error::InvalidRangeHeader: return "Invalid Range header";
|
||||
case Error::UnsupportedContentEncoding: return "Unsupported Content-Encoding";
|
||||
case Error::WebSocketHandshake: return "WebSocket handshake failed";
|
||||
case Error::UserCallbackException: return "User callback threw an exception";
|
||||
default: break;
|
||||
}
|
||||
|
||||
@@ -14159,15 +14190,18 @@ inline bool Server::process_and_close_socket(socket_t sock) {
|
||||
detail::get_local_ip_and_port(sock, local_addr, local_port);
|
||||
|
||||
bool websocket_upgraded = false;
|
||||
auto ret = detail::process_server_socket(
|
||||
svr_sock_, sock, keep_alive_max_count_, keep_alive_timeout_sec_,
|
||||
read_timeout_sec_, read_timeout_usec_, write_timeout_sec_,
|
||||
write_timeout_usec_,
|
||||
[&](Stream &strm, bool close_connection, bool &connection_closed) {
|
||||
return process_request(strm, remote_addr, remote_port, local_addr,
|
||||
local_port, close_connection, connection_closed,
|
||||
nullptr, &websocket_upgraded);
|
||||
});
|
||||
auto ret = serve_guarded([&]() {
|
||||
return detail::process_server_socket(
|
||||
svr_sock_, sock, keep_alive_max_count_, keep_alive_timeout_sec_,
|
||||
read_timeout_sec_, read_timeout_usec_, write_timeout_sec_,
|
||||
write_timeout_usec_,
|
||||
[&](Stream &strm, bool close_connection, bool &connection_closed) {
|
||||
return process_request(strm, remote_addr, remote_port, local_addr,
|
||||
local_port, close_connection,
|
||||
connection_closed, nullptr,
|
||||
&websocket_upgraded);
|
||||
});
|
||||
});
|
||||
|
||||
detail::drain_and_close_socket(sock);
|
||||
return ret;
|
||||
@@ -17567,16 +17601,18 @@ inline bool SSLServer::process_and_close_socket(socket_t sock) {
|
||||
int local_port = 0;
|
||||
detail::get_local_ip_and_port(sock, local_addr, local_port);
|
||||
|
||||
ret = detail::process_server_socket_ssl(
|
||||
svr_sock_, session, sock, keep_alive_max_count_, keep_alive_timeout_sec_,
|
||||
read_timeout_sec_, read_timeout_usec_, write_timeout_sec_,
|
||||
write_timeout_usec_,
|
||||
[&](Stream &strm, bool close_connection, bool &connection_closed) {
|
||||
return process_request(
|
||||
strm, remote_addr, remote_port, local_addr, local_port,
|
||||
close_connection, connection_closed,
|
||||
[&](Request &req) { req.ssl = session; }, &websocket_upgraded);
|
||||
});
|
||||
ret = serve_guarded([&]() {
|
||||
return detail::process_server_socket_ssl(
|
||||
svr_sock_, session, sock, keep_alive_max_count_,
|
||||
keep_alive_timeout_sec_, read_timeout_sec_, read_timeout_usec_,
|
||||
write_timeout_sec_, write_timeout_usec_,
|
||||
[&](Stream &strm, bool close_connection, bool &connection_closed) {
|
||||
return process_request(
|
||||
strm, remote_addr, remote_port, local_addr, local_port,
|
||||
close_connection, connection_closed,
|
||||
[&](Request &req) { req.ssl = session; }, &websocket_upgraded);
|
||||
});
|
||||
});
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user