From 0886fcc138038152d7c9f54365b7976038364ab7 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Sun, 23 Feb 2020 10:51:19 -0800 Subject: [PATCH] Fix tcp::resolver data race in the asio backend and be defensive against 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 jinming.hu@microsoft.com --- Release/src/http/client/http_client_asio.cpp | 34 +++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index ad4238afc8..215b03e58a 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -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()) { } @@ -502,8 +501,6 @@ class asio_client final : public _http_client_communicator virtual pplx::task propagate(http_request request) override; - tcp::resolver m_resolver; - private: const std::shared_ptr m_pool; }; @@ -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()) + , m_resolver(crossplat::threadpool::shared_instance().service()) , m_connection(connection) #ifdef CPPREST_PLATFORM_ASIO_CERT_VERIFICATION_AVAILABLE , m_openssl_failed(false) @@ -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(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: @@ -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(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. @@ -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(); @@ -1452,8 +1453,8 @@ class asio_context final : public request_context, public std::enable_shared_fro } } - m_content_length = (std::numeric_limits::max)(); // Without Content-Length header, size should be same as - // TCP stream - set it size_t max. + m_content_length = (std::numeric_limits::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()) @@ -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 m_connection;