Skip to content

Commit

Permalink
HttpServerConnection: Prefer error codes over Boost exceptions
Browse files Browse the repository at this point in the history
When run within a coroutine, exceptions on Windows may influence
bad behaviour here. Instead, we'll check for the error code
and extract the message from memory. In contrast to exceptions
which are stored on the stack frame and then return, this costs
a little more memory but simplifies the logic.

This doesn't fix the linked issue, but is related to the analysis.

refs #7431
  • Loading branch information
Michael Friedrich committed Sep 6, 2019
1 parent 1f50a70 commit a208f7b
Showing 1 changed file with 48 additions and 37 deletions.
85 changes: 48 additions & 37 deletions lib/remote/httpserverconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,20 +78,13 @@ void HttpServerConnection::Disconnect()
Log(LogInformation, "HttpServerConnection")
<< "HTTP client disconnected (from " << m_PeerAddress << ")";

try {
m_Stream->next_layer().async_shutdown(yc);
} catch (...) {
}
boost::system::error_code ec;

try {
m_Stream->lowest_layer().shutdown(m_Stream->lowest_layer().shutdown_both);
} catch (...) {
}
m_Stream->next_layer().async_shutdown(yc[ec]);

try {
m_Stream->lowest_layer().cancel();
} catch (...) {
}
m_Stream->lowest_layer().shutdown(m_Stream->lowest_layer().shutdown_both, ec);

m_Stream->lowest_layer().cancel(ec);

m_CheckLivenessTimer.cancel();

Expand Down Expand Up @@ -148,16 +141,18 @@ bool EnsureValidHeaders(
bool httpError = true;

try {
try {
http::async_read_header(stream, buf, parser, yc);
} catch (const boost::system::system_error& ex) {
boost::system::error_code ec;

http::async_read_header(stream, buf, parser, yc[ec]);

if (ec) {
/**
* Unfortunately there's no way to tell an HTTP protocol error
* from an error on a lower layer:
*
* <https://github.com/boostorg/beast/issues/643>
*/
throw std::invalid_argument(ex.what());
throw std::invalid_argument(ec.message());
}

httpError = false;
Expand Down Expand Up @@ -185,8 +180,10 @@ bool EnsureValidHeaders(

response.set(http::field::connection, "close");

http::async_write(stream, response, yc);
stream.async_flush(yc);
boost::system::error_code ec;

http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);

return false;
}
Expand All @@ -208,8 +205,10 @@ void HandleExpect100(

response.result(http::status::continue_);

http::async_write(stream, response, yc);
stream.async_flush(yc);
boost::system::error_code ec;

http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);
}
}

Expand Down Expand Up @@ -252,8 +251,10 @@ bool HandleAccessControl(
response.set(http::field::content_length, response.body().size());
response.set(http::field::connection, "close");

http::async_write(stream, response, yc);
stream.async_flush(yc);
boost::system::error_code ec;

http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);

return false;
}
Expand Down Expand Up @@ -281,8 +282,10 @@ bool EnsureAcceptHeader(
response.set(http::field::content_length, response.body().size());
response.set(http::field::connection, "close");

http::async_write(stream, response, yc);
stream.async_flush(yc);
boost::system::error_code ec;

http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);

return false;
}
Expand Down Expand Up @@ -320,8 +323,10 @@ bool EnsureAuthenticatedUser(
response.set(http::field::content_length, response.body().size());
}

http::async_write(stream, response, yc);
stream.async_flush(yc);
boost::system::error_code ec;

http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);

return false;
}
Expand Down Expand Up @@ -378,9 +383,11 @@ bool EnsureValidBody(
parser.body_limit(maxSize);
}

try {
http::async_read(stream, buf, parser, yc);
} catch (const boost::system::system_error& ex) {
boost::system::error_code ec;

http::async_read(stream, buf, parser, yc[ec]);

if (ec) {
/**
* Unfortunately there's no way to tell an HTTP protocol error
* from an error on a lower layer:
Expand All @@ -393,18 +400,18 @@ bool EnsureValidBody(
if (parser.get()[http::field::accept] == "application/json") {
HttpUtility::SendJsonBody(response, nullptr, new Dictionary({
{ "error", 400 },
{ "status", String("Bad Request: ") + ex.what() }
{ "status", String("Bad Request: ") + ec.message() }
}));
} else {
response.set(http::field::content_type, "text/html");
response.body() = String("<h1>Bad Request</h1><p><pre>") + ex.what() + "</pre></p>";
response.body() = String("<h1>Bad Request</h1><p><pre>") + ec.message() + "</pre></p>";
response.set(http::field::content_length, response.body().size());
}

response.set(http::field::connection, "close");

http::async_write(stream, response, yc);
stream.async_flush(yc);
http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);

return false;
}
Expand Down Expand Up @@ -438,8 +445,10 @@ bool ProcessRequest(

HttpUtility::SendJsonError(response, nullptr, 500, "Unhandled exception" , DiagnosticInformation(ex));

http::async_write(stream, response, yc);
stream.async_flush(yc);
boost::system::error_code ec;

http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);

return true;
}
Expand All @@ -448,8 +457,10 @@ bool ProcessRequest(
return false;
}

http::async_write(stream, response, yc);
stream.async_flush(yc);
boost::system::error_code ec;

http::async_write(stream, response, yc[ec]);
stream.async_flush(yc[ec]);

return true;
}
Expand Down

0 comments on commit a208f7b

Please sign in to comment.