From 700c5a13d7c4105238189e51ecc77089134e8768 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Wed, 7 Feb 2024 10:32:58 +0100 Subject: [PATCH] HttpServerConnection: use exceptions for error handling When a HTTP connection dies prematurely while the response is sent, `http::async_write()` sets the error code to something like broken pipe for example. When calling `async_flush()` afterwards, it sometimes happens that this never returns. This results in a resource leak as the coroutine isn't cleaned up. This commit makes the individual functions throw exceptions instead of silently ignoring the errors, resulting in the function terminating early and also resulting in an error being logged as well. --- lib/remote/httpserverconnection.cpp | 50 +++++++++++------------------ 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 44073719f..dc6492a18 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -190,10 +190,8 @@ bool EnsureValidHeaders( response.set(http::field::connection, "close"); - boost::system::error_code ec; - - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); return false; } @@ -215,10 +213,8 @@ void HandleExpect100( response.result(http::status::continue_); - boost::system::error_code ec; - - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); } } @@ -257,10 +253,8 @@ bool HandleAccessControl( response.content_length(response.body().size()); response.set(http::field::connection, "close"); - boost::system::error_code ec; - - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); return false; } @@ -288,10 +282,8 @@ bool EnsureAcceptHeader( response.content_length(response.body().size()); response.set(http::field::connection, "close"); - boost::system::error_code ec; - - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); return false; } @@ -329,10 +321,8 @@ bool EnsureAuthenticatedUser( response.content_length(response.body().size()); } - boost::system::error_code ec; - - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); return false; } @@ -423,8 +413,8 @@ bool EnsureValidBody( response.set(http::field::connection, "close"); - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); return false; } @@ -464,10 +454,8 @@ bool ProcessRequest( HttpUtility::SendJsonError(response, nullptr, 500, "Unhandled exception" , DiagnosticInformation(ex)); - boost::system::error_code ec; - - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); return true; } @@ -476,10 +464,8 @@ bool ProcessRequest( return false; } - boost::system::error_code ec; - - http::async_write(stream, response, yc[ec]); - stream.async_flush(yc[ec]); + http::async_write(stream, response, yc); + stream.async_flush(yc); return true; } @@ -574,8 +560,8 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) } } catch (const std::exception& ex) { if (!m_ShuttingDown) { - Log(LogCritical, "HttpServerConnection") - << "Unhandled exception while processing HTTP request: " << ex.what(); + Log(LogWarning, "HttpServerConnection") + << "Exception while processing HTTP request from " << m_PeerAddress << ": " << ex.what(); } }