Skip to content

Commit

Permalink
Fix tcp::resolver data race in the asio backend and be defensive agai…
Browse files Browse the repository at this point in the history
…nst empty results (#1339)

* Move TCP resolver to asio_context rather than asio_client, as that type is not safe to touch from multiple threads.

* Defend against empty responses from async_resolve.

Co-authored-by: Jinming Hu [email protected]
  • Loading branch information
BillyONeal authored Feb 23, 2020
1 parent 43f6f34 commit 0886fcc
Showing 1 changed file with 18 additions and 16 deletions.
34 changes: 18 additions & 16 deletions Release/src/http/client/http_client_asio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,6 @@ class asio_client final : public _http_client_communicator
public:
asio_client(http::uri&& address, http_client_config&& client_config)
: _http_client_communicator(std::move(address), std::move(client_config))
, m_resolver(crossplat::threadpool::shared_instance().service())
, m_pool(std::make_shared<asio_connection_pool>())
{
}
Expand Down Expand Up @@ -502,8 +501,6 @@ class asio_client final : public _http_client_communicator

virtual pplx::task<http_response> propagate(http_request request) override;

tcp::resolver m_resolver;

private:
const std::shared_ptr<asio_connection_pool> m_pool;
};
Expand All @@ -520,6 +517,7 @@ class asio_context final : public request_context, public std::enable_shared_fro
, m_content_length(0)
, m_needChunked(false)
, m_timer(client->client_config().timeout<std::chrono::microseconds>())
, m_resolver(crossplat::threadpool::shared_instance().service())
, m_connection(connection)
#ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE
, m_openssl_failed(false)
Expand Down Expand Up @@ -585,11 +583,11 @@ class asio_context final : public request_context, public std::enable_shared_fro
tcp::resolver::query query(utility::conversions::to_utf8string(proxy_host), to_string(proxy_port));

auto client = std::static_pointer_cast<asio_client>(m_context->m_http_client);
client->m_resolver.async_resolve(query,
boost::bind(&ssl_proxy_tunnel::handle_resolve,
shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::iterator));
m_context->m_resolver.async_resolve(query,
boost::bind(&ssl_proxy_tunnel::handle_resolve,
shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::iterator));
}

private:
Expand Down Expand Up @@ -887,12 +885,11 @@ class asio_context final : public request_context, public std::enable_shared_fro
auto tcp_port = proxy_type == http_proxy_type::http ? proxy_port : port;

tcp::resolver::query query(tcp_host, to_string(tcp_port));
auto client = std::static_pointer_cast<asio_client>(ctx->m_http_client);
client->m_resolver.async_resolve(query,
boost::bind(&asio_context::handle_resolve,
ctx,
boost::asio::placeholders::error,
boost::asio::placeholders::iterator));
ctx->m_resolver.async_resolve(query,
boost::bind(&asio_context::handle_resolve,
ctx,
boost::asio::placeholders::error,
boost::asio::placeholders::iterator));
}

// Register for notification on cancellation to abort this request.
Expand Down Expand Up @@ -1053,6 +1050,10 @@ class asio_context final : public request_context, public std::enable_shared_fro
{
report_error("Error resolving address", ec, httpclient_errorcode_context::connect);
}
else if (endpoints == tcp::resolver::iterator())
{
report_error("Failed to resolve address", ec, httpclient_errorcode_context::connect);
}
else
{
m_timer.reset();
Expand Down Expand Up @@ -1452,8 +1453,8 @@ class asio_context final : public request_context, public std::enable_shared_fro
}
}

m_content_length = (std::numeric_limits<size_t>::max)(); // Without Content-Length header, size should be same as
// TCP stream - set it size_t max.
m_content_length = (std::numeric_limits<size_t>::max)(); // Without Content-Length header, size should be same
// as TCP stream - set it size_t max.
m_response.headers().match(header_names::content_length, m_content_length);

if (!this->handle_compression())
Expand Down Expand Up @@ -1936,6 +1937,7 @@ class asio_context final : public request_context, public std::enable_shared_fro
uint64_t m_content_length;
bool m_needChunked;
timeout_timer m_timer;
tcp::resolver m_resolver;
boost::asio::streambuf m_body_buf;
std::shared_ptr<asio_connection> m_connection;

Expand Down

0 comments on commit 0886fcc

Please sign in to comment.