From b7d476dd952e083b2c919958d171c3d514027589 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Fri, 11 Aug 2023 21:09:25 +0800 Subject: [PATCH 01/29] feat: implement http client based on libcurl --- src/http/http_client.h | 34 ++++++++++++++++++++++++++++++ src/http/http_message_parser.cpp | 4 ++-- src/http/http_method.h | 28 ++++++++++++++++++++++++ src/http/http_server.h | 7 +----- src/http/pprof_http_service.cpp | 2 +- src/http/test/http_server_test.cpp | 8 +++---- src/utils/metrics.cpp | 2 +- thirdparty/CMakeLists.txt | 1 - 8 files changed, 71 insertions(+), 15 deletions(-) create mode 100644 src/http/http_client.h create mode 100644 src/http/http_method.h diff --git a/src/http/http_client.h b/src/http/http_client.h new file mode 100644 index 0000000000..5006bd208d --- /dev/null +++ b/src/http/http_client.h @@ -0,0 +1,34 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include + +namespace pegasus { + +class http_client +{ +public: + http_client(); + ~http_client(); + +private: + DISALLOW_COPY_AND_ASSIGN(http_client); +}; + +} // namespace pegasus diff --git a/src/http/http_message_parser.cpp b/src/http/http_message_parser.cpp index ce2e0054ce..5bd3745575 100644 --- a/src/http/http_message_parser.cpp +++ b/src/http/http_message_parser.cpp @@ -132,10 +132,10 @@ http_message_parser::http_message_parser() message_header *header = msg->header; if (parser->type == HTTP_REQUEST && parser->method == HTTP_GET) { - header->hdr_type = http_method::HTTP_METHOD_GET; + header->hdr_type = http_method::GET; header->context.u.is_request = 1; } else if (parser->type == HTTP_REQUEST && parser->method == HTTP_POST) { - header->hdr_type = http_method::HTTP_METHOD_POST; + header->hdr_type = http_method::POST; header->context.u.is_request = 1; } else { // Bit fields don't work with "perfect" forwarding, see diff --git a/src/http/http_method.h b/src/http/http_method.h new file mode 100644 index 0000000000..6e962454ab --- /dev/null +++ b/src/http/http_method.h @@ -0,0 +1,28 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +namespace pegasus { + +enum class http_method +{ + GET = 1, + POST = 2, +}; + +} // namespace pegasus diff --git a/src/http/http_server.h b/src/http/http_server.h index b182947e41..594ec70f80 100644 --- a/src/http/http_server.h +++ b/src/http/http_server.h @@ -25,6 +25,7 @@ #include #include +#include "http_method.h" #include "runtime/task/task_code.h" #include "utils/blob.h" #include "utils/errors.h" @@ -38,12 +39,6 @@ DSN_DECLARE_bool(enable_http_server); /// The rpc code for all the HTTP RPCs. DEFINE_TASK_CODE_RPC(RPC_HTTP_SERVICE, TASK_PRIORITY_COMMON, THREAD_POOL_DEFAULT); -enum http_method -{ - HTTP_METHOD_GET = 1, - HTTP_METHOD_POST = 2, -}; - class message_ex; struct http_request diff --git a/src/http/pprof_http_service.cpp b/src/http/pprof_http_service.cpp index f57c57b193..cf6d519b21 100644 --- a/src/http/pprof_http_service.cpp +++ b/src/http/pprof_http_service.cpp @@ -308,7 +308,7 @@ void pprof_http_service::symbol_handler(const http_request &req, http_response & // Load /proc/self/maps pthread_once(&s_load_symbolmap_once, load_symbols); - if (req.method != http_method::HTTP_METHOD_POST) { + if (req.method != http_method::POST) { char buf[64]; snprintf(buf, sizeof(buf), "num_symbols: %lu\n", symbol_map.size()); resp.body = buf; diff --git a/src/http/test/http_server_test.cpp b/src/http/test/http_server_test.cpp index 9aa9f17c39..8e50b2e0c8 100644 --- a/src/http/test/http_server_test.cpp +++ b/src/http/test/http_server_test.cpp @@ -174,7 +174,7 @@ class http_message_parser_test : public testing::Test message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::HTTP_METHOD_GET); + ASSERT_EQ(msg->header->hdr_type, http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 1); // url @@ -215,7 +215,7 @@ TEST_F(http_message_parser_test, parse_request) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::HTTP_METHOD_POST); + ASSERT_EQ(msg->header->hdr_type, http_method::POST); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), "Message Body sdfsdf"); // body @@ -266,7 +266,7 @@ TEST_F(http_message_parser_test, eof) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::HTTP_METHOD_GET); + ASSERT_EQ(msg->header->hdr_type, http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), ""); // body @@ -297,7 +297,7 @@ TEST_F(http_message_parser_test, parse_long_url) message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::HTTP_METHOD_GET); + ASSERT_EQ(msg->header->hdr_type, http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 4097); // url diff --git a/src/utils/metrics.cpp b/src/utils/metrics.cpp index 40caace7a4..bbbf81c397 100644 --- a/src/utils/metrics.cpp +++ b/src/utils/metrics.cpp @@ -275,7 +275,7 @@ const dsn::metric_filters::metric_fields_type kBriefMetricFields = get_brief_met void metrics_http_service::get_metrics_handler(const http_request &req, http_response &resp) { - if (req.method != http_method::HTTP_METHOD_GET) { + if (req.method != http_method::GET) { resp.body = encode_error_as_json("please use 'GET' method while querying for metrics"); resp.status_code = http_status_code::bad_request; return; diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index 4558aa94f7..f32d3c11ea 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -295,7 +295,6 @@ ExternalProject_Add(curl --disable-smtp --disable-telnet --disable-tftp - --disable-shared --without-librtmp --without-zlib --without-libssh2 From e5c8ed046cbd4cb4e29caca938582a736d4ae6b0 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Sat, 12 Aug 2023 17:18:33 +0800 Subject: [PATCH 02/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 20 ++++++++++++++++++++ src/http/http_client.h | 4 ++++ 2 files changed, 24 insertions(+) create mode 100644 src/http/http_client.cpp diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp new file mode 100644 index 0000000000..fa2560b00b --- /dev/null +++ b/src/http/http_client.cpp @@ -0,0 +1,20 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +namespace pegasus { + +} // namespace pegasus diff --git a/src/http/http_client.h b/src/http/http_client.h index 5006bd208d..5c5a7c732e 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -19,6 +19,8 @@ #include +#include "http/http_method.h" + namespace pegasus { class http_client @@ -28,6 +30,8 @@ class http_client ~http_client(); private: + CURL* _curl = nullptr; + DISALLOW_COPY_AND_ASSIGN(http_client); }; From 839cb294cc7f8cce11ae7584785ec564cb8e7a53 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Mon, 14 Aug 2023 18:17:16 +0800 Subject: [PATCH 03/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 105 +++++++++++++++++++++++++++++++++++++++ src/http/http_client.h | 8 +++ 2 files changed, 113 insertions(+) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index fa2560b00b..d8aad86113 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -15,6 +15,111 @@ // specific language governing permissions and limitations // under the License. +#include "http/http_client.h" + +#include +#include "utils/errors.h" +#include "utils/ports.h" + namespace pegasus { +DSN_DEFINE_uint32(http, + libcurl_error_buffer_bytes, + CURL_ERROR_SIZE, + "The size of a buffer that is used by libcurl to store human readable " + "error messages on failures or problems"); +DSN_DEFINE_validator(libcurl_error_buffer_bytes, [](uint32_t value) -> bool { return value >= CURL_ERROR_SIZE; }); + +static_assert(http_client::kErrorBufferBytes >= CURL_ERROR_SIZE, "The error buffer used by libcurl must be at least CURL_ERROR_SIZE bytes big"); + +http_client::http_client() +{ + +} + +http_client::~http_client() +{ + if (_curl != nullptr) { + curl_easy_cleanup(_curl); + _curl = nullptr; + } +} + +#define RETURN_IF_CURL_NOT_OK(expr, err, ...) \ + do { \ + const auto code = (expr); \ + if (dsn_unlikely(code != CURLE_OK)) { \ + std::string msg(fmt::format("{}: {}", fmt::format(__VA_ARGS__), to_error_msg(code))); \ + // TODO: err + return dsn::error_s::make(err, msg); \ + } \ + } while (0) + +#define RETURN_IF_SETOPT_NOT_OK(opt, param, err) \ + // TODO: err + RETURN_IF_CURL_NOT_OK(curl_easy_setopt(_curl, opt, param), err, "failed to set " #opt " to " #param) + +dsn::error_s http_client::init() +{ + if (_curl == nullptr) { + _curl = curl_easy_init(); + if (_curl == nullptr) { + // TODO: err + return dsn::error_s::make(err, "fail to initialize curl"); + } + } else { + curl_easy_reset(_curl); + } + + // Additional messages for errors are needed. + RETURN_IF_SETOPT_NOT_OK(CURLOPT_ERRORBUFFER, _error_buf); + + // Set with NOSIGNAL since we are multi-threaded. + RETURN_IF_SETOPT_NOT_OK(CURLOPT_NOSIGNAL, 1L); + + // Redirects are supported. + RETURN_IF_SETOPT_NOT_OK(CURLOPT_FOLLOWLOCATION, 1L); + + // Before 8.3.0, it was unlimited. + RETURN_IF_SETOPT_NOT_OK(CURLOPT_MAXREDIRS, 20); + + // A lambda can only be converted to a function pointer if it does not capture. See + // https://stackoverflow.com/questions/28746744/passing-capturing-lambda-as-function-pointer + // http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf + curl_write_callback callback = [](char* buffer, size_t size, size_t nmemb, void* param) { + HttpClient* client = (HttpClient*)param; + return client->on_response_data(buffer, size * nmemb); + }; + + RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEDATA, static_cast(this)); + + return error_s::ok(); +} + +void http_client::clear_error_buf() +{ + _error_buf[0] = 0; +} + +bool http_client::is_error_buf_empty() +{ + return _error_buf[0] == 0; +} + +std::string http_client::to_error_msg(CURLcode code) +{ + std::string err_msg = fmt::format("code={}", curl_easy_strerror(code)); + if (is_error_buf_empty()) { + return err_msg; + } + + err_msg += fmt::format(", msg={}", _error_buf); + return err_msg; +} + + // Error buffer should be cleared + clear_error_buf(); + +#undef RETURN_IF_SETOPT_NOT_OK + } // namespace pegasus diff --git a/src/http/http_client.h b/src/http/http_client.h index 5c5a7c732e..3ed72b8143 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -30,7 +30,15 @@ class http_client ~http_client(); private: + void clear_error_buffer(); + bool is_error_buffer_empty(); + + // The size of a buffer that is used by libcurl to store human readable + // error messages on failures or problems. + static const constexpr size_t kErrorBufferBytes = CURL_ERROR_SIZE; + CURL* _curl = nullptr; + char _error_buf[kErrorBufferBytes]; DISALLOW_COPY_AND_ASSIGN(http_client); }; From 71feffb3151257ddfca33744caff78d40e20e15d Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Mon, 14 Aug 2023 21:52:19 +0800 Subject: [PATCH 04/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 71 +++++++++++++++++++++++++++--- src/http/http_client.h | 13 ++++++ src/http/http_message_parser.cpp | 4 +- src/http/http_method.h | 7 +++ src/http/http_server.cpp | 2 +- src/http/http_server.h | 2 +- src/http/pprof_http_service.cpp | 2 +- src/http/test/http_server_test.cpp | 8 ++-- src/utils/metrics.cpp | 2 +- 9 files changed, 95 insertions(+), 16 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index d8aad86113..a23790dd7a 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -18,7 +18,8 @@ #include "http/http_client.h" #include -#include "utils/errors.h" +#include +#include "utils/fmt_logging.h" #include "utils/ports.h" namespace pegasus { @@ -45,20 +46,23 @@ http_client::~http_client() } } + // TODO: err #define RETURN_IF_CURL_NOT_OK(expr, err, ...) \ do { \ const auto code = (expr); \ if (dsn_unlikely(code != CURLE_OK)) { \ std::string msg(fmt::format("{}: {}", fmt::format(__VA_ARGS__), to_error_msg(code))); \ - // TODO: err return dsn::error_s::make(err, msg); \ } \ } while (0) -#define RETURN_IF_SETOPT_NOT_OK(opt, param, err) \ // TODO: err +#define RETURN_IF_SETOPT_NOT_OK(opt, param, err) \ RETURN_IF_CURL_NOT_OK(curl_easy_setopt(_curl, opt, param), err, "failed to set " #opt " to " #param) +#define RETURN_IF_DO_METHOD_NOT_OK(err) \ + RETURN_IF_CURL_NOT_OK(curl_easy_perform(_curl), err, "failed to perform [method={}, url={}]", enum_to_string(_method), _url, action) + dsn::error_s http_client::init() { if (_curl == nullptr) { @@ -83,15 +87,17 @@ dsn::error_s http_client::init() // Before 8.3.0, it was unlimited. RETURN_IF_SETOPT_NOT_OK(CURLOPT_MAXREDIRS, 20); - // A lambda can only be converted to a function pointer if it does not capture. See + // A lambda can only be converted to a function pointer if it does not capture: // https://stackoverflow.com/questions/28746744/passing-capturing-lambda-as-function-pointer // http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf curl_write_callback callback = [](char* buffer, size_t size, size_t nmemb, void* param) { - HttpClient* client = (HttpClient*)param; + http_client* client = reinterpret_cast(param); return client->on_response_data(buffer, size * nmemb); }; + RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEFUNCTION, callback); - RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEDATA, static_cast(this)); + // This http_client object itself is passed to the callback function. + RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEDATA, reinterpret_cast(this)); return error_s::ok(); } @@ -117,9 +123,62 @@ std::string http_client::to_error_msg(CURLcode code) return err_msg; } +size_t http_client::on_response_data(const void* data, size_t length) { + if (_callback == nullptr) { + return length; + } + + if (!(*_callback)) { + return length; + } + + return (*_callback)(data, length) ? length : std::numeric_limits::max(); +} + // Error buffer should be cleared clear_error_buf(); +dsn::error_s http_client::set_method(http_method method) { + switch (method) { + case http_method::GET: + RETURN_IF_SETOPT_NOT_OK(CURLOPT_HTTPGET, 1L); + break; + case http_method::POST: + RETURN_IF_SETOPT_NOT_OK(CURLOPT_POST, 1L); + break; + default: + LOG_FATAL("Unsupported http_method"); + } + + _method = method; + return dsn::error_s::ok(); +} + +dsn::error_s http_client::set_url(const std::string& url) { + RETURN_IF_SETOPT_NOT_OK(CURLOPT_URL, url.c_str()); + _url = url; +} + +dsn::error_s http_client::do_method(const http_client::http_callback& callback) { + _callback = &callback; + RETURN_IF_DO_METHOD_NOT_OK(); + return dsn::error_s::ok(); +} + +dsn::error_s http_client::do_method(std::string* response) { + if (response == nullptr) { + return do_method(); + } + + auto callback = [response](const void* data, size_t length) { + response->append(reinterpret_castdata, length); + return true; + }; + + return do_method(callback); +} + +#undef RETURN_IF_DO_METHOD_NOT_OK #undef RETURN_IF_SETOPT_NOT_OK } // namespace pegasus diff --git a/src/http/http_client.h b/src/http/http_client.h index 3ed72b8143..fac4465114 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -18,26 +18,39 @@ #pragma once #include +#include +#include #include "http/http_method.h" +#include "utils/errors.h" namespace pegasus { +// Not thread-safe. class http_client { public: + using http_callback = std::function; + http_client(); ~http_client(); + dsn::error_s do_method(const http_callback& callback = {}); + dsn::error_s do_method(std::string* response); + private: void clear_error_buffer(); bool is_error_buffer_empty(); + // The size of a buffer that is used by libcurl to store human readable // error messages on failures or problems. static const constexpr size_t kErrorBufferBytes = CURL_ERROR_SIZE; CURL* _curl = nullptr; + http_method _method; + std::string _url; + const http_callback* _callback = nullptr; char _error_buf[kErrorBufferBytes]; DISALLOW_COPY_AND_ASSIGN(http_client); diff --git a/src/http/http_message_parser.cpp b/src/http/http_message_parser.cpp index 5bd3745575..7642e965f1 100644 --- a/src/http/http_message_parser.cpp +++ b/src/http/http_message_parser.cpp @@ -132,10 +132,10 @@ http_message_parser::http_message_parser() message_header *header = msg->header; if (parser->type == HTTP_REQUEST && parser->method == HTTP_GET) { - header->hdr_type = http_method::GET; + header->hdr_type = pegasus::http_method::GET; header->context.u.is_request = 1; } else if (parser->type == HTTP_REQUEST && parser->method == HTTP_POST) { - header->hdr_type = http_method::POST; + header->hdr_type = pegasus::http_method::POST; header->context.u.is_request = 1; } else { // Bit fields don't work with "perfect" forwarding, see diff --git a/src/http/http_method.h b/src/http/http_method.h index 6e962454ab..8028354818 100644 --- a/src/http/http_method.h +++ b/src/http/http_method.h @@ -17,6 +17,8 @@ #pragma once +#include "utils/enum_helper.h" + namespace pegasus { enum class http_method @@ -25,4 +27,9 @@ enum class http_method POST = 2, }; +ENUM_BEGIN(http_method) +ENUM_REG2(http_method, GET) +ENUM_REG2(http_method, POST) +ENUM_END(http_method) + } // namespace pegasus diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp index cbc179c815..7555327d44 100644 --- a/src/http/http_server.cpp +++ b/src/http/http_server.cpp @@ -168,7 +168,7 @@ void http_server::serve(message_ex *msg) http_request ret; ret.body = m->buffers[1]; ret.full_url = m->buffers[2]; - ret.method = static_cast(m->header->hdr_type); + ret.method = static_cast(m->header->hdr_type); http_parser_url u{0}; http_parser_parse_url(ret.full_url.data(), ret.full_url.length(), false, &u); diff --git a/src/http/http_server.h b/src/http/http_server.h index 594ec70f80..5bb12f0321 100644 --- a/src/http/http_server.h +++ b/src/http/http_server.h @@ -50,7 +50,7 @@ struct http_request std::unordered_map query_args; blob body; blob full_url; - http_method method; + pegasus::http_method method; }; enum class http_status_code diff --git a/src/http/pprof_http_service.cpp b/src/http/pprof_http_service.cpp index cf6d519b21..90d2f5c6ba 100644 --- a/src/http/pprof_http_service.cpp +++ b/src/http/pprof_http_service.cpp @@ -308,7 +308,7 @@ void pprof_http_service::symbol_handler(const http_request &req, http_response & // Load /proc/self/maps pthread_once(&s_load_symbolmap_once, load_symbols); - if (req.method != http_method::POST) { + if (req.method != pegasus::http_method::POST) { char buf[64]; snprintf(buf, sizeof(buf), "num_symbols: %lu\n", symbol_map.size()); resp.body = buf; diff --git a/src/http/test/http_server_test.cpp b/src/http/test/http_server_test.cpp index 8e50b2e0c8..cea9ba1d8d 100644 --- a/src/http/test/http_server_test.cpp +++ b/src/http/test/http_server_test.cpp @@ -174,7 +174,7 @@ class http_message_parser_test : public testing::Test message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::GET); + ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 1); // url @@ -215,7 +215,7 @@ TEST_F(http_message_parser_test, parse_request) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::POST); + ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::POST); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), "Message Body sdfsdf"); // body @@ -266,7 +266,7 @@ TEST_F(http_message_parser_test, eof) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::GET); + ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), ""); // body @@ -297,7 +297,7 @@ TEST_F(http_message_parser_test, parse_long_url) message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::GET); + ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 4097); // url diff --git a/src/utils/metrics.cpp b/src/utils/metrics.cpp index bbbf81c397..c8a7602be6 100644 --- a/src/utils/metrics.cpp +++ b/src/utils/metrics.cpp @@ -275,7 +275,7 @@ const dsn::metric_filters::metric_fields_type kBriefMetricFields = get_brief_met void metrics_http_service::get_metrics_handler(const http_request &req, http_response &resp) { - if (req.method != http_method::GET) { + if (req.method != pegasus::http_method::GET) { resp.body = encode_error_as_json("please use 'GET' method while querying for metrics"); resp.status_code = http_status_code::bad_request; return; From 92fe08ae8d2ddf12bc03bcb0a1106cf071e44563 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Tue, 15 Aug 2023 15:57:47 +0800 Subject: [PATCH 05/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 133 +++++++++++++++++++++++++-------------- src/http/http_client.h | 23 ++++--- src/utils/error_code.h | 2 + src/utils/errors.h | 2 + 4 files changed, 106 insertions(+), 54 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index a23790dd7a..8fd44e0eac 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -25,17 +25,17 @@ namespace pegasus { DSN_DEFINE_uint32(http, - libcurl_error_buffer_bytes, - CURL_ERROR_SIZE, - "The size of a buffer that is used by libcurl to store human readable " - "error messages on failures or problems"); -DSN_DEFINE_validator(libcurl_error_buffer_bytes, [](uint32_t value) -> bool { return value >= CURL_ERROR_SIZE; }); + curl_timeout_ms, + 10000, + "The maximum time in milliseconds that you allow the libcurl transfer operation " + "to complete"); -static_assert(http_client::kErrorBufferBytes >= CURL_ERROR_SIZE, "The error buffer used by libcurl must be at least CURL_ERROR_SIZE bytes big"); +static_assert(http_client::kErrorBufferBytes >= CURL_ERROR_SIZE, + "The error buffer used by libcurl must be at least CURL_ERROR_SIZE bytes big"); -http_client::http_client() +http_client::http_client() : _curl(nullptr), _method(http_method::GET), _callback(nullptr) { - + clear_error_buf(); } http_client::~http_client() @@ -46,22 +46,43 @@ http_client::~http_client() } } - // TODO: err -#define RETURN_IF_CURL_NOT_OK(expr, err, ...) \ - do { \ - const auto code = (expr); \ - if (dsn_unlikely(code != CURLE_OK)) { \ - std::string msg(fmt::format("{}: {}", fmt::format(__VA_ARGS__), to_error_msg(code))); \ - return dsn::error_s::make(err, msg); \ - } \ +namespace { + +inline dsn::error_code to_error_code(CURLcode code) +{ + if (code == CURLE_OK) { + return dsn::ERR_OK; + } + + if (code == CURLE_OPERATION_TIMEDOUT) { + return dsn::ERR_TIMEOUT; + } + return dsn::ERR_CURL_FAILED; +} + +} // anonymous namespace + +#define RETURN_IF_CURL_NOT_OK(expr, ...) \ + do { \ + const auto code = (expr); \ + if (dsn_unlikely(code != CURLE_OK)) { \ + std::string msg(fmt::format("{}: {}", fmt::format(__VA_ARGS__), to_error_msg(code))); \ + return dsn::error_s::make(to_error_code(code), msg); \ + } \ } while (0) - // TODO: err -#define RETURN_IF_SETOPT_NOT_OK(opt, param, err) \ - RETURN_IF_CURL_NOT_OK(curl_easy_setopt(_curl, opt, param), err, "failed to set " #opt " to " #param) +#define RETURN_IF_SETOPT_NOT_OK(opt, input) \ + RETURN_IF_CURL_NOT_OK(curl_easy_setopt(_curl, opt, input), "failed to set " #opt " to" \ + " " #input) + +#define RETURN_IF_GETINFO_NOT_OK(info, output) \ + RETURN_IF_CURL_NOT_OK(curl_easy_getinfo(_curl, info, output), "failed to get from " #info) -#define RETURN_IF_DO_METHOD_NOT_OK(err) \ - RETURN_IF_CURL_NOT_OK(curl_easy_perform(_curl), err, "failed to perform [method={}, url={}]", enum_to_string(_method), _url, action) +#define RETURN_IF_DO_METHOD_NOT_OK() \ + RETURN_IF_CURL_NOT_OK(curl_easy_perform(_curl), \ + "failed to perform [method={}, url={}]", \ + enum_to_string(_method), \ + _url) dsn::error_s http_client::init() { @@ -76,6 +97,7 @@ dsn::error_s http_client::init() } // Additional messages for errors are needed. + clear_error_buf(); RETURN_IF_SETOPT_NOT_OK(CURLOPT_ERRORBUFFER, _error_buf); // Set with NOSIGNAL since we are multi-threaded. @@ -84,46 +106,45 @@ dsn::error_s http_client::init() // Redirects are supported. RETURN_IF_SETOPT_NOT_OK(CURLOPT_FOLLOWLOCATION, 1L); - // Before 8.3.0, it was unlimited. + // Before 8.3.0, CURLOPT_MAXREDIRS was unlimited. RETURN_IF_SETOPT_NOT_OK(CURLOPT_MAXREDIRS, 20); + // Set common timeout for transfer operation. Users could also change it with their + // custom values by `set_timeout`. + RETURN_IF_SETOPT_NOT_OK(CURLOPT_TIMEOUT_MS, static_cast(FLAGS_curl_timeout_ms)); + // A lambda can only be converted to a function pointer if it does not capture: // https://stackoverflow.com/questions/28746744/passing-capturing-lambda-as-function-pointer // http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf - curl_write_callback callback = [](char* buffer, size_t size, size_t nmemb, void* param) { - http_client* client = reinterpret_cast(param); + curl_write_callback callback = [](char *buffer, size_t size, size_t nmemb, void *param) { + http_client *client = reinterpret_cast(param); return client->on_response_data(buffer, size * nmemb); }; RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEFUNCTION, callback); // This http_client object itself is passed to the callback function. - RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEDATA, reinterpret_cast(this)); + RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEDATA, reinterpret_cast(this)); return error_s::ok(); } -void http_client::clear_error_buf() -{ - _error_buf[0] = 0; -} +void http_client::clear_error_buf() { _error_buf[0] = 0; } -bool http_client::is_error_buf_empty() -{ - return _error_buf[0] == 0; -} +bool http_client::is_error_buf_empty() const { return _error_buf[0] == 0; } -std::string http_client::to_error_msg(CURLcode code) +std::string http_client::to_error_msg(CURLcode code) const { - std::string err_msg = fmt::format("code={}", curl_easy_strerror(code)); - if (is_error_buf_empty()) { + std::string err_msg = fmt::format("code={}", curl_easy_strerror(code)); + if (is_error_buf_empty()) { return err_msg; - } + } err_msg += fmt::format(", msg={}", _error_buf); return err_msg; } -size_t http_client::on_response_data(const void* data, size_t length) { +size_t http_client::on_response_data(const void *data, size_t length) +{ if (_callback == nullptr) { return length; } @@ -135,10 +156,8 @@ size_t http_client::on_response_data(const void* data, size_t length) { return (*_callback)(data, length) ? length : std::numeric_limits::max(); } - // Error buffer should be cleared - clear_error_buf(); - -dsn::error_s http_client::set_method(http_method method) { +dsn::error_s http_client::set_method(http_method method) +{ switch (method) { case http_method::GET: RETURN_IF_SETOPT_NOT_OK(CURLOPT_HTTPGET, 1L); @@ -154,31 +173,51 @@ dsn::error_s http_client::set_method(http_method method) { return dsn::error_s::ok(); } -dsn::error_s http_client::set_url(const std::string& url) { +dsn::error_s http_client::set_url(const std::string &url) +{ RETURN_IF_SETOPT_NOT_OK(CURLOPT_URL, url.c_str()); + _url = url; + return dsn::error_s::ok(); +} + +dsn::error_s http_client::set_timeout(long timeout_ms) +{ + RETURN_IF_SETOPT_NOT_OK(CURLOPT_TIMEOUT_MS, timeout_ms); + return dsn::error_s::ok(); } -dsn::error_s http_client::do_method(const http_client::http_callback& callback) { +dsn::error_s http_client::do_method(const http_client::http_callback &callback) +{ + // `curl_easy_perform` would run synchronously, thus it is safe to use the pointer to + // `callback`. _callback = &callback; RETURN_IF_DO_METHOD_NOT_OK(); return dsn::error_s::ok(); } -dsn::error_s http_client::do_method(std::string* response) { +dsn::error_s http_client::do_method(std::string *response) +{ if (response == nullptr) { return do_method(); } - auto callback = [response](const void* data, size_t length) { - response->append(reinterpret_castdata, length); + auto callback = [response](const void *data, size_t length) { + response->append(reinterpret_cast data, length); return true; }; return do_method(callback); } +dsn::error_s http_client::get_http_status(long &http_status) const +{ + RETURN_IF_GETINFO_NOT_OK(CURLINFO_RESPONSE_CODE, &http_status); + return dsn::error_s::ok(); +} + #undef RETURN_IF_DO_METHOD_NOT_OK #undef RETURN_IF_SETOPT_NOT_OK +#undef RETURN_IF_CURL_NOT_OK } // namespace pegasus diff --git a/src/http/http_client.h b/src/http/http_client.h index fac4465114..3f33820956 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -30,27 +30,36 @@ namespace pegasus { class http_client { public: - using http_callback = std::function; + using http_callback = std::function; http_client(); ~http_client(); - dsn::error_s do_method(const http_callback& callback = {}); - dsn::error_s do_method(std::string* response); + dsn::error_s init(); + + dsn::error_s set_method(http_method method); + dsn::error_s set_url(const std::string &url); + dsn::error_s set_timeout(long timeout_ms); + + dsn::error_s do_method(long &http_status, const http_callback &callback = {}); + dsn::error_s do_method(long &http_status, std::string *response); + + dsn::error_s get_http_status(long &http_status) const; private: void clear_error_buffer(); - bool is_error_buffer_empty(); - + bool is_error_buffer_empty() const; + std::string to_error_msg(CURLcode code) const; + size_t on_response_data(const void *data, size_t length); // The size of a buffer that is used by libcurl to store human readable // error messages on failures or problems. static const constexpr size_t kErrorBufferBytes = CURL_ERROR_SIZE; - CURL* _curl = nullptr; + CURL *_curl; http_method _method; std::string _url; - const http_callback* _callback = nullptr; + const http_callback *_callback; char _error_buf[kErrorBufferBytes]; DISALLOW_COPY_AND_ASSIGN(http_client); diff --git a/src/utils/error_code.h b/src/utils/error_code.h index 45a6e793cc..04df97947a 100644 --- a/src/utils/error_code.h +++ b/src/utils/error_code.h @@ -180,6 +180,8 @@ DEFINE_ERR_CODE(ERR_RANGER_POLICIES_NO_NEED_UPDATE) DEFINE_ERR_CODE(ERR_RDB_CORRUPTION) DEFINE_ERR_CODE(ERR_DISK_IO_ERROR) + +DEFINE_ERR_CODE(ERR_CURL_FAILED) } // namespace dsn USER_DEFINED_STRUCTURE_FORMATTER(::dsn::error_code); diff --git a/src/utils/errors.h b/src/utils/errors.h index 9cbf70e092..b5dec795d7 100644 --- a/src/utils/errors.h +++ b/src/utils/errors.h @@ -97,6 +97,8 @@ class error_s return true; } + explicit operator bool() const noexcept { return is_ok(); } + std::string description() const { if (!_info) { From 1d4cd0e03961af5e9a2cb70bce8b2361830916a0 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Tue, 15 Aug 2023 18:29:40 +0800 Subject: [PATCH 06/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 104 +++++++++++++++++++++++++---- src/http/http_client.h | 29 ++++++-- src/http/http_message_parser.cpp | 4 +- src/http/http_method.h | 7 +- src/http/http_server.cpp | 2 +- src/http/http_server.h | 2 +- src/http/pprof_http_service.cpp | 2 +- src/http/test/http_server_test.cpp | 8 +-- src/utils/metrics.cpp | 2 +- 9 files changed, 129 insertions(+), 31 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 8fd44e0eac..480a598eec 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -19,10 +19,10 @@ #include #include +#include "utils/flags.h" #include "utils/fmt_logging.h" -#include "utils/ports.h" -namespace pegasus { +namespace dsn { DSN_DEFINE_uint32(http, curl_timeout_ms, @@ -30,11 +30,17 @@ DSN_DEFINE_uint32(http, "The maximum time in milliseconds that you allow the libcurl transfer operation " "to complete"); -static_assert(http_client::kErrorBufferBytes >= CURL_ERROR_SIZE, - "The error buffer used by libcurl must be at least CURL_ERROR_SIZE bytes big"); - -http_client::http_client() : _curl(nullptr), _method(http_method::GET), _callback(nullptr) +http_client::http_client() + : _curl(nullptr), + _method(http_method::GET), + _callback(nullptr), + _header_changed(true), + _header_list(nullptr) { + // Since `kErrorBufferBytes` is private, `static_assert` have to be put in constructor. + static_assert(http_client::kErrorBufferBytes >= CURL_ERROR_SIZE, + "The error buffer used by libcurl must be at least CURL_ERROR_SIZE bytes big"); + clear_error_buf(); } @@ -44,6 +50,8 @@ http_client::~http_client() curl_easy_cleanup(_curl); _curl = nullptr; } + + free_header_list(); } namespace { @@ -72,8 +80,9 @@ inline dsn::error_code to_error_code(CURLcode code) } while (0) #define RETURN_IF_SETOPT_NOT_OK(opt, input) \ - RETURN_IF_CURL_NOT_OK(curl_easy_setopt(_curl, opt, input), "failed to set " #opt " to" \ - " " #input) + RETURN_IF_CURL_NOT_OK(curl_easy_setopt(_curl, opt, input), \ + "failed to set " #opt " to" \ + " " #input) #define RETURN_IF_GETINFO_NOT_OK(info, output) \ RETURN_IF_CURL_NOT_OK(curl_easy_getinfo(_curl, info, output), "failed to get from " #info) @@ -89,13 +98,15 @@ dsn::error_s http_client::init() if (_curl == nullptr) { _curl = curl_easy_init(); if (_curl == nullptr) { - // TODO: err - return dsn::error_s::make(err, "fail to initialize curl"); + return dsn::error_s::make(dsn::ERR_CURL_FAILED, "fail to initialize curl"); } } else { curl_easy_reset(_curl); } + clear_header_fields(); + free_header_list(); + // Additional messages for errors are needed. clear_error_buf(); RETURN_IF_SETOPT_NOT_OK(CURLOPT_ERRORBUFFER, _error_buf); @@ -187,11 +198,80 @@ dsn::error_s http_client::set_timeout(long timeout_ms) return dsn::error_s::ok(); } +void http_client::clear_header_fields() +{ + _header_fields.clear(); + + _header_changed = true; +} + +void http_client::free_header_list() +{ + if (_header_list == nullptr) { + return; + } + + curl_slist_free_all(_header_list); + _header_list = nullptr; +} + +void http_client::set_header_field(const dsn::string_view &key, const dsn::string_view &val) +{ + _header_fields[key] = val; + _header_changed = true; +} + +void http_client::set_accept(const dsn::string_view &val) { set_header_field("Accept", val); } + +void http_client::set_content_type(const dsn::string_view &val) +{ + set_header_field("Content-Type", val); +} + +dsn::error_s http_client::process_header() +{ + if (!_header_changed) { + return dsn::error_s::ok(); + } + + free_header_list(); + + for (const auto &field : _header_fields) { + auto str = fmt::format("{}: {}", field.first, field.second); + + // A null pointer is returned if anything went wrong, otherwise the new list pointer is + // returned. To avoid overwriting an existing non-empty list on failure, the new list + // should be returned to a temporary variable which can be tested for NULL before updating + // the original list pointer. (https://curl.se/libcurl/c/curl_slist_append.html) + struct curl_slist *temp = curl_slist_append(_header_list, str.c_str()); + if (temp == nullptr) { + free_header_list(); + return dsn::error_s::make(dsn::ERR_CURL_FAILED, "curl_slist_append failed"); + } + _header_list = temp; + } + + // This would work well even if `_header_list` is NULL pointer. Pass a NULL to this option + // to reset back to no custom headers. (https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html) + RETURN_IF_SETOPT_NOT_OK(CURLOPT_HTTPHEADER, _header_list); + + // New header has been built successfully, thus mark it unchanged. + _header_changed = false; + + return dsn::error_s::ok(); +} + dsn::error_s http_client::do_method(const http_client::http_callback &callback) { // `curl_easy_perform` would run synchronously, thus it is safe to use the pointer to // `callback`. _callback = &callback; + + const auto &err = process_header(); + if (!err) { + return err; + } + RETURN_IF_DO_METHOD_NOT_OK(); return dsn::error_s::ok(); } @@ -203,7 +283,7 @@ dsn::error_s http_client::do_method(std::string *response) } auto callback = [response](const void *data, size_t length) { - response->append(reinterpret_cast data, length); + response->append(reinterpret_cast(data), length); return true; }; @@ -220,4 +300,4 @@ dsn::error_s http_client::get_http_status(long &http_status) const #undef RETURN_IF_SETOPT_NOT_OK #undef RETURN_IF_CURL_NOT_OK -} // namespace pegasus +} // namespace dsn diff --git a/src/http/http_client.h b/src/http/http_client.h index 3f33820956..6fee2640aa 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -20,11 +20,14 @@ #include #include #include +#include #include "http/http_method.h" #include "utils/errors.h" +#include "utils/ports.h" +#include "utils/string_view.h" -namespace pegasus { +namespace dsn { // Not thread-safe. class http_client @@ -41,17 +44,27 @@ class http_client dsn::error_s set_url(const std::string &url); dsn::error_s set_timeout(long timeout_ms); - dsn::error_s do_method(long &http_status, const http_callback &callback = {}); - dsn::error_s do_method(long &http_status, std::string *response); + void clear_header_fields(); + void set_accept(const dsn::string_view &val); + void set_content_type(const dsn::string_view &val); + + dsn::error_s do_method(const http_callback &callback = {}); + dsn::error_s do_method(std::string *response); dsn::error_s get_http_status(long &http_status) const; private: - void clear_error_buffer(); - bool is_error_buffer_empty() const; + using header_field_map = std::unordered_map; + + void clear_error_buf(); + bool is_error_buf_empty() const; std::string to_error_msg(CURLcode code) const; size_t on_response_data(const void *data, size_t length); + void free_header_list(); + void set_header_field(const dsn::string_view &key, const dsn::string_view &val); + dsn::error_s process_header(); + // The size of a buffer that is used by libcurl to store human readable // error messages on failures or problems. static const constexpr size_t kErrorBufferBytes = CURL_ERROR_SIZE; @@ -62,7 +75,11 @@ class http_client const http_callback *_callback; char _error_buf[kErrorBufferBytes]; + bool _header_changed; + header_field_map _header_fields; + struct curl_slist *_header_list; + DISALLOW_COPY_AND_ASSIGN(http_client); }; -} // namespace pegasus +} // namespace dsn diff --git a/src/http/http_message_parser.cpp b/src/http/http_message_parser.cpp index 7642e965f1..5bd3745575 100644 --- a/src/http/http_message_parser.cpp +++ b/src/http/http_message_parser.cpp @@ -132,10 +132,10 @@ http_message_parser::http_message_parser() message_header *header = msg->header; if (parser->type == HTTP_REQUEST && parser->method == HTTP_GET) { - header->hdr_type = pegasus::http_method::GET; + header->hdr_type = http_method::GET; header->context.u.is_request = 1; } else if (parser->type == HTTP_REQUEST && parser->method == HTTP_POST) { - header->hdr_type = pegasus::http_method::POST; + header->hdr_type = http_method::POST; header->context.u.is_request = 1; } else { // Bit fields don't work with "perfect" forwarding, see diff --git a/src/http/http_method.h b/src/http/http_method.h index 8028354818..f337d24e78 100644 --- a/src/http/http_method.h +++ b/src/http/http_method.h @@ -19,17 +19,18 @@ #include "utils/enum_helper.h" -namespace pegasus { +namespace dsn { enum class http_method { GET = 1, POST = 2, + INVALID = 100, }; -ENUM_BEGIN(http_method) +ENUM_BEGIN(http_method, http_method::INVALID) ENUM_REG2(http_method, GET) ENUM_REG2(http_method, POST) ENUM_END(http_method) -} // namespace pegasus +} // namespace dsn diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp index 7555327d44..cbc179c815 100644 --- a/src/http/http_server.cpp +++ b/src/http/http_server.cpp @@ -168,7 +168,7 @@ void http_server::serve(message_ex *msg) http_request ret; ret.body = m->buffers[1]; ret.full_url = m->buffers[2]; - ret.method = static_cast(m->header->hdr_type); + ret.method = static_cast(m->header->hdr_type); http_parser_url u{0}; http_parser_parse_url(ret.full_url.data(), ret.full_url.length(), false, &u); diff --git a/src/http/http_server.h b/src/http/http_server.h index 5bb12f0321..594ec70f80 100644 --- a/src/http/http_server.h +++ b/src/http/http_server.h @@ -50,7 +50,7 @@ struct http_request std::unordered_map query_args; blob body; blob full_url; - pegasus::http_method method; + http_method method; }; enum class http_status_code diff --git a/src/http/pprof_http_service.cpp b/src/http/pprof_http_service.cpp index 90d2f5c6ba..cf6d519b21 100644 --- a/src/http/pprof_http_service.cpp +++ b/src/http/pprof_http_service.cpp @@ -308,7 +308,7 @@ void pprof_http_service::symbol_handler(const http_request &req, http_response & // Load /proc/self/maps pthread_once(&s_load_symbolmap_once, load_symbols); - if (req.method != pegasus::http_method::POST) { + if (req.method != http_method::POST) { char buf[64]; snprintf(buf, sizeof(buf), "num_symbols: %lu\n", symbol_map.size()); resp.body = buf; diff --git a/src/http/test/http_server_test.cpp b/src/http/test/http_server_test.cpp index cea9ba1d8d..8e50b2e0c8 100644 --- a/src/http/test/http_server_test.cpp +++ b/src/http/test/http_server_test.cpp @@ -174,7 +174,7 @@ class http_message_parser_test : public testing::Test message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::GET); + ASSERT_EQ(msg->header->hdr_type, http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 1); // url @@ -215,7 +215,7 @@ TEST_F(http_message_parser_test, parse_request) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::POST); + ASSERT_EQ(msg->header->hdr_type, http_method::POST); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), "Message Body sdfsdf"); // body @@ -266,7 +266,7 @@ TEST_F(http_message_parser_test, eof) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::GET); + ASSERT_EQ(msg->header->hdr_type, http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), ""); // body @@ -297,7 +297,7 @@ TEST_F(http_message_parser_test, parse_long_url) message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, pegasus::http_method::GET); + ASSERT_EQ(msg->header->hdr_type, http_method::GET); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 4097); // url diff --git a/src/utils/metrics.cpp b/src/utils/metrics.cpp index c8a7602be6..bbbf81c397 100644 --- a/src/utils/metrics.cpp +++ b/src/utils/metrics.cpp @@ -275,7 +275,7 @@ const dsn::metric_filters::metric_fields_type kBriefMetricFields = get_brief_met void metrics_http_service::get_metrics_handler(const http_request &req, http_response &resp) { - if (req.method != pegasus::http_method::GET) { + if (req.method != http_method::GET) { resp.body = encode_error_as_json("please use 'GET' method while querying for metrics"); resp.status_code = http_status_code::bad_request; return; From 163dcaad5e5397ec18b595b201ec1f049c2400c3 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Tue, 15 Aug 2023 20:04:00 +0800 Subject: [PATCH 07/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 13 +++++-------- src/http/http_client.h | 6 +++--- src/http/http_message_parser.cpp | 4 ++-- src/http/test/http_server_test.cpp | 8 ++++---- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 480a598eec..fd224c6de7 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -215,18 +215,15 @@ void http_client::free_header_list() _header_list = nullptr; } -void http_client::set_header_field(const dsn::string_view &key, const dsn::string_view &val) +void http_client::set_header_field(dsn::string_view key, dsn::string_view val) { - _header_fields[key] = val; + _header_fields[std::string(key)] = std::string(val); _header_changed = true; } -void http_client::set_accept(const dsn::string_view &val) { set_header_field("Accept", val); } +void http_client::set_accept(dsn::string_view val) { set_header_field("Accept", val); } -void http_client::set_content_type(const dsn::string_view &val) -{ - set_header_field("Content-Type", val); -} +void http_client::set_content_type(dsn::string_view val) { set_header_field("Content-Type", val); } dsn::error_s http_client::process_header() { @@ -283,7 +280,7 @@ dsn::error_s http_client::do_method(std::string *response) } auto callback = [response](const void *data, size_t length) { - response->append(reinterpret_cast(data), length); + response->append(reinterpret_cast(data), length); return true; }; diff --git a/src/http/http_client.h b/src/http/http_client.h index 6fee2640aa..51af959dc0 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -45,8 +45,8 @@ class http_client dsn::error_s set_timeout(long timeout_ms); void clear_header_fields(); - void set_accept(const dsn::string_view &val); - void set_content_type(const dsn::string_view &val); + void set_accept(dsn::string_view val); + void set_content_type(dsn::string_view val); dsn::error_s do_method(const http_callback &callback = {}); dsn::error_s do_method(std::string *response); @@ -62,7 +62,7 @@ class http_client size_t on_response_data(const void *data, size_t length); void free_header_list(); - void set_header_field(const dsn::string_view &key, const dsn::string_view &val); + void set_header_field(dsn::string_view key, dsn::string_view val); dsn::error_s process_header(); // The size of a buffer that is used by libcurl to store human readable diff --git a/src/http/http_message_parser.cpp b/src/http/http_message_parser.cpp index 5bd3745575..6a7a238cba 100644 --- a/src/http/http_message_parser.cpp +++ b/src/http/http_message_parser.cpp @@ -132,10 +132,10 @@ http_message_parser::http_message_parser() message_header *header = msg->header; if (parser->type == HTTP_REQUEST && parser->method == HTTP_GET) { - header->hdr_type = http_method::GET; + header->hdr_type = static_cast(http_method::GET); header->context.u.is_request = 1; } else if (parser->type == HTTP_REQUEST && parser->method == HTTP_POST) { - header->hdr_type = http_method::POST; + header->hdr_type = static_cast(http_method::POST); header->context.u.is_request = 1; } else { // Bit fields don't work with "perfect" forwarding, see diff --git a/src/http/test/http_server_test.cpp b/src/http/test/http_server_test.cpp index 8e50b2e0c8..eb72245bef 100644 --- a/src/http/test/http_server_test.cpp +++ b/src/http/test/http_server_test.cpp @@ -174,7 +174,7 @@ class http_message_parser_test : public testing::Test message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::GET); + ASSERT_EQ(msg->header->hdr_type, static_cast(http_method::GET)); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 1); // url @@ -215,7 +215,7 @@ TEST_F(http_message_parser_test, parse_request) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::POST); + ASSERT_EQ(msg->header->hdr_type, static_cast(http_method::POST)); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), "Message Body sdfsdf"); // body @@ -266,7 +266,7 @@ TEST_F(http_message_parser_test, eof) ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::GET); + ASSERT_EQ(msg->header->hdr_type, static_cast(http_method::GET)); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[1].to_string(), ""); // body @@ -297,7 +297,7 @@ TEST_F(http_message_parser_test, parse_long_url) message_ptr msg = parser.get_message_on_receive(&reader, read_next); ASSERT_NE(msg, nullptr); ASSERT_EQ(msg->hdr_format, NET_HDR_HTTP); - ASSERT_EQ(msg->header->hdr_type, http_method::GET); + ASSERT_EQ(msg->header->hdr_type, static_cast(http_method::GET)); ASSERT_EQ(msg->header->context.u.is_request, 1); ASSERT_EQ(msg->buffers.size(), HTTP_MSG_BUFFERS_NUM); ASSERT_EQ(msg->buffers[2].size(), 4097); // url From 3537ac00c637c2a1a387006059611a54b61bdbb4 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Tue, 15 Aug 2023 20:12:20 +0800 Subject: [PATCH 08/29] feat: implement http client based on libcurl --- src/utils/test/metrics_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/test/metrics_test.cpp b/src/utils/test/metrics_test.cpp index b2118f03ec..8d65c65dae 100644 --- a/src/utils/test/metrics_test.cpp +++ b/src/utils/test/metrics_test.cpp @@ -2513,7 +2513,7 @@ void test_http_get_metrics(const std::string &request_string, ASSERT_TRUE(req_res.is_ok()); const auto &req = req_res.get_value(); - std::cout << "method: " << req.method << std::endl; + std::cout << "method: " << enum_to_string(req.method) << std::endl; http_response resp; test_get_metrics_handler(req, resp); From 0c1814f57465a1b1f30428cc3c2fd800fff9b37b Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Wed, 16 Aug 2023 19:49:48 +0800 Subject: [PATCH 09/29] feat: implement http client based on libcurl --- src/http/test/CMakeLists.txt | 6 +- src/http/test/config-test.ini | 76 +++++++++++++++++++++++ src/http/test/http_client_test.cpp | 52 ++++++++++++++++ src/http/test/main.cpp | 97 ++++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 src/http/test/config-test.ini create mode 100644 src/http/test/http_client_test.cpp create mode 100644 src/http/test/main.cpp diff --git a/src/http/test/CMakeLists.txt b/src/http/test/CMakeLists.txt index d4da7e5b3a..85f2c3798a 100644 --- a/src/http/test/CMakeLists.txt +++ b/src/http/test/CMakeLists.txt @@ -24,14 +24,16 @@ set(MY_SRC_SEARCH_MODE "GLOB") set(MY_PROJ_LIBS dsn_http dsn_runtime + curl gtest - gtest_main - rocksdb) + rocksdb + ) set(MY_BOOST_LIBS Boost::system Boost::filesystem Boost::regex) set(MY_BINPLACES "${CMAKE_CURRENT_SOURCE_DIR}/run.sh" + "${CMAKE_CURRENT_SOURCE_DIR}/config-test.ini" ) dsn_add_test() diff --git a/src/http/test/config-test.ini b/src/http/test/config-test.ini new file mode 100644 index 0000000000..bc6bee57e7 --- /dev/null +++ b/src/http/test/config-test.ini @@ -0,0 +1,76 @@ +; Licensed to the Apache Software Foundation (ASF) under one +; or more contributor license agreements. See the NOTICE file +; distributed with this work for additional information +; regarding copyright ownership. The ASF licenses this file +; to you under the Apache License, Version 2.0 (the +; "License"); you may not use this file except in compliance +; with the License. You may obtain a copy of the License at +; +; http://www.apache.org/licenses/LICENSE-2.0 +; +; Unless required by applicable law or agreed to in writing, +; software distributed under the License is distributed on an +; "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +; KIND, either express or implied. See the License for the +; specific language governing permissions and limitations +; under the License. + +[apps..default] +run = true +count = 1 +network.client.RPC_CHANNEL_TCP = dsn::tools::asio_network_provider, 65536 +network.client.RPC_CHANNEL_UDP = dsn::tools::asio_udp_provider, 65536 +network.server.0.RPC_CHANNEL_TCP = dsn::tools::asio_network_provider, 65536 +network.server.0.RPC_CHANNEL_UDP = dsn::tools::asio_udp_provider, 65536 + +[apps.test] +type = test +arguments = +run = true +ports = 20001 +count = 1 +delay_seconds = 1 +pools = THREAD_POOL_DEFAULT + +[core] +tool = nativerun + +toollets = tracer, profiler +pause_on_start = false + +logging_start_level = LOG_LEVEL_DEBUG +logging_factory_name = dsn::tools::simple_logger + +[tools.simple_logger] +fast_flush = true +short_header = false +stderr_start_level = LOG_LEVEL_ERROR + +[network] +; how many network threads for network library (used by asio) +io_service_worker_count = 2 + +[task..default] +is_trace = true +is_profile = true +allow_inline = false +rpc_call_channel = RPC_CHANNEL_TCP +rpc_message_header_format = dsn +rpc_timeout_milliseconds = 1000 + +[task.LPC_RPC_TIMEOUT] +is_trace = false +is_profile = false + +[task.RPC_TEST_UDP] +rpc_call_channel = RPC_CHANNEL_UDP +rpc_message_crc_required = true + +; specification for each thread pool +[threadpool..default] +worker_count = 2 + +[threadpool.THREAD_POOL_DEFAULT] +partitioned = false +worker_priority = THREAD_xPRIORITY_NORMAL + diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp new file mode 100644 index 0000000000..08b62b2005 --- /dev/null +++ b/src/http/test/http_client_test.cpp @@ -0,0 +1,52 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include + +#include "http/http_client.h" + +namespace dsn { + +void test_http_client(http_client &client, + const http_method method, + const long expected_http_status, + const std::string &expected_response) +{ + client.set_method(method); + + std::string actual_response; + client.do_method(&actual_response); + + long actual_http_status; + ASSERT_TRUE(client.get_http_status(actual_http_status).is_ok()); + EXPECT_EQ(expected_http_status, actual_http_status); + EXPECT_EQ(expected_response, actual_response); +} + +TEST(http_client_test, get) +{ + http_client client; + ASSERT_TRUE(client.init().is_ok()); + + client.set_url("http://127.0.0.1:20001/test/get"); + + test_http_client(client, http_method::POST, 400, "please use GET method"); + test_http_client(client, http_method::GET, 200, "you are using GET method"); +} + +} // namespace dsn diff --git a/src/http/test/main.cpp b/src/http/test/main.cpp new file mode 100644 index 0000000000..ade2493dab --- /dev/null +++ b/src/http/test/main.cpp @@ -0,0 +1,97 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include +#include +#include +#include + +#include "http/http_server.h" +#include "runtime/service_app.h" +#include "utils/ports.h" + +int gtest_flags = 0; +int gtest_ret = 0; + +class test_http_service : public dsn::http_server_base +{ +public: + test_http_service() + { + register_handler("get", + std::bind(&test_http_service::get_handler, + this, + std::placeholders::_1, + std::placeholders::_2), + "ip:port/test/get"); + } + + ~test_http_service() = default; + + std::string path() const override { return "test"; } + +private: + void get_handler(const dsn::http_request &req, dsn::http_response &resp) + { + if (req.method != dsn::http_method::GET) { + resp.body = "please use GET method"; + resp.status_code = dsn::http_status_code::bad_request; + return; + } + + resp.body = "you are using GET method"; + resp.status_code = dsn::http_status_code::ok; + } + + DISALLOW_COPY_AND_ASSIGN(test_http_service); +}; + +class test_service_app : public dsn::service_app +{ +public: + test_service_app(const dsn::service_app_info *info) : dsn::service_app(info) + { + dsn::register_http_service(new test_http_service); + dsn::start_http_server(); + } + + dsn::error_code start(const std::vector &args) + { + gtest_ret = RUN_ALL_TESTS(); + gtest_flags = 1; + return dsn::ERR_OK; + } +}; + +GTEST_API_ int main(int argc, char **argv) +{ + testing::InitGoogleTest(&argc, argv); + + // register all possible services + dsn::service_app::register_factory("test"); + + dsn_run_config("config-test.ini", false); + while (gtest_flags == 0) { + std::this_thread::sleep_for(std::chrono::seconds(1)); + } + +#ifndef ENABLE_GCOV + dsn_exit(gtest_ret); +#endif + return gtest_ret; +} From 6a4c3bbbfc717df88cea7262d4c4c507bed0547e Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Wed, 16 Aug 2023 20:37:02 +0800 Subject: [PATCH 10/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 4 ++-- src/http/test/http_client_test.cpp | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index fd224c6de7..89f2a00cdc 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -145,12 +145,12 @@ bool http_client::is_error_buf_empty() const { return _error_buf[0] == 0; } std::string http_client::to_error_msg(CURLcode code) const { - std::string err_msg = fmt::format("code={}", curl_easy_strerror(code)); + std::string err_msg = fmt::format("desc=\"{}\"", curl_easy_strerror(code)); if (is_error_buf_empty()) { return err_msg; } - err_msg += fmt::format(", msg={}", _error_buf); + err_msg += fmt::format(", msg=\"{}\"", _error_buf); return err_msg; } diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 08b62b2005..49a5de3815 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -22,6 +22,21 @@ namespace dsn { +TEST(http_client_test, connect) +{ + http_client client; + ASSERT_TRUE(client.init().is_ok()); + + // No one has listened on port 20000, thus this would lead to "Connection refused". + client.set_url("http://127.0.0.1:20000/test/get"); + + const auto &err = client.do_method(); + ASSERT_EQ(dsn::ERR_CURL_FAILED, err.code()); + + // Would print something like "Failed to connect to 127.0.0.1 port 20000: Connection refused". + std::cout << "failed to connect: " << err.description() << std::endl; +} + void test_http_client(http_client &client, const http_method method, const long expected_http_status, @@ -30,7 +45,7 @@ void test_http_client(http_client &client, client.set_method(method); std::string actual_response; - client.do_method(&actual_response); + ASSERT_TRUE(client.do_method(&actual_response)); long actual_http_status; ASSERT_TRUE(client.get_http_status(actual_http_status).is_ok()); From debf9dab9a443e2e9e96b128e0dc2c8fafd944ec Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Wed, 16 Aug 2023 20:43:10 +0800 Subject: [PATCH 11/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 89f2a00cdc..a5e540a920 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -160,6 +160,7 @@ size_t http_client::on_response_data(const void *data, size_t length) return length; } + // callback function is empty. if (!(*_callback)) { return length; } From 23a4558aa19ce76362692a60d1b15d02954acdfb Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Thu, 17 Aug 2023 11:06:40 +0800 Subject: [PATCH 12/29] feat: implement http client based on libcurl --- src/http/CMakeLists.txt | 2 +- src/http/http_client.cpp | 4 ++++ src/http/http_client.h | 13 ++++++++++++- src/http/http_message_parser.cpp | 3 ++- src/http/http_server.cpp | 1 + src/http/pprof_http_service.cpp | 1 + src/http/test/http_client_test.cpp | 6 ++++++ src/http/test/http_server_test.cpp | 2 ++ src/http/test/main.cpp | 6 +++++- src/utils/metrics.cpp | 1 + 10 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/http/CMakeLists.txt b/src/http/CMakeLists.txt index 44f93796d8..9978323f34 100644 --- a/src/http/CMakeLists.txt +++ b/src/http/CMakeLists.txt @@ -21,7 +21,7 @@ set(MY_PROJ_SRC ${THIRDPARTY_ROOT}/build/Source/http-parser/http_parser.c) set(MY_SRC_SEARCH_MODE "GLOB") -set(MY_PROJ_LIBS "") +set(MY_PROJ_LIBS curl) dsn_add_static_library() diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index a5e540a920..00f9b2fa42 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -19,6 +19,10 @@ #include #include +#include + +#include "curl/curl.h" +#include "utils/error_code.h" #include "utils/flags.h" #include "utils/fmt_logging.h" diff --git a/src/http/http_client.h b/src/http/http_client.h index 51af959dc0..821d56a9f5 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include #include #include @@ -29,7 +30,17 @@ namespace dsn { -// Not thread-safe. +// A library for http client that provides convenient APIs to access http services, implemented +// based on libcurl (https://curl.se/libcurl/c/). +// +// This class is not thread-safe. Thus maintain one instance for each thread. +// +// Example of submitting GET request to remote http service +// -------------------------------------------------------- +// Create an instance of http_client: +// http_client client; +// +// const auto &err = client.init(); class http_client { public: diff --git a/src/http/http_message_parser.cpp b/src/http/http_message_parser.cpp index 6a7a238cba..df873eea2e 100644 --- a/src/http/http_message_parser.cpp +++ b/src/http/http_message_parser.cpp @@ -26,12 +26,13 @@ #include "http_message_parser.h" +#include // IWYU pragma: no_include #include #include #include -#include "http_server.h" +#include "http/http_method.h" #include "nodejs/http_parser.h" #include "runtime/rpc/rpc_message.h" #include "utils/blob.h" diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp index cbc179c815..088deb3be4 100644 --- a/src/http/http_server.cpp +++ b/src/http/http_server.cpp @@ -25,6 +25,7 @@ #include "builtin_http_calls.h" #include "fmt/core.h" +#include "http/http_method.h" #include "http_call_registry.h" #include "http_message_parser.h" #include "http_server_impl.h" diff --git a/src/http/pprof_http_service.cpp b/src/http/pprof_http_service.cpp index cf6d519b21..f5aeb8b505 100644 --- a/src/http/pprof_http_service.cpp +++ b/src/http/pprof_http_service.cpp @@ -38,6 +38,7 @@ #include #include +#include "http/http_method.h" #include "http/http_server.h" #include "runtime/api_layer1.h" #include "utils/blob.h" diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 49a5de3815..db805f99ec 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -15,10 +15,16 @@ // specific language governing permissions and limitations // under the License. +// IWYU pragma: no_include +// IWYU pragma: no_include #include +#include #include #include "http/http_client.h" +#include "http/http_method.h" +#include "utils/error_code.h" +#include "utils/errors.h" namespace dsn { diff --git a/src/http/test/http_server_test.cpp b/src/http/test/http_server_test.cpp index eb72245bef..101d7fa8a8 100644 --- a/src/http/test/http_server_test.cpp +++ b/src/http/test/http_server_test.cpp @@ -18,6 +18,7 @@ // IWYU pragma: no_include // IWYU pragma: no_include #include +#include #include #include #include @@ -29,6 +30,7 @@ #include "http/builtin_http_calls.h" #include "http/http_call_registry.h" #include "http/http_message_parser.h" +#include "http/http_method.h" #include "http/http_server.h" #include "runtime/rpc/message_parser.h" #include "runtime/rpc/rpc_message.h" diff --git a/src/http/test/main.cpp b/src/http/test/main.cpp index ade2493dab..d2f24828a3 100644 --- a/src/http/test/main.cpp +++ b/src/http/test/main.cpp @@ -15,14 +15,18 @@ // specific language governing permissions and limitations // under the License. +#include #include #include -#include +#include #include #include +#include "http/http_method.h" #include "http/http_server.h" +#include "runtime/app_model.h" #include "runtime/service_app.h" +#include "utils/error_code.h" #include "utils/ports.h" int gtest_flags = 0; diff --git a/src/utils/metrics.cpp b/src/utils/metrics.cpp index bbbf81c397..b5800d3cd1 100644 --- a/src/utils/metrics.cpp +++ b/src/utils/metrics.cpp @@ -23,6 +23,7 @@ #include #include +#include "http/http_method.h" #include "runtime/api_layer1.h" #include "utils/flags.h" #include "utils/rand.h" From e0dc77126cf34eb709c45edf6a651b4dd7c57aa5 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Thu, 17 Aug 2023 11:59:46 +0800 Subject: [PATCH 13/29] feat: implement http client based on libcurl --- src/http/CMakeLists.txt | 2 +- src/http/http_client.h | 40 +++++++++++++++++++++++++++++- src/http/test/http_client_test.cpp | 10 ++++---- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/http/CMakeLists.txt b/src/http/CMakeLists.txt index 9978323f34..44f93796d8 100644 --- a/src/http/CMakeLists.txt +++ b/src/http/CMakeLists.txt @@ -21,7 +21,7 @@ set(MY_PROJ_SRC ${THIRDPARTY_ROOT}/build/Source/http-parser/http_parser.c) set(MY_SRC_SEARCH_MODE "GLOB") -set(MY_PROJ_LIBS curl) +set(MY_PROJ_LIBS "") dsn_add_static_library() diff --git a/src/http/http_client.h b/src/http/http_client.h index 821d56a9f5..5380663a86 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -40,7 +40,32 @@ namespace dsn { // Create an instance of http_client: // http_client client; // -// const auto &err = client.init(); +// It's necessary to initialize the new instance before coming into use: +// auto err = client.init(); +// +// Specify which http method you would use, such as GET or POST: +// err = client.set_method(method); +// +// Specify the target url that you would request for: +// err = client.set_url(method); +// +// Submit the request to remote http service: +// err = client.do_method(); +// +// If response data should be processed, use callback function: +// auto callback = [...](const void *data, size_t length) { +// ...... +// return true; +// }; +// err = client.do_method(callback); +// +// Or just provide a string pointer: +// std::string response; +// err = client.do_method(&response); +// +// Get the http status code after requesting: +// long http_status; +// err = client.get_http_status(http_status); class http_client { public: @@ -49,19 +74,31 @@ class http_client http_client(); ~http_client(); + // Before coming into use, init() must be called to initialize http client. It could also be + // called to reset the http clients that have been initialized previously. dsn::error_s init(); + // Specify which http method would be used, such as GET or POST: dsn::error_s set_method(http_method method); + + // Specify the target url that the request would be sent for: dsn::error_s set_url(const std::string &url); + + // Specify the maximum time in milliseconds that a request is allowed to complete. dsn::error_s set_timeout(long timeout_ms); + // Operations for the header fields. void clear_header_fields(); void set_accept(dsn::string_view val); void set_content_type(dsn::string_view val); + // Submit request to remote http service, with response processed by callback function. dsn::error_s do_method(const http_callback &callback = {}); + + // Submit request to remote http service, with response data returned in a string. dsn::error_s do_method(std::string *response); + // Get the http status code after requesting. dsn::error_s get_http_status(long &http_status) const; private: @@ -70,6 +107,7 @@ class http_client void clear_error_buf(); bool is_error_buf_empty() const; std::string to_error_msg(CURLcode code) const; + size_t on_response_data(const void *data, size_t length); void free_header_list(); diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index db805f99ec..8b9a86e051 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -31,10 +31,10 @@ namespace dsn { TEST(http_client_test, connect) { http_client client; - ASSERT_TRUE(client.init().is_ok()); + ASSERT_TRUE(client.init()); // No one has listened on port 20000, thus this would lead to "Connection refused". - client.set_url("http://127.0.0.1:20000/test/get"); + ASSERT_TRUE(client.set_url("http://127.0.0.1:20000/test/get")); const auto &err = client.do_method(); ASSERT_EQ(dsn::ERR_CURL_FAILED, err.code()); @@ -48,13 +48,13 @@ void test_http_client(http_client &client, const long expected_http_status, const std::string &expected_response) { - client.set_method(method); + ASSERT_TRUE(client.set_method(method)); std::string actual_response; ASSERT_TRUE(client.do_method(&actual_response)); long actual_http_status; - ASSERT_TRUE(client.get_http_status(actual_http_status).is_ok()); + ASSERT_TRUE(client.get_http_status(actual_http_status)); EXPECT_EQ(expected_http_status, actual_http_status); EXPECT_EQ(expected_response, actual_response); } @@ -62,7 +62,7 @@ void test_http_client(http_client &client, TEST(http_client_test, get) { http_client client; - ASSERT_TRUE(client.init().is_ok()); + ASSERT_TRUE(client.init()); client.set_url("http://127.0.0.1:20001/test/get"); From 9a8cc78eacd54b1a8e67b2cbc5bb89fdd1d8fc94 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Thu, 17 Aug 2023 21:22:36 +0800 Subject: [PATCH 14/29] feat: implement http client based on libcurl --- src/http/test/config-test.ini | 1 - src/http/test/main.cpp | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/http/test/config-test.ini b/src/http/test/config-test.ini index bc6bee57e7..d6ddedf1d0 100644 --- a/src/http/test/config-test.ini +++ b/src/http/test/config-test.ini @@ -29,7 +29,6 @@ arguments = run = true ports = 20001 count = 1 -delay_seconds = 1 pools = THREAD_POOL_DEFAULT [core] diff --git a/src/http/test/main.cpp b/src/http/test/main.cpp index d2f24828a3..81c43bc6bc 100644 --- a/src/http/test/main.cpp +++ b/src/http/test/main.cpp @@ -74,7 +74,7 @@ class test_service_app : public dsn::service_app dsn::start_http_server(); } - dsn::error_code start(const std::vector &args) + dsn::error_code start(const std::vector &args) override { gtest_ret = RUN_ALL_TESTS(); gtest_flags = 1; @@ -86,7 +86,7 @@ GTEST_API_ int main(int argc, char **argv) { testing::InitGoogleTest(&argc, argv); - // register all possible services + // Register test service. dsn::service_app::register_factory("test"); dsn_run_config("config-test.ini", false); From cba9830a08bae7ded970cbc99d8511190e5b6146 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Fri, 18 Aug 2023 12:14:46 +0800 Subject: [PATCH 15/29] feat: implement http client based on libcurl --- src/http/http_call_registry.h | 17 +++++--- src/http/http_server.cpp | 4 +- src/http/test/http_client_test.cpp | 64 ++++++++++++++++++++---------- src/http/test/http_server_test.cpp | 11 +++++ 4 files changed, 68 insertions(+), 28 deletions(-) diff --git a/src/http/http_call_registry.h b/src/http/http_call_registry.h index fef3b0c467..2e944463ce 100644 --- a/src/http/http_call_registry.h +++ b/src/http/http_call_registry.h @@ -35,11 +35,11 @@ class http_call_registry : public utils::singleton std::shared_ptr find(const std::string &path) const { std::lock_guard guard(_mu); - auto it = _call_map.find(path); - if (it == _call_map.end()) { + const auto &iter = _call_map.find(path); + if (iter == _call_map.end()) { return nullptr; } - return it->second; + return iter->second; } void remove(const std::string &path) @@ -48,14 +48,19 @@ class http_call_registry : public utils::singleton _call_map.erase(path); } - void add(std::unique_ptr call_uptr) + void add(const std::shared_ptr &call) { - auto call = std::shared_ptr(call_uptr.release()); std::lock_guard guard(_mu); - CHECK_EQ_MSG(_call_map.count(call->path), 0, call->path); + CHECK_EQ_MSG(_call_map.count(call->path), 0, "{} has been added", call->path); _call_map[call->path] = call; } + void add(std::unique_ptr call_uptr) + { + auto call = std::shared_ptr(call_uptr.release()); + add(call); + } + std::vector> list_all_calls() const { std::lock_guard guard(_mu); diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp index 088deb3be4..4c2f704016 100644 --- a/src/http/http_server.cpp +++ b/src/http/http_server.cpp @@ -147,8 +147,8 @@ void http_server::serve(message_ex *msg) resp.body = fmt::format("failed to parse request: {}", res.get_error()); } else { const http_request &req = res.get_value(); - std::shared_ptr call = http_call_registry::instance().find(req.path); - if (call != nullptr) { + auto call = http_call_registry::instance().find(req.path); + if (call) { call->callback(req, resp); } else { resp.status_code = http_status_code::not_found; diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 8b9a86e051..95ed8f7a8c 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -28,7 +28,7 @@ namespace dsn { -TEST(http_client_test, connect) +TEST(HttpClientTest, Connect) { http_client client; ASSERT_TRUE(client.init()); @@ -43,31 +43,55 @@ TEST(http_client_test, connect) std::cout << "failed to connect: " << err.description() << std::endl; } -void test_http_client(http_client &client, - const http_method method, - const long expected_http_status, - const std::string &expected_response) +using http_client_method_case = std::tuple; + +class HttpClientMethodTest : public testing::TestWithParam { - ASSERT_TRUE(client.set_method(method)); +public: + void SetUp() override + { + ASSERT_TRUE(_client.init()); + _client.set_url("http://127.0.0.1:20001/test/get"); + } - std::string actual_response; - ASSERT_TRUE(client.do_method(&actual_response)); + void test_mothod(const http_method method, + const long expected_http_status, + const std::string &expected_response) + { + ASSERT_TRUE(_client.set_method(method)); - long actual_http_status; - ASSERT_TRUE(client.get_http_status(actual_http_status)); - EXPECT_EQ(expected_http_status, actual_http_status); - EXPECT_EQ(expected_response, actual_response); -} + std::string actual_response; + ASSERT_TRUE(_client.do_method(&actual_response)); -TEST(http_client_test, get) -{ - http_client client; - ASSERT_TRUE(client.init()); + long actual_http_status; + ASSERT_TRUE(_client.get_http_status(actual_http_status)); - client.set_url("http://127.0.0.1:20001/test/get"); + EXPECT_EQ(expected_http_status, actual_http_status); + EXPECT_EQ(expected_response, actual_response); + } - test_http_client(client, http_method::POST, 400, "please use GET method"); - test_http_client(client, http_method::GET, 200, "you are using GET method"); +private: + http_client _client; +}; + +TEST_P(HttpClientMethodTest, Get) +{ + http_method method; + long expected_http_status; + const char *expected_response; + std::tie(method, expected_http_status, expected_response) = GetParam(); + + http_client _client; + test_mothod(method, expected_http_status, expected_response); } +const std::vector http_client_method_tests = { + {http_method::POST, 400, "please use GET method"}, + {http_method::GET, 200, "you are using GET method"}, +}; + +INSTANTIATE_TEST_CASE_P(HttpClientTest, + HttpClientMethodTest, + testing::ValuesIn(http_client_method_tests)); + } // namespace dsn diff --git a/src/http/test/http_server_test.cpp b/src/http/test/http_server_test.cpp index 101d7fa8a8..0c6b9ec6d3 100644 --- a/src/http/test/http_server_test.cpp +++ b/src/http/test/http_server_test.cpp @@ -89,7 +89,12 @@ TEST(bultin_http_calls_test, meta_query) TEST(bultin_http_calls_test, get_help) { + // Used to save current http calls as backup. + std::vector> backup_calls; + + // Remove all http calls. for (const auto &call : http_call_registry::instance().list_all_calls()) { + backup_calls.push_back(call); http_call_registry::instance().remove(call->path); } @@ -113,9 +118,15 @@ TEST(bultin_http_calls_test, get_help) get_help_handler(req, resp); ASSERT_EQ(resp.body, "{\"/\":\"ip:port/\",\"/recentStartTime\":\"ip:port/recentStartTime\"}\n"); + // Remove all http calls, especially `recentStartTime`. for (const auto &call : http_call_registry::instance().list_all_calls()) { http_call_registry::instance().remove(call->path); } + + // Recover http calls from backup. + for (const auto &call : backup_calls) { + http_call_registry::instance().add(call); + } } class http_message_parser_test : public testing::Test From 8f2ee80fd38dc042e3dd1700af011618b6448510 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Fri, 18 Aug 2023 16:33:04 +0800 Subject: [PATCH 16/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 35 ++++++++++++++-------- src/http/http_client.h | 24 ++++++++++----- src/http/test/config-test.ini | 2 +- src/http/test/http_client_test.cpp | 48 ++++++++++++++++++++++-------- src/http/test/main.cpp | 27 +++++++++++++---- 5 files changed, 99 insertions(+), 37 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 00f9b2fa42..809ab21414 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -140,7 +140,7 @@ dsn::error_s http_client::init() // This http_client object itself is passed to the callback function. RETURN_IF_SETOPT_NOT_OK(CURLOPT_WRITEDATA, reinterpret_cast(this)); - return error_s::ok(); + return dsn::error_s::ok(); } void http_client::clear_error_buf() { _error_buf[0] = 0; } @@ -172,15 +172,34 @@ size_t http_client::on_response_data(const void *data, size_t length) return (*_callback)(data, length) ? length : std::numeric_limits::max(); } +dsn::error_s http_client::set_url(const std::string &url) +{ + RETURN_IF_SETOPT_NOT_OK(CURLOPT_URL, url.c_str()); + + _url = url; + return dsn::error_s::ok(); +} + +dsn::error_s http_client::with_post_method(const std::string &data) +{ + // No need to enable CURLOPT_POST by `RETURN_IF_SETOPT_NOT_OK(CURLOPT_POST, 1L)`, since using + // either of CURLOPT_POSTFIELDS or CURLOPT_COPYPOSTFIELDS implies setting CURLOPT_POST to 1. + RETURN_IF_SETOPT_NOT_OK(CURLOPT_POSTFIELDSIZE, static_cast(data.size())); + RETURN_IF_SETOPT_NOT_OK(CURLOPT_COPYPOSTFIELDS, data.data()); + _method = http_method::POST; + return dsn::error_s::ok(); +} + +dsn::error_s http_client::with_get_method() { return set_method(http_method::GET); } + dsn::error_s http_client::set_method(http_method method) { + // No need to process the case of http_method::POST, since it should be enabled by + // `with_post_method`. switch (method) { case http_method::GET: RETURN_IF_SETOPT_NOT_OK(CURLOPT_HTTPGET, 1L); break; - case http_method::POST: - RETURN_IF_SETOPT_NOT_OK(CURLOPT_POST, 1L); - break; default: LOG_FATAL("Unsupported http_method"); } @@ -189,14 +208,6 @@ dsn::error_s http_client::set_method(http_method method) return dsn::error_s::ok(); } -dsn::error_s http_client::set_url(const std::string &url) -{ - RETURN_IF_SETOPT_NOT_OK(CURLOPT_URL, url.c_str()); - - _url = url; - return dsn::error_s::ok(); -} - dsn::error_s http_client::set_timeout(long timeout_ms) { RETURN_IF_SETOPT_NOT_OK(CURLOPT_TIMEOUT_MS, timeout_ms); diff --git a/src/http/http_client.h b/src/http/http_client.h index 5380663a86..61ab74f40f 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -43,12 +43,15 @@ namespace dsn { // It's necessary to initialize the new instance before coming into use: // auto err = client.init(); // -// Specify which http method you would use, such as GET or POST: -// err = client.set_method(method); -// // Specify the target url that you would request for: // err = client.set_url(method); // +// If you would use GET method, call `with_get_method`: +// err = client.with_get_method(); +// +// If you would use POST method, call `with_post_method` with post data: +// err = client.with_post_method(post_data); +// // Submit the request to remote http service: // err = client.do_method(); // @@ -78,12 +81,15 @@ class http_client // called to reset the http clients that have been initialized previously. dsn::error_s init(); - // Specify which http method would be used, such as GET or POST: - dsn::error_s set_method(http_method method); - - // Specify the target url that the request would be sent for: + // Specify the target url that the request would be sent for. dsn::error_s set_url(const std::string &url); + // Using post method, with `data` as the payload for post body. + dsn::error_s with_post_method(const std::string &data); + + // Using get method. + dsn::error_s with_get_method(); + // Specify the maximum time in milliseconds that a request is allowed to complete. dsn::error_s set_timeout(long timeout_ms); @@ -110,6 +116,10 @@ class http_client size_t on_response_data(const void *data, size_t length); + // Specify which http method would be used, such as GET. Enabling POST should not use this + // function (use `with_post_method` instead). + dsn::error_s set_method(http_method method); + void free_header_list(); void set_header_field(dsn::string_view key, dsn::string_view val); dsn::error_s process_header(); diff --git a/src/http/test/config-test.ini b/src/http/test/config-test.ini index d6ddedf1d0..744d8d412a 100644 --- a/src/http/test/config-test.ini +++ b/src/http/test/config-test.ini @@ -43,7 +43,7 @@ logging_factory_name = dsn::tools::simple_logger [tools.simple_logger] fast_flush = true short_header = false -stderr_start_level = LOG_LEVEL_ERROR +stderr_start_level = LOG_LEVEL_DEBUG [network] ; how many network threads for network library (used by asio) diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 95ed8f7a8c..6b1e3507d0 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -16,10 +16,12 @@ // under the License. // IWYU pragma: no_include +// IWYU pragma: no_include // IWYU pragma: no_include #include #include #include +#include #include "http/http_client.h" #include "http/http_method.h" @@ -43,22 +45,32 @@ TEST(HttpClientTest, Connect) std::cout << "failed to connect: " << err.description() << std::endl; } -using http_client_method_case = std::tuple; +using http_client_method_case = + std::tuple; class HttpClientMethodTest : public testing::TestWithParam { public: - void SetUp() override - { - ASSERT_TRUE(_client.init()); - _client.set_url("http://127.0.0.1:20001/test/get"); - } + void SetUp() override { ASSERT_TRUE(_client.init()); } - void test_mothod(const http_method method, + void test_mothod(const std::string &url, + const http_method method, + const std::string &post_data, const long expected_http_status, const std::string &expected_response) { - ASSERT_TRUE(_client.set_method(method)); + _client.set_url(url); + + switch (method) { + case http_method::GET: + ASSERT_TRUE(_client.with_get_method()); + break; + case http_method::POST: + ASSERT_TRUE(_client.with_post_method(post_data)); + break; + default: + LOG_FATAL("Unsupported http_method"); + } std::string actual_response; ASSERT_TRUE(_client.do_method(&actual_response)); @@ -76,18 +88,30 @@ class HttpClientMethodTest : public testing::TestWithParam http_client_method_tests = { - {http_method::POST, 400, "please use GET method"}, - {http_method::GET, 200, "you are using GET method"}, + {"http://127.0.0.1:20001/test/get", + http_method::POST, + "with POST DATA", + 400, + "please use GET method"}, + {"http://127.0.0.1:20001/test/get", http_method::GET, "", 200, "you are using GET method"}, + {"http://127.0.0.1:20001/test/post", + http_method::POST, + "with POST DATA", + 200, + "you are using POST method with POST DATA"}, + {"http://127.0.0.1:20001/test/post", http_method::GET, "", 400, "please use POST method"}, }; INSTANTIATE_TEST_CASE_P(HttpClientTest, diff --git a/src/http/test/main.cpp b/src/http/test/main.cpp index 81c43bc6bc..8e9a33d829 100644 --- a/src/http/test/main.cpp +++ b/src/http/test/main.cpp @@ -38,11 +38,19 @@ class test_http_service : public dsn::http_server_base test_http_service() { register_handler("get", - std::bind(&test_http_service::get_handler, + std::bind(&test_http_service::method_handler, this, + dsn::http_method::GET, std::placeholders::_1, std::placeholders::_2), "ip:port/test/get"); + register_handler("post", + std::bind(&test_http_service::method_handler, + this, + dsn::http_method::POST, + std::placeholders::_1, + std::placeholders::_2), + "ip:port/test/post"); } ~test_http_service() = default; @@ -50,15 +58,24 @@ class test_http_service : public dsn::http_server_base std::string path() const override { return "test"; } private: - void get_handler(const dsn::http_request &req, dsn::http_response &resp) + void method_handler(dsn::http_method target_method, + const dsn::http_request &req, + dsn::http_response &resp) { - if (req.method != dsn::http_method::GET) { - resp.body = "please use GET method"; + if (req.method != target_method) { + resp.body = fmt::format("please use {} method", enum_to_string(target_method)); resp.status_code = dsn::http_status_code::bad_request; return; } - resp.body = "you are using GET method"; + std::string postfix; + if (target_method == dsn::http_method::POST) { + postfix = " "; + postfix += req.body.to_string(); + } + + resp.body = + fmt::format("you are using {} method{}", enum_to_string(target_method), postfix); resp.status_code = dsn::http_status_code::ok; } From 146d894f0d1e4d798bb2f53efed2866fbbc7fb03 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Fri, 18 Aug 2023 21:00:19 +0800 Subject: [PATCH 17/29] feat: implement http client based on libcurl --- src/http/http_client.cpp | 1 + src/http/test/http_client_test.cpp | 1 + src/http/test/main.cpp | 2 ++ 3 files changed, 4 insertions(+) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 809ab21414..fdba8fd15b 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -184,6 +184,7 @@ dsn::error_s http_client::with_post_method(const std::string &data) { // No need to enable CURLOPT_POST by `RETURN_IF_SETOPT_NOT_OK(CURLOPT_POST, 1L)`, since using // either of CURLOPT_POSTFIELDS or CURLOPT_COPYPOSTFIELDS implies setting CURLOPT_POST to 1. + // See https://curl.se/libcurl/c/CURLOPT_POSTFIELDS.html for details. RETURN_IF_SETOPT_NOT_OK(CURLOPT_POSTFIELDSIZE, static_cast(data.size())); RETURN_IF_SETOPT_NOT_OK(CURLOPT_COPYPOSTFIELDS, data.data()); _method = http_method::POST; diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 6b1e3507d0..5e98fc6b2a 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -27,6 +27,7 @@ #include "http/http_method.h" #include "utils/error_code.h" #include "utils/errors.h" +#include "utils/fmt_logging.h" namespace dsn { diff --git a/src/http/test/main.cpp b/src/http/test/main.cpp index 8e9a33d829..1eeb3d04ef 100644 --- a/src/http/test/main.cpp +++ b/src/http/test/main.cpp @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include #include @@ -26,6 +27,7 @@ #include "http/http_server.h" #include "runtime/app_model.h" #include "runtime/service_app.h" +#include "utils/blob.h" #include "utils/error_code.h" #include "utils/ports.h" From efdac545a62b8d0252b5df346c55b09141ed8af8 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Fri, 22 Sep 2023 12:23:06 +0800 Subject: [PATCH 18/29] feat(http): implement http client based on libcurl --- src/http/http_client.cpp | 2 +- src/http/http_client.h | 8 +++++++- src/http/test/http_client_test.cpp | 6 +++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index fdba8fd15b..752d87b3c3 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -93,7 +93,7 @@ inline dsn::error_code to_error_code(CURLcode code) #define RETURN_IF_DO_METHOD_NOT_OK() \ RETURN_IF_CURL_NOT_OK(curl_easy_perform(_curl), \ - "failed to perform [method={}, url={}]", \ + "failed to perform http request(method={}, url={})", \ enum_to_string(_method), \ _url) diff --git a/src/http/http_client.h b/src/http/http_client.h index 61ab74f40f..83e338fbc6 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -99,12 +99,18 @@ class http_client void set_content_type(dsn::string_view val); // Submit request to remote http service, with response processed by callback function. + // + // This function would run synchronously, which means it would wait until the response was + // returned and processed appropriately. dsn::error_s do_method(const http_callback &callback = {}); // Submit request to remote http service, with response data returned in a string. + // + // This function would run synchronously, which means it would wait until the response was + // returned and processed appropriately. dsn::error_s do_method(std::string *response); - // Get the http status code after requesting. + // Get the last http status code after requesting. dsn::error_s get_http_status(long &http_status) const; private: diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 5e98fc6b2a..f240450113 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -42,8 +42,12 @@ TEST(HttpClientTest, Connect) const auto &err = client.do_method(); ASSERT_EQ(dsn::ERR_CURL_FAILED, err.code()); - // Would print something like "Failed to connect to 127.0.0.1 port 20000: Connection refused". std::cout << "failed to connect: " << err.description() << std::endl; + const std::string expected_description("ERR_CURL_FAILED: failed to perform http request(" + "method=GET, url=http://127.0.0.1:20000/test/get): " + "desc=\"Couldn't connect to server\", msg=\"Failed to " + "connect to 127.0.0.1 port 20000: Connection refused\""); + EXPECT_EQ(expected_description, err.description()); } using http_client_method_case = From 2d337b96747cd4d311715ad76a7b69559ca3d562 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Fri, 22 Sep 2023 21:55:15 +0800 Subject: [PATCH 19/29] feat(http): implement http client based on libcurl --- src/http/http_client.cpp | 35 ++++++++--- src/http/http_client.h | 10 +++- src/http/test/http_client_test.cpp | 96 ++++++++++++++++++++++++------ src/utils/error_code.h | 2 + 4 files changed, 115 insertions(+), 28 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 752d87b3c3..fb021cc65e 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -37,7 +37,7 @@ DSN_DEFINE_uint32(http, http_client::http_client() : _curl(nullptr), _method(http_method::GET), - _callback(nullptr), + _recv_callback(nullptr), _header_changed(true), _header_list(nullptr) { @@ -66,9 +66,18 @@ inline dsn::error_code to_error_code(CURLcode code) return dsn::ERR_OK; } + if (code == CURLE_COULDNT_CONNECT) { + return dsn::ERR_CURL_CONNECT_FAILED; + } + if (code == CURLE_OPERATION_TIMEDOUT) { return dsn::ERR_TIMEOUT; } + + if (code == CURLE_WRITE_ERROR) { + return dsn::ERR_CURL_WRITE_ERROR; + } + return dsn::ERR_CURL_FAILED; } @@ -149,7 +158,8 @@ bool http_client::is_error_buf_empty() const { return _error_buf[0] == 0; } std::string http_client::to_error_msg(CURLcode code) const { - std::string err_msg = fmt::format("desc=\"{}\"", curl_easy_strerror(code)); + std::string err_msg = + fmt::format("code={}, desc=\"{}\"", static_cast(code), curl_easy_strerror(code)); if (is_error_buf_empty()) { return err_msg; } @@ -158,18 +168,27 @@ std::string http_client::to_error_msg(CURLcode code) const return err_msg; } +// `data` passed to this function is NOT null-terminated. +// `length` might be zero. size_t http_client::on_response_data(const void *data, size_t length) { - if (_callback == nullptr) { + if (_recv_callback == nullptr) { return length; } - // callback function is empty. - if (!(*_callback)) { + if (!(*_recv_callback)) { + // callback function is empty. return length; } - return (*_callback)(data, length) ? length : std::numeric_limits::max(); + // According to libcurl, callback should return the number of bytes actually taken care of. + // If that amount differs from the amount passed to callback function, it would signals an + // error condition. This causes the transfer to get aborted and the libcurl function used + // returns CURLE_WRITE_ERROR. Therefore, here we just return the max limit of size_t for + // failure. + // + // See https://curl.se/libcurl/c/CURLOPT_WRITEFUNCTION.html for details. + return (*_recv_callback)(data, length) ? length : std::numeric_limits::max(); } dsn::error_s http_client::set_url(const std::string &url) @@ -275,11 +294,11 @@ dsn::error_s http_client::process_header() return dsn::error_s::ok(); } -dsn::error_s http_client::do_method(const http_client::http_callback &callback) +dsn::error_s http_client::do_method(const http_client::recv_callback &callback) { // `curl_easy_perform` would run synchronously, thus it is safe to use the pointer to // `callback`. - _callback = &callback; + _recv_callback = &callback; const auto &err = process_header(); if (!err) { diff --git a/src/http/http_client.h b/src/http/http_client.h index 83e338fbc6..9a4e935aa4 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -72,7 +72,7 @@ namespace dsn { class http_client { public: - using http_callback = std::function; + using recv_callback = std::function; http_client(); ~http_client(); @@ -100,9 +100,13 @@ class http_client // Submit request to remote http service, with response processed by callback function. // + // `callback` function gets called by libcurl as soon as there is data received that needs + // to be saved. For most transfers, this callback gets called many times and each invoke + // delivers another chunk of data. + // // This function would run synchronously, which means it would wait until the response was // returned and processed appropriately. - dsn::error_s do_method(const http_callback &callback = {}); + dsn::error_s do_method(const recv_callback &callback = {}); // Submit request to remote http service, with response data returned in a string. // @@ -137,7 +141,7 @@ class http_client CURL *_curl; http_method _method; std::string _url; - const http_callback *_callback; + const recv_callback *_recv_callback; char _error_buf[kErrorBufferBytes]; bool _header_changed; diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index f240450113..3096b7bfdf 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -18,6 +18,7 @@ // IWYU pragma: no_include // IWYU pragma: no_include // IWYU pragma: no_include +#include #include #include #include @@ -40,14 +41,45 @@ TEST(HttpClientTest, Connect) ASSERT_TRUE(client.set_url("http://127.0.0.1:20000/test/get")); const auto &err = client.do_method(); - ASSERT_EQ(dsn::ERR_CURL_FAILED, err.code()); - - std::cout << "failed to connect: " << err.description() << std::endl; - const std::string expected_description("ERR_CURL_FAILED: failed to perform http request(" - "method=GET, url=http://127.0.0.1:20000/test/get): " - "desc=\"Couldn't connect to server\", msg=\"Failed to " - "connect to 127.0.0.1 port 20000: Connection refused\""); - EXPECT_EQ(expected_description, err.description()); + ASSERT_EQ(dsn::ERR_CURL_CONNECT_FAILED, err.code()); + + const std::string actual_description(err.description()); + std::cout << "failed to connect: " << actual_description << std::endl; + + const std::string expected_description_prefix( + "ERR_CURL_CONNECT_FAILED: failed to perform http request(" + "method=GET, url=http://127.0.0.1:20000/test/get): code=7"); + ASSERT_LT(expected_description_prefix.size(), actual_description.size()); + EXPECT_EQ(expected_description_prefix, + actual_description.substr(0, expected_description_prefix.size())); +} + +TEST(HttpClientTest, Callback) +{ + http_client client; + ASSERT_TRUE(client.init()); + + ASSERT_TRUE(client.set_url("http://127.0.0.1:20001/test/get")); + ASSERT_TRUE(client.with_get_method()); + + auto callback = [](const void *, size_t) { return false; }; + + const auto &err = client.do_method(callback); + ASSERT_EQ(dsn::ERR_CURL_WRITE_ERROR, err.code()); + + long actual_http_status; + ASSERT_TRUE(client.get_http_status(actual_http_status)); + EXPECT_EQ(200, actual_http_status); + + const std::string actual_description(err.description()); + std::cout << "failed for callback: " << actual_description << std::endl; + const auto expected_description = + fmt::format("ERR_CURL_WRITE_ERROR: failed to perform http request(" + "method=GET, url=http://127.0.0.1:20001/test/get): code=23, " + "desc=\"Failed writing received data to disk/application\", " + "msg=\"Failed writing body ({} != 24)\"", + std::numeric_limits::max()); + EXPECT_EQ(expected_description, actual_description); } using http_client_method_case = @@ -58,6 +90,42 @@ class HttpClientMethodTest : public testing::TestWithParam Date: Sun, 24 Sep 2023 16:31:00 +0800 Subject: [PATCH 20/29] feat(http): implement http client based on libcurl --- src/http/test/http_client_test.cpp | 47 +++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 3096b7bfdf..7bb15ea2d7 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -15,12 +15,14 @@ // specific language governing permissions and limitations // under the License. +#include +#include // IWYU pragma: no_include // IWYU pragma: no_include // IWYU pragma: no_include #include -#include #include +#include #include #include @@ -32,6 +34,17 @@ namespace dsn { +void check_expected_description_prefix(const std::string &expected_description_prefix, + const dsn::error_s &err) +{ + const std::string actual_description(err.description()); + std::cout << actual_description << std::endl; + + ASSERT_LT(expected_description_prefix.size(), actual_description.size()); + EXPECT_EQ(expected_description_prefix, + actual_description.substr(0, expected_description_prefix.size())); +} + TEST(HttpClientTest, Connect) { http_client client; @@ -43,15 +56,18 @@ TEST(HttpClientTest, Connect) const auto &err = client.do_method(); ASSERT_EQ(dsn::ERR_CURL_CONNECT_FAILED, err.code()); - const std::string actual_description(err.description()); - std::cout << "failed to connect: " << actual_description << std::endl; + std::cout << "failed to connect: "; + // We just check the prefix of description, including `method`, `url`, `code` and `desc`. + // The `msg` differ in various systems, such as: + // * msg="Failed to connect to 127.0.0.1 port 20000: Connection refused" + // * msg="Failed to connect to 127.0.0.1 port 20000 after 0 ms: Connection refused" + // Thus we don't check if `msg` fields are consistent. const std::string expected_description_prefix( "ERR_CURL_CONNECT_FAILED: failed to perform http request(" - "method=GET, url=http://127.0.0.1:20000/test/get): code=7"); - ASSERT_LT(expected_description_prefix.size(), actual_description.size()); - EXPECT_EQ(expected_description_prefix, - actual_description.substr(0, expected_description_prefix.size())); + "method=GET, url=http://127.0.0.1:20000/test/get): code=7, " + "desc=\"Couldn't connect to server\""); + check_expected_description_prefix(expected_description_prefix, err); } TEST(HttpClientTest, Callback) @@ -71,15 +87,18 @@ TEST(HttpClientTest, Callback) ASSERT_TRUE(client.get_http_status(actual_http_status)); EXPECT_EQ(200, actual_http_status); - const std::string actual_description(err.description()); - std::cout << "failed for callback: " << actual_description << std::endl; - const auto expected_description = + std::cout << "failed for callback: "; + + // We just check the prefix of description, including `method`, `url`, `code` and `desc`. + // The `msg` differ in various systems, such as: + // * msg="Failed writing body (18446744073709551615 != 24)" + // * msg="Failure writing output to destination" + // Thus we don't check if `msg` fields are consistent. + const auto expected_description_prefix = fmt::format("ERR_CURL_WRITE_ERROR: failed to perform http request(" "method=GET, url=http://127.0.0.1:20001/test/get): code=23, " - "desc=\"Failed writing received data to disk/application\", " - "msg=\"Failed writing body ({} != 24)\"", - std::numeric_limits::max()); - EXPECT_EQ(expected_description, actual_description); + "desc=\"Failed writing received data to disk/application\""); + check_expected_description_prefix(expected_description_prefix, err); } using http_client_method_case = From c9f7fc04c7f8a9754485fb0d4ea8ca5c3573ecda Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Mon, 25 Sep 2023 10:36:32 +0800 Subject: [PATCH 21/29] feat(http): implement http client based on libcurl --- src/http/test/http_client_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 7bb15ea2d7..051c936bc5 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -22,7 +22,6 @@ // IWYU pragma: no_include #include #include -#include #include #include From e819f03df4a2f5040ae19fe73f2bc25af80ffcd1 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Mon, 25 Sep 2023 12:49:29 +0800 Subject: [PATCH 22/29] feat(http): implement http client based on libcurl --- src/http/http_client.cpp | 8 ++++---- src/http/http_client.h | 10 +++++----- src/http/test/http_client_test.cpp | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index fb021cc65e..075ea4742b 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -294,7 +294,7 @@ dsn::error_s http_client::process_header() return dsn::error_s::ok(); } -dsn::error_s http_client::do_method(const http_client::recv_callback &callback) +dsn::error_s http_client::exec_method(const http_client::recv_callback &callback) { // `curl_easy_perform` would run synchronously, thus it is safe to use the pointer to // `callback`. @@ -309,10 +309,10 @@ dsn::error_s http_client::do_method(const http_client::recv_callback &callback) return dsn::error_s::ok(); } -dsn::error_s http_client::do_method(std::string *response) +dsn::error_s http_client::exec_method(std::string *response) { if (response == nullptr) { - return do_method(); + return exec_method(); } auto callback = [response](const void *data, size_t length) { @@ -320,7 +320,7 @@ dsn::error_s http_client::do_method(std::string *response) return true; }; - return do_method(callback); + return exec_method(callback); } dsn::error_s http_client::get_http_status(long &http_status) const diff --git a/src/http/http_client.h b/src/http/http_client.h index 9a4e935aa4..190af11f2b 100644 --- a/src/http/http_client.h +++ b/src/http/http_client.h @@ -53,18 +53,18 @@ namespace dsn { // err = client.with_post_method(post_data); // // Submit the request to remote http service: -// err = client.do_method(); +// err = client.exec_method(); // // If response data should be processed, use callback function: // auto callback = [...](const void *data, size_t length) { // ...... // return true; // }; -// err = client.do_method(callback); +// err = client.exec_method(callback); // // Or just provide a string pointer: // std::string response; -// err = client.do_method(&response); +// err = client.exec_method(&response); // // Get the http status code after requesting: // long http_status; @@ -106,13 +106,13 @@ class http_client // // This function would run synchronously, which means it would wait until the response was // returned and processed appropriately. - dsn::error_s do_method(const recv_callback &callback = {}); + dsn::error_s exec_method(const recv_callback &callback = {}); // Submit request to remote http service, with response data returned in a string. // // This function would run synchronously, which means it would wait until the response was // returned and processed appropriately. - dsn::error_s do_method(std::string *response); + dsn::error_s exec_method(std::string *response); // Get the last http status code after requesting. dsn::error_s get_http_status(long &http_status) const; diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 051c936bc5..8354354f20 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -52,7 +52,7 @@ TEST(HttpClientTest, Connect) // No one has listened on port 20000, thus this would lead to "Connection refused". ASSERT_TRUE(client.set_url("http://127.0.0.1:20000/test/get")); - const auto &err = client.do_method(); + const auto &err = client.exec_method(); ASSERT_EQ(dsn::ERR_CURL_CONNECT_FAILED, err.code()); std::cout << "failed to connect: "; @@ -79,7 +79,7 @@ TEST(HttpClientTest, Callback) auto callback = [](const void *, size_t) { return false; }; - const auto &err = client.do_method(callback); + const auto &err = client.exec_method(callback); ASSERT_EQ(dsn::ERR_CURL_WRITE_ERROR, err.code()); long actual_http_status; @@ -112,7 +112,7 @@ class HttpClientMethodTest : public testing::TestWithParam Date: Mon, 25 Sep 2023 14:37:50 +0800 Subject: [PATCH 23/29] feat(http): implement http client based on libcurl --- thirdparty/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index f32d3c11ea..4558aa94f7 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -295,6 +295,7 @@ ExternalProject_Add(curl --disable-smtp --disable-telnet --disable-tftp + --disable-shared --without-librtmp --without-zlib --without-libssh2 From ff1fadbc49447b09e2eab04e17eb33e6d3c02afb Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Mon, 25 Sep 2023 16:31:52 +0800 Subject: [PATCH 24/29] feat(http): implement http client based on libcurl --- thirdparty/CMakeLists.txt | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index 4558aa94f7..a70b4b100e 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -276,11 +276,7 @@ ExternalProject_Add(civetweb ExternalProject_Get_property(civetweb SOURCE_DIR) set(civetweb_SRC ${SOURCE_DIR}) -ExternalProject_Add(curl - URL ${OSS_URL_PREFIX}/curl-7.47.0.tar.gz - http://curl.haxx.se/download/curl-7.47.0.tar.gz - URL_MD5 5109d1232d208dfd712c0272b8360393 - CONFIGURE_COMMAND ./configure --prefix=${TP_OUTPUT} +set(CURL_OPTIONS --disable-dict --disable-file --disable-ftp @@ -300,7 +296,16 @@ ExternalProject_Add(curl --without-zlib --without-libssh2 --without-ssl - --without-libidn + --without-libidn) +if (APPLE) + set(CURL_OPTIONS ${CURL_OPTIONS} --without-nghttp2) +endif () +ExternalProject_Add(curl + URL ${OSS_URL_PREFIX}/curl-7.47.0.tar.gz + http://curl.haxx.se/download/curl-7.47.0.tar.gz + URL_MD5 5109d1232d208dfd712c0272b8360393 + CONFIGURE_COMMAND ./configure --prefix=${TP_OUTPUT} + ${CURL_OPTIONS} BUILD_IN_SOURCE 1 ) From 5d87a7f29f08831613b75dc36ad71191164565f6 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Tue, 26 Sep 2023 10:54:15 +0800 Subject: [PATCH 25/29] feat(http): implement http client based on libcurl --- thirdparty/CMakeLists.txt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index a70b4b100e..63435f5ec1 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -296,14 +296,15 @@ set(CURL_OPTIONS --without-zlib --without-libssh2 --without-ssl - --without-libidn) + --without-libidn + --without-zstd) if (APPLE) set(CURL_OPTIONS ${CURL_OPTIONS} --without-nghttp2) endif () ExternalProject_Add(curl - URL ${OSS_URL_PREFIX}/curl-7.47.0.tar.gz - http://curl.haxx.se/download/curl-7.47.0.tar.gz - URL_MD5 5109d1232d208dfd712c0272b8360393 + URL ${OSS_URL_PREFIX}/curl-8.3.0.tar.gz + http://curl.haxx.se/download/curl-8.3.0.tar.gz + URL_MD5 7e614827c4165bf30f9770577259561e CONFIGURE_COMMAND ./configure --prefix=${TP_OUTPUT} ${CURL_OPTIONS} BUILD_IN_SOURCE 1 From b54db08618dd5c5e40dfd45b747b62e0fb23d6eb Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Tue, 26 Sep 2023 12:28:51 +0800 Subject: [PATCH 26/29] feat(http): implement http client based on libcurl --- thirdparty/CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index 63435f5ec1..2af9d43356 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -299,7 +299,11 @@ set(CURL_OPTIONS --without-libidn --without-zstd) if (APPLE) - set(CURL_OPTIONS ${CURL_OPTIONS} --without-nghttp2) + set(CURL_OPTIONS + ${CURL_OPTIONS} + --without-nghttp2 + --without-libidn2 + --without-brotli) endif () ExternalProject_Add(curl URL ${OSS_URL_PREFIX}/curl-8.3.0.tar.gz From 832c7746b790a9f0a9bfd5148e1019de56e27744 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Tue, 26 Sep 2023 15:36:16 +0800 Subject: [PATCH 27/29] feat(http): implement http client based on libcurl --- src/http/http_client.cpp | 6 +++--- src/http/test/http_client_test.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 075ea4742b..12ff7a8e22 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -100,7 +100,7 @@ inline dsn::error_code to_error_code(CURLcode code) #define RETURN_IF_GETINFO_NOT_OK(info, output) \ RETURN_IF_CURL_NOT_OK(curl_easy_getinfo(_curl, info, output), "failed to get from " #info) -#define RETURN_IF_DO_METHOD_NOT_OK() \ +#define RETURN_IF_EXEC_METHOD_NOT_OK() \ RETURN_IF_CURL_NOT_OK(curl_easy_perform(_curl), \ "failed to perform http request(method={}, url={})", \ enum_to_string(_method), \ @@ -305,7 +305,7 @@ dsn::error_s http_client::exec_method(const http_client::recv_callback &callback return err; } - RETURN_IF_DO_METHOD_NOT_OK(); + RETURN_IF_EXEC_METHOD_NOT_OK(); return dsn::error_s::ok(); } @@ -329,7 +329,7 @@ dsn::error_s http_client::get_http_status(long &http_status) const return dsn::error_s::ok(); } -#undef RETURN_IF_DO_METHOD_NOT_OK +#undef RETURN_IF_EXEC_METHOD_NOT_OK #undef RETURN_IF_SETOPT_NOT_OK #undef RETURN_IF_CURL_NOT_OK diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 8354354f20..5475cfde70 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -171,7 +171,7 @@ class HttpClientMethodTest : public testing::TestWithParam Date: Thu, 28 Sep 2023 04:42:56 +0800 Subject: [PATCH 28/29] feat(http): implement http client based on libcurl --- src/http/http_client.cpp | 24 ++++++------------------ src/http/test/http_client_test.cpp | 19 +++++++++++++------ src/utils/error_code.h | 2 -- src/utils/errors.h | 2 +- 4 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/http/http_client.cpp b/src/http/http_client.cpp index 12ff7a8e22..32ae5eaea1 100644 --- a/src/http/http_client.cpp +++ b/src/http/http_client.cpp @@ -62,23 +62,14 @@ namespace { inline dsn::error_code to_error_code(CURLcode code) { - if (code == CURLE_OK) { + switch (code) { + case CURLE_OK: return dsn::ERR_OK; - } - - if (code == CURLE_COULDNT_CONNECT) { - return dsn::ERR_CURL_CONNECT_FAILED; - } - - if (code == CURLE_OPERATION_TIMEDOUT) { + case CURLE_OPERATION_TIMEDOUT: return dsn::ERR_TIMEOUT; + default: + return dsn::ERR_CURL_FAILED; } - - if (code == CURLE_WRITE_ERROR) { - return dsn::ERR_CURL_WRITE_ERROR; - } - - return dsn::ERR_CURL_FAILED; } } // anonymous namespace @@ -300,10 +291,7 @@ dsn::error_s http_client::exec_method(const http_client::recv_callback &callback // `callback`. _recv_callback = &callback; - const auto &err = process_header(); - if (!err) { - return err; - } + RETURN_NOT_OK(process_header()); RETURN_IF_EXEC_METHOD_NOT_OK(); return dsn::error_s::ok(); diff --git a/src/http/test/http_client_test.cpp b/src/http/test/http_client_test.cpp index 5475cfde70..d15cb7bdd4 100644 --- a/src/http/test/http_client_test.cpp +++ b/src/http/test/http_client_test.cpp @@ -30,6 +30,7 @@ #include "utils/error_code.h" #include "utils/errors.h" #include "utils/fmt_logging.h" +#include "utils/test_macros.h" namespace dsn { @@ -53,20 +54,23 @@ TEST(HttpClientTest, Connect) ASSERT_TRUE(client.set_url("http://127.0.0.1:20000/test/get")); const auto &err = client.exec_method(); - ASSERT_EQ(dsn::ERR_CURL_CONNECT_FAILED, err.code()); + ASSERT_EQ(dsn::ERR_CURL_FAILED, err.code()); std::cout << "failed to connect: "; + // "code=7" means CURLE_COULDNT_CONNECT, see https://curl.se/libcurl/c/libcurl-errors.html + // for details. + // // We just check the prefix of description, including `method`, `url`, `code` and `desc`. // The `msg` differ in various systems, such as: // * msg="Failed to connect to 127.0.0.1 port 20000: Connection refused" // * msg="Failed to connect to 127.0.0.1 port 20000 after 0 ms: Connection refused" // Thus we don't check if `msg` fields are consistent. const std::string expected_description_prefix( - "ERR_CURL_CONNECT_FAILED: failed to perform http request(" + "ERR_CURL_FAILED: failed to perform http request(" "method=GET, url=http://127.0.0.1:20000/test/get): code=7, " "desc=\"Couldn't connect to server\""); - check_expected_description_prefix(expected_description_prefix, err); + NO_FATALS(check_expected_description_prefix(expected_description_prefix, err)); } TEST(HttpClientTest, Callback) @@ -80,7 +84,7 @@ TEST(HttpClientTest, Callback) auto callback = [](const void *, size_t) { return false; }; const auto &err = client.exec_method(callback); - ASSERT_EQ(dsn::ERR_CURL_WRITE_ERROR, err.code()); + ASSERT_EQ(dsn::ERR_CURL_FAILED, err.code()); long actual_http_status; ASSERT_TRUE(client.get_http_status(actual_http_status)); @@ -88,16 +92,19 @@ TEST(HttpClientTest, Callback) std::cout << "failed for callback: "; + // "code=23" means CURLE_WRITE_ERROR, see https://curl.se/libcurl/c/libcurl-errors.html + // for details. + // // We just check the prefix of description, including `method`, `url`, `code` and `desc`. // The `msg` differ in various systems, such as: // * msg="Failed writing body (18446744073709551615 != 24)" // * msg="Failure writing output to destination" // Thus we don't check if `msg` fields are consistent. const auto expected_description_prefix = - fmt::format("ERR_CURL_WRITE_ERROR: failed to perform http request(" + fmt::format("ERR_CURL_FAILED: failed to perform http request(" "method=GET, url=http://127.0.0.1:20001/test/get): code=23, " "desc=\"Failed writing received data to disk/application\""); - check_expected_description_prefix(expected_description_prefix, err); + NO_FATALS(check_expected_description_prefix(expected_description_prefix, err)); } using http_client_method_case = diff --git a/src/utils/error_code.h b/src/utils/error_code.h index 97255bf5a1..04df97947a 100644 --- a/src/utils/error_code.h +++ b/src/utils/error_code.h @@ -181,8 +181,6 @@ DEFINE_ERR_CODE(ERR_RDB_CORRUPTION) DEFINE_ERR_CODE(ERR_DISK_IO_ERROR) -DEFINE_ERR_CODE(ERR_CURL_CONNECT_FAILED) -DEFINE_ERR_CODE(ERR_CURL_WRITE_ERROR) DEFINE_ERR_CODE(ERR_CURL_FAILED) } // namespace dsn diff --git a/src/utils/errors.h b/src/utils/errors.h index b5dec795d7..ce36befca9 100644 --- a/src/utils/errors.h +++ b/src/utils/errors.h @@ -229,7 +229,7 @@ USER_DEFINED_STRUCTURE_FORMATTER(::dsn::error_s); #define RETURN_NOT_OK(s) \ do { \ const ::dsn::error_s &_s = (s); \ - if (dsn_unlikely(!_s.is_ok())) { \ + if (dsn_unlikely(!_s)) { \ return _s; \ } \ } while (false); From 53dff8412c3da03af29374c37354cbf036557a92 Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Thu, 28 Sep 2023 11:58:38 +0800 Subject: [PATCH 29/29] feat(http): implement http client based on libcurl --- thirdparty/CMakeLists.txt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index 2af9d43356..f6fd11d4ba 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -297,18 +297,17 @@ set(CURL_OPTIONS --without-libssh2 --without-ssl --without-libidn - --without-zstd) + ) if (APPLE) set(CURL_OPTIONS ${CURL_OPTIONS} --without-nghttp2 - --without-libidn2 - --without-brotli) + ) endif () ExternalProject_Add(curl - URL ${OSS_URL_PREFIX}/curl-8.3.0.tar.gz - http://curl.haxx.se/download/curl-8.3.0.tar.gz - URL_MD5 7e614827c4165bf30f9770577259561e + URL ${OSS_URL_PREFIX}/curl-7.47.0.tar.gz + http://curl.haxx.se/download/curl-7.47.0.tar.gz + URL_MD5 5109d1232d208dfd712c0272b8360393 CONFIGURE_COMMAND ./configure --prefix=${TP_OUTPUT} ${CURL_OPTIONS} BUILD_IN_SOURCE 1