Skip to content

Commit 6039ee9

Browse files
glynosglynos
committed
Made changes to the error handling in the client; attempted to fix an apparent bug in the client_options copy constructor.
1 parent 86523b9 commit 6039ee9

File tree

11 files changed

+96
-73
lines changed

11 files changed

+96
-73
lines changed

‎http/src/http/v2/client/client.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,6 @@ namespace network {
160160
tcp::resolver::iterator endpoint_iterator,
161161
std::shared_ptr<request_helper> helper) {
162162
if (ec) {
163-
if (endpoint_iterator == tcp::resolver::iterator()) {
164-
helper->response_promise_.set_exception(
165-
std::make_exception_ptr(client_exception(client_error::host_not_found)));
166-
return;
167-
}
168-
169163
helper->response_promise_.set_exception(
170164
std::make_exception_ptr(std::system_error(ec.value(), std::system_category())));
171165
return;
@@ -344,9 +338,7 @@ namespace network {
344338
client::client(std::unique_ptr<client_connection::async_resolver> mock_resolver,
345339
std::unique_ptr<client_connection::async_connection> mock_connection,
346340
client_options options)
347-
: pimpl_(new impl(std::move(mock_resolver), std::move(mock_connection), options)) {
348-
349-
}
341+
: pimpl_(new impl(std::move(mock_resolver), std::move(mock_connection), options)) { }
350342

351343
client::~client() noexcept {
352344
delete pimpl_;

‎http/src/http/v2/client/client_errors.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ namespace network {
3838

3939
case client_error::invalid_request:
4040
return "Invalid HTTP request.";
41-
case client_error::host_not_found:
42-
return "Unable to resolve host.";
4341
case client_error::invalid_response:
4442
return "Invalid HTTP response.";
4543
default:

‎http/src/network/http/v2/client/client.hpp

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <boost/asio/io_service.hpp>
2424
#include <boost/optional.hpp>
2525
#include <network/config.hpp>
26+
#include <network/version.hpp>
2627
#include <network/http/v2/client/request.hpp>
2728
#include <network/http/v2/client/response.hpp>
2829

@@ -51,17 +52,37 @@ namespace network {
5152
, follow_redirects_(false)
5253
, cache_resolved_(false)
5354
, use_proxy_(false)
55+
, always_verify_peer_(false)
56+
, user_agent_(std::string("cpp-netlib/") + NETLIB_VERSION)
5457
, timeout_(30000) { }
5558

5659
/**
5760
* \brief Copy constructor.
5861
*/
59-
client_options(client_options const &) = default;
62+
client_options(const client_options &other)
63+
: io_service_(other.io_service_)
64+
, follow_redirects_(other.follow_redirects_)
65+
, cache_resolved_(other.cache_resolved_)
66+
, use_proxy_(other.use_proxy_)
67+
, always_verify_peer_(other.always_verify_peer_)
68+
, user_agent_(other.user_agent_)
69+
, timeout_(other.timeout_)
70+
, openssl_certificate_paths_(other.openssl_certificate_paths_)
71+
, openssl_verify_paths_(other.openssl_verify_paths_) { }
6072

6173
/**
6274
* \brief Move constructor.
6375
*/
64-
client_options(client_options &&) = default;
76+
client_options(client_options &&other)
77+
: io_service_(std::move(io_service_))
78+
, follow_redirects_(std::move(other.follow_redirects_))
79+
, cache_resolved_(std::move(other.cache_resolved_))
80+
, use_proxy_(std::move(other.use_proxy_))
81+
, always_verify_peer_(std::move(other.always_verify_peer_))
82+
, user_agent_(std::move(other.user_agent_))
83+
, timeout_(std::move(other.timeout_))
84+
, openssl_certificate_paths_(std::move(other.openssl_certificate_paths_))
85+
, openssl_verify_paths_(std::move(other.openssl_verify_paths_)) { }
6586

6687
/**
6788
* \brief Assignment operator.
@@ -87,6 +108,8 @@ namespace network {
87108
swap(follow_redirects_, other.follow_redirects_);
88109
swap(cache_resolved_, other.cache_resolved_);
89110
swap(use_proxy_, other.use_proxy_);
111+
swap(always_verify_peer_, other.always_verify_peer_);
112+
swap(user_agent_, other.user_agent_);
90113
swap(timeout_, other.timeout_);
91114
swap(openssl_certificate_paths_, other.openssl_certificate_paths_);
92115
swap(openssl_verify_paths_, other.openssl_verify_paths_);
@@ -220,12 +243,32 @@ namespace network {
220243
return openssl_verify_paths_;
221244
}
222245

246+
client_options &always_verify_peer(bool always_verify_peer) {
247+
always_verify_peer_ = always_verify_peer;
248+
return *this;
249+
}
250+
251+
bool always_verify_peer() const {
252+
return always_verify_peer_;
253+
}
254+
255+
client_options &user_agent(const std::string &user_agent) {
256+
user_agent_ = user_agent;
257+
return *this;
258+
}
259+
260+
std::string user_agent() const {
261+
return user_agent_;
262+
}
263+
223264
private:
224265

225266
boost::optional<boost::asio::io_service &> io_service_;
226267
bool follow_redirects_;
227268
bool cache_resolved_;
228269
bool use_proxy_;
270+
bool always_verify_peer_;
271+
std::string user_agent_;
229272
std::chrono::milliseconds timeout_;
230273
std::vector<std::string> openssl_certificate_paths_;
231274
std::vector<std::string> openssl_verify_paths_;

‎http/src/network/http/v2/client/client_errors.hpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ namespace network {
2828
// request
2929
invalid_request,
3030

31-
// connection
32-
host_not_found,
33-
3431
// response
3532
invalid_response,
3633
};

‎http/src/network/http/v2/client/request.hpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#include <boost/range/iterator_range.hpp>
2626
#include <boost/range/algorithm/equal.hpp>
2727
#include <boost/range/as_literal.hpp>
28+
#include <boost/algorithm/string/predicate.hpp>
29+
#include <boost/optional.hpp>
2830
#include <network/config.hpp>
2931
#include <network/http/v2/method.hpp>
3032
#include <network/http/v2/client/client_errors.hpp>
@@ -395,6 +397,15 @@ namespace network {
395397
return *this;
396398
}
397399

400+
boost::optional<string_type> header(const string_type &name) {
401+
for (auto header : headers_) {
402+
if (boost::iequals(header.first, name)) {
403+
return header.second;
404+
}
405+
}
406+
return boost::optional<string_type>();
407+
}
408+
398409
/**
399410
* \brief Returns the headers range.
400411
* \returns An iterator range covering all headers.
@@ -410,10 +421,10 @@ namespace network {
410421
* If the header name can not be found, nothing happens. If
411422
* the header is duplicated, then both entries are removed.
412423
*/
413-
void remove_header(string_type name) {
424+
void remove_header(const string_type &name) {
414425
auto it = std::remove_if(std::begin(headers_), std::end(headers_),
415426
[&name] (const std::pair<string_type, string_type> &header) {
416-
return header.first == name;
427+
return boost::iequals(header.first, name);
417428
});
418429
headers_.erase(it, std::end(headers_));
419430
}

‎http/test/v2/client/features/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
set(CPP-NETLIB_CLIENT_TESTS
77
async_resolver_test
88
normal_connection_test
9-
client_test
9+
http_client_test
1010
)
1111

1212
#if (OPENSSL_FOUND)

‎http/test/v2/client/features/client_test.cpp

Lines changed: 0 additions & 49 deletions
This file was deleted.

‎http/test/v2/client/units/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
set(CPP-NETLIB_CLIENT_TESTS
77
client_options_test
8+
client_test
89
client_resolution_test
910
request_options_test
1011
byte_source_test

‎http/test/v2/client/units/client_options_test.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,26 @@ TEST(client_options_test, default_options_use_proxy) {
2121
ASSERT_FALSE(opts.use_proxy());
2222
}
2323

24-
TEST(client_options, default_options_openssl_certificate_paths) {
24+
TEST(client_options_test, default_options_openssl_certificate_paths) {
2525
network::http::v2::client_options opts;
2626
ASSERT_TRUE(opts.openssl_certificate_paths().empty());
2727
}
2828

29-
TEST(client_options, default_options_openssl_verify_paths) {
29+
TEST(client_options_test, default_options_openssl_verify_paths) {
3030
network::http::v2::client_options opts;
3131
ASSERT_TRUE(opts.openssl_verify_paths().empty());
3232
}
3333

34+
TEST(client_options_test, always_verify_peer) {
35+
network::http::v2::client_options opts;
36+
ASSERT_FALSE(opts.always_verify_peer());
37+
}
38+
39+
TEST(client_options_test, user_agent) {
40+
network::http::v2::client_options opts;
41+
ASSERT_EQ("cpp-netlib/1.0.0a", opts.user_agent());
42+
}
43+
3444
TEST(client_options_test, set_option_follow_redirects) {
3545
network::http::v2::client_options opts;
3646
opts.follow_redirects(true);
@@ -60,3 +70,9 @@ TEST(client_options_test, set_option_openssl_verify_path) {
6070
opts.openssl_verify_path("openssl_verify");
6171
ASSERT_EQ(std::vector<std::string>{"openssl_verify"}, opts.openssl_verify_paths());
6272
}
73+
74+
TEST(client_options_test, set_always_verify_peer) {
75+
network::http::v2::client_options opts;
76+
opts.always_verify_peer(true);
77+
ASSERT_TRUE(opts.always_verify_peer());
78+
}

‎http/test/v2/client/units/client_resolution_test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ class mock_async_resolver : public http_cc::async_resolver {
2121
virtual void async_resolve(const std::string &, std::uint16_t,
2222
resolve_callback callback) {
2323
// arbitrary error code, I just want this to fail
24-
boost::system::error_code ec(boost::system::errc::broken_pipe,
25-
boost::system::system_category());
24+
boost::system::error_code ec(boost::asio::error::host_not_found,
25+
boost::asio::error::get_netdb_category());
2626
callback(ec, resolver_iterator());
2727
}
2828

@@ -84,5 +84,5 @@ TEST_F(client_resolution_test, host_not_found) {
8484
.append_header("User-Agent", "cpp-netlib client_test")
8585
.append_header("Connection", "close");
8686
auto future_response = client_->head(request);
87-
ASSERT_THROW(future_response.get(), http::client_exception);
87+
ASSERT_THROW(future_response.get(), std::system_error);
8888
}

0 commit comments

Comments
 (0)