This is an automated email from the ASF dual-hosted git repository. szaszm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
commit 67b63be7263955d0a3850b56fee970282fec6e24 Author: Ferenc Gerlits <[email protected]> AuthorDate: Mon Oct 2 13:55:29 2023 +0200 MINIFICPP-2207 HttpRequestMethod should be an enum There is a well-defined list of possible HTTP request methods; we should not accept arbitrary strings. Closes #1657 Signed-off-by: Marton Szasz <[email protected]> --- .clang-tidy | 1 - extensions/civetweb/tests/ListenHTTPTests.cpp | 125 +++++++++++---------- extensions/elasticsearch/PostElasticsearch.cpp | 2 +- extensions/http-curl/client/HTTPClient.cpp | 62 +++++----- extensions/http-curl/client/HTTPClient.h | 7 +- extensions/http-curl/processors/InvokeHTTP.cpp | 10 +- extensions/http-curl/processors/InvokeHTTP.h | 10 +- extensions/http-curl/protocols/RESTSender.cpp | 8 +- extensions/http-curl/protocols/RESTSender.h | 2 +- extensions/http-curl/sitetosite/HTTPProtocol.cpp | 11 +- extensions/http-curl/sitetosite/HTTPProtocol.h | 2 +- .../http-curl/tests/unit/HTTPClientTests.cpp | 17 +-- extensions/splunk/SplunkHECProcessor.cpp | 2 +- libminifi/include/utils/BaseHTTPClient.h | 10 +- libminifi/src/RemoteProcessorGroupPort.cpp | 11 +- libminifi/src/core/logging/alert/AlertSink.cpp | 2 +- libminifi/src/utils/BaseHTTPClient.cpp | 2 +- 17 files changed, 148 insertions(+), 136 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 2345a2425..ac7598997 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -55,7 +55,6 @@ Checks: > bugprone-terminating-continue, bugprone-throw-keyword-missing, bugprone-too-small-loop-variable, - bugprone-unchecked-optional-access, bugprone-undefined-memory-manipulation, bugprone-undelegated-constructor, bugprone-unhandled-exception-at-new, diff --git a/extensions/civetweb/tests/ListenHTTPTests.cpp b/extensions/civetweb/tests/ListenHTTPTests.cpp index f8a51abe9..d64e2bbd5 100644 --- a/extensions/civetweb/tests/ListenHTTPTests.cpp +++ b/extensions/civetweb/tests/ListenHTTPTests.cpp @@ -41,6 +41,7 @@ namespace org::apache::nifi::minifi::test { using namespace std::literals::chrono_literals; +using HttpRequestMethod = org::apache::nifi::minifi::utils::HttpRequestMethod; class ListenHTTPTestsFixture { public: @@ -154,7 +155,7 @@ class ListenHTTPTestsFixture { for (const auto &header : headers) { client->setRequestHeader(header.first, header.second); } - if (method == "POST") { + if (method == HttpRequestMethod::POST) { client->setPostFields(payload); } } @@ -173,7 +174,7 @@ class ListenHTTPTestsFixture { } void check_response_body() { - if (method != "GET" && method != "POST") { + if (method != HttpRequestMethod::GET && method != HttpRequestMethod::POST) { return; } @@ -212,7 +213,7 @@ class ListenHTTPTestsFixture { plan->runCurrentProcessor(); // ListenHTTP plan->runNextProcessor(); // LogAttribute - if (expected_commited_requests > 0 && (method == "GET" || method == "POST")) { + if (expected_commited_requests > 0 && (method == HttpRequestMethod::GET || method == HttpRequestMethod::POST)) { REQUIRE(LogTestController::getInstance().contains("Size:" + std::to_string(payload.size()) + " Offset:0")); } REQUIRE(LogTestController::getInstance().contains("Logged " + std::to_string(expected_commited_requests) + " flow files")); @@ -228,7 +229,7 @@ class ListenHTTPTestsFixture { std::shared_ptr<core::Processor> log_attribute; std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service; - std::string method = "GET"; + HttpRequestMethod method = HttpRequestMethod::GET; std::map<std::string, std::string> headers; std::string payload; std::string endpoint = "test"; @@ -253,7 +254,7 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP GET", "[basic]") { TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP POST", "[basic]") { run_server(); - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; test_connect(); @@ -262,7 +263,7 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP POST", "[basic]") { TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP PUT", "[basic]") { run_server(); - method = "PUT"; + method = HttpRequestMethod::PUT; test_connect({HttpResponseExpectations{true, 405}}, 0); } @@ -270,7 +271,7 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP PUT", "[basic]") { TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP DELETE", "[basic]") { run_server(); - method = "DELETE"; + method = HttpRequestMethod::DELETE; test_connect({HttpResponseExpectations{true, 405}}, 0); } @@ -278,7 +279,7 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP DELETE", "[basic]") { TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP HEAD", "[basic]") { run_server(); - method = "HEAD"; + method = HttpRequestMethod::HEAD; test_connect({HttpResponseExpectations{}}, 0); } @@ -288,18 +289,18 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP no body", "[basic]") { SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } run_server(); - const std::size_t expected_commited_requests = (method == "POST" || method == "GET") ? 1 : 0; + const std::size_t expected_commited_requests = (method == HttpRequestMethod::POST || method == HttpRequestMethod::GET) ? 1 : 0; test_connect({HttpResponseExpectations{}}, expected_commited_requests); } @@ -307,18 +308,18 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP body with different mime type", " plan->setDynamicProperty(update_attribute, "mime.type", "text/plain"); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } run_server(); - const std::size_t expected_commited_requests = (method == "POST" || method == "GET") ? 1 : 0; + const std::size_t expected_commited_requests = (method == HttpRequestMethod::POST || method == HttpRequestMethod::GET) ? 1 : 0; test_connect({HttpResponseExpectations{}}, expected_commited_requests); } @@ -329,10 +330,10 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP all headers", "[basic][headers]") {"bar", "2"}}; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } @@ -350,10 +351,10 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP filtered headers", "[headers]") { {"bar", "2"}}; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } @@ -382,10 +383,10 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP Batch tests", "[batch]") { expected_processed_request_count = 5; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } } @@ -396,10 +397,10 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP Batch tests", "[batch]") { expected_processed_request_count = 4; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } } @@ -411,10 +412,10 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTP Batch tests", "[batch]") { expected_processed_request_count = 3; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } } @@ -430,18 +431,18 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS without CA", "[basic][https]") { create_ssl_context_service("goodCA.crt", nullptr); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } run_server(); - const std::size_t expected_commited_requests = (method == "POST" || method == "GET") ? 1 : 0; + const std::size_t expected_commited_requests = (method == HttpRequestMethod::POST || method == HttpRequestMethod::GET) ? 1 : 0; test_connect({HttpResponseExpectations{}}, expected_commited_requests); } @@ -453,18 +454,18 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS without client cert", "[basic][h create_ssl_context_service("goodCA.crt", nullptr); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } run_server(); - const std::size_t expected_commited_requests = (method == "POST" || method == "GET") ? 1 : 0; + const std::size_t expected_commited_requests = (method == HttpRequestMethod::POST || method == HttpRequestMethod::GET) ? 1 : 0; test_connect({HttpResponseExpectations{}}, expected_commited_requests); } @@ -477,18 +478,18 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS with client cert from good CA", create_ssl_context_service("goodCA.crt", "goodCA_goodClient.pem"); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } run_server(); - const std::size_t expected_commited_requests = (method == "POST" || method == "GET") ? 1 : 0; + const std::size_t expected_commited_requests = (method == HttpRequestMethod::POST || method == HttpRequestMethod::GET) ? 1 : 0; test_connect({HttpResponseExpectations{}}, expected_commited_requests); } @@ -501,18 +502,18 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS with PKCS12 client cert from goo create_ssl_context_service("goodCA.crt", "goodCA_goodClient.p12"); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } run_server(); - const std::size_t expected_commited_requests = (method == "POST" || method == "GET") ? 1 : 0; + const std::size_t expected_commited_requests = (method == HttpRequestMethod::POST || method == HttpRequestMethod::GET) ? 1 : 0; test_connect({HttpResponseExpectations{}}, expected_commited_requests); } @@ -529,14 +530,14 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS with client cert from bad CA", " plan->setProperty(listen_http, minifi::processors::ListenHTTP::SSLVerifyPeer, "yes"); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } } SECTION("do not verify peer") { @@ -544,16 +545,16 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS with client cert from bad CA", " response_code = 200; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; expected_commited_requests = 1; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; expected_commited_requests = 1; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } } @@ -573,18 +574,18 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS with client cert with matching D create_ssl_context_service("goodCA.crt", "goodCA_goodClient.pem"); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } run_server(); - const std::size_t expected_commited_requests = (method == "POST" || method == "GET") ? 1 : 0; + const std::size_t expected_commited_requests = (method == HttpRequestMethod::POST || method == HttpRequestMethod::GET) ? 1 : 0; test_connect({HttpResponseExpectations{}}, expected_commited_requests); } @@ -601,30 +602,30 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS with client cert with non-matchi response_code = 403; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } } SECTION("do not verify peer") { response_code = 200; SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; expected_commited_requests = 1; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; expected_commited_requests = 1; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } } @@ -641,14 +642,14 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS minimum SSL version", "[https]") plan->setProperty(listen_http, minifi::processors::ListenHTTP::SSLCertificateAuthority, (executable_dir / "resources" / "goodCA.crt").string()); SECTION("GET") { - method = "GET"; + method = HttpRequestMethod::GET; } SECTION("POST") { - method = "POST"; + method = HttpRequestMethod::POST; payload = "Test payload"; } SECTION("HEAD") { - method = "HEAD"; + method = HttpRequestMethod::HEAD; } create_ssl_context_service("goodCA.crt", "goodCA_goodClient.pem"); @@ -658,7 +659,7 @@ TEST_CASE_METHOD(ListenHTTPTestsFixture, "HTTPS minimum SSL version", "[https]") client = std::make_unique<minifi::extensions::curl::HTTPClient>(); client->setVerbose(false); client->initialize(method, url, ssl_context_service); - if (method == "POST") { + if (method == HttpRequestMethod::POST) { client->setPostFields(payload); } REQUIRE(client->setSpecificSSLVersion(minifi::utils::SSLVersion::TLSv1_1)); diff --git a/extensions/elasticsearch/PostElasticsearch.cpp b/extensions/elasticsearch/PostElasticsearch.cpp index f7bf8bdb7..a0f0f3740 100644 --- a/extensions/elasticsearch/PostElasticsearch.cpp +++ b/extensions/elasticsearch/PostElasticsearch.cpp @@ -71,7 +71,7 @@ void PostElasticsearch::onSchedule(const std::shared_ptr<core::ProcessContext>& if (!credentials_service_) throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Missing Elasticsearch credentials service"); - client_.initialize("POST", host_url_ + "/_bulk", getSSLContextService(*context)); + client_.initialize(utils::HttpRequestMethod::POST, host_url_ + "/_bulk", getSSLContextService(*context)); client_.setContentType("application/json"); credentials_service_->authenticateClient(client_); } diff --git a/extensions/http-curl/client/HTTPClient.cpp b/extensions/http-curl/client/HTTPClient.cpp index 8d73f62a4..d9374818a 100644 --- a/extensions/http-curl/client/HTTPClient.cpp +++ b/extensions/http-curl/client/HTTPClient.cpp @@ -22,13 +22,14 @@ #include <utility> #include <vector> -#include "utils/gsl.h" -#include "utils/StringUtils.h" #include "core/Resource.h" +#include "magic_enum.hpp" #include "range/v3/algorithm/all_of.hpp" #include "range/v3/action/transform.hpp" +#include "utils/gsl.h" #include "utils/HTTPUtils.h" #include "utils/Literals.h" +#include "utils/StringUtils.h" using namespace std::literals::chrono_literals; @@ -109,8 +110,8 @@ bool isSecure(const std::string& url) { } } // namespace -void HTTPClient::initialize(std::string method, std::string url, std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service) { - set_request_method(std::move(method)); +void HTTPClient::initialize(utils::HttpRequestMethod method, std::string url, std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service) { + set_request_method(method); if (ssl_context_service) { ssl_context_service_ = std::move(ssl_context_service); } @@ -272,7 +273,7 @@ void HTTPClient::setReadCallback(std::unique_ptr<utils::HTTPReadCallback> callba void HTTPClient::setUploadCallback(std::unique_ptr<utils::HTTPUploadCallback> callback) { logger_->log_debug("Setting callback for %s", url_); write_callback_ = std::move(callback); - if (method_ == "PUT") { + if (method_ == utils::HttpRequestMethod::PUT) { curl_easy_setopt(http_session_.get(), CURLOPT_INFILESIZE_LARGE, (curl_off_t) write_callback_->size()); } curl_easy_setopt(http_session_.get(), CURLOPT_READFUNCTION, &utils::HTTPRequestResponse::send_write); @@ -344,6 +345,10 @@ bool HTTPClient::submit() { logger_->log_error("Tried to submit to an empty url"); return false; } + if (!method_) { + logger_->log_error("Tried to use HTTPClient without setting an HTTP method"); + return false; + } response_data_.clear(); @@ -420,29 +425,34 @@ const std::vector<char> &HTTPClient::getResponseBody() { return response_data_.response_body; } -void HTTPClient::set_request_method(std::string method) { - ranges::actions::transform(method, [](auto ch) { return ::toupper(static_cast<unsigned char>(ch)); }); +void HTTPClient::set_request_method(utils::HttpRequestMethod method) { if (method_ == method) return; - method_ = std::move(method); - if (method_ == "POST") { - curl_easy_setopt(http_session_.get(), CURLOPT_POST, 1L); - curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); - } else if (method_ == "HEAD") { - curl_easy_setopt(http_session_.get(), CURLOPT_NOBODY, 1L); - curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); - } else if (method_ == "GET") { - curl_easy_setopt(http_session_.get(), CURLOPT_HTTPGET, 1L); - curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); - } else if (method_ == "PUT") { - curl_easy_setopt(http_session_.get(), CURLOPT_UPLOAD, 1L); - curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); - } else { - curl_easy_setopt(http_session_.get(), CURLOPT_POST, 0L); - curl_easy_setopt(http_session_.get(), CURLOPT_NOBODY, 0L); - curl_easy_setopt(http_session_.get(), CURLOPT_HTTPGET, 0L); - curl_easy_setopt(http_session_.get(), CURLOPT_UPLOAD, 0L); - curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, method_.c_str()); + method_ = method; + switch (*method_) { + case utils::HttpRequestMethod::POST: + curl_easy_setopt(http_session_.get(), CURLOPT_POST, 1L); + curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); + break; + case utils::HttpRequestMethod::HEAD: + curl_easy_setopt(http_session_.get(), CURLOPT_NOBODY, 1L); + curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); + break; + case utils::HttpRequestMethod::GET: + curl_easy_setopt(http_session_.get(), CURLOPT_HTTPGET, 1L); + curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); + break; + case utils::HttpRequestMethod::PUT: + curl_easy_setopt(http_session_.get(), CURLOPT_UPLOAD, 1L); + curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, nullptr); + break; + default: + curl_easy_setopt(http_session_.get(), CURLOPT_POST, 0L); + curl_easy_setopt(http_session_.get(), CURLOPT_NOBODY, 0L); + curl_easy_setopt(http_session_.get(), CURLOPT_HTTPGET, 0L); + curl_easy_setopt(http_session_.get(), CURLOPT_UPLOAD, 0L); + curl_easy_setopt(http_session_.get(), CURLOPT_CUSTOMREQUEST, std::string(magic_enum::enum_name(*method_)).c_str()); + break; } } diff --git a/extensions/http-curl/client/HTTPClient.h b/extensions/http-curl/client/HTTPClient.h index 144edb305..3bc893ede 100644 --- a/extensions/http-curl/client/HTTPClient.h +++ b/extensions/http-curl/client/HTTPClient.h @@ -32,6 +32,7 @@ #include <limits> #include <map> #include <memory> +#include <optional> #include <string> #include <string_view> #include <unordered_map> @@ -89,7 +90,7 @@ class HTTPClient : public utils::BaseHTTPClient, public core::Connectable { void forceClose(); - void initialize(std::string method, std::string url, std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service) override; + void initialize(utils::HttpRequestMethod method, std::string url, std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service) override; void setConnectionTimeout(std::chrono::milliseconds timeout) override; @@ -118,7 +119,7 @@ class HTTPClient : public utils::BaseHTTPClient, public core::Connectable { const std::vector<char>& getResponseBody() override; - void set_request_method(std::string method) override; + void set_request_method(utils::HttpRequestMethod method) override; void setPeerVerification(bool peer_verification) override; void setHostVerification(bool host_verification) override; @@ -227,7 +228,7 @@ class HTTPClient : public utils::BaseHTTPClient, public core::Connectable { std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service_; std::string url_; - std::string method_; + std::optional<utils::HttpRequestMethod> method_; std::chrono::milliseconds connect_timeout_{std::chrono::seconds(30)}; std::chrono::milliseconds read_timeout_{std::chrono::seconds(30)}; diff --git a/extensions/http-curl/processors/InvokeHTTP.cpp b/extensions/http-curl/processors/InvokeHTTP.cpp index 549b03add..84fa17667 100644 --- a/extensions/http-curl/processors/InvokeHTTP.cpp +++ b/extensions/http-curl/processors/InvokeHTTP.cpp @@ -93,7 +93,7 @@ void setupClientTransferEncoding(extensions::curl::HTTPClient& client, bool use_ } // namespace void InvokeHTTP::setupMembersFromProperties(const core::ProcessContext& context) { - method_ = (context.getProperty(Method) | utils::orElse([] { throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Method property missing or invalid"); })).value(); + method_ = utils::parseEnumProperty<utils::HttpRequestMethod>(context, Method); context.getProperty(SendMessageBody, send_message_body_); @@ -162,7 +162,7 @@ void InvokeHTTP::onSchedule(const std::shared_ptr<core::ProcessContext>& context } bool InvokeHTTP::shouldEmitFlowFile() const { - return ("POST" == method_ || "PUT" == method_ || "PATCH" == method_); + return (utils::HttpRequestMethod::POST == method_ || utils::HttpRequestMethod::PUT == method_ || utils::HttpRequestMethod::PATCH == method_); } /** @@ -205,10 +205,10 @@ void InvokeHTTP::onTrigger(const std::shared_ptr<core::ProcessContext>& context, if (flow_file == nullptr) { if (!shouldEmitFlowFile()) { - logger_->log_debug("InvokeHTTP -- create flow file with %s", method_); + logger_->log_debug("InvokeHTTP -- create flow file with %s", std::string(magic_enum::enum_name(method_))); flow_file = session->create(); } else { - logger_->log_debug("Exiting because method is %s and there is no flowfile available to execute it, yielding", method_); + logger_->log_debug("Exiting because method is %s and there is no flowfile available to execute it, yielding", std::string(magic_enum::enum_name(method_))); yield(); return; } @@ -223,7 +223,7 @@ void InvokeHTTP::onTrigger(const std::shared_ptr<core::ProcessContext>& context, void InvokeHTTP::onTriggerWithClient(const std::shared_ptr<core::ProcessContext>& context, const std::shared_ptr<core::ProcessSession>& session, const std::shared_ptr<core::FlowFile>& flow_file, minifi::extensions::curl::HTTPClient& client) { - logger_->log_debug("onTrigger InvokeHTTP with %s to %s", method_, client.getURL()); + logger_->log_debug("onTrigger InvokeHTTP with %s to %s", std::string(magic_enum::enum_name(method_)), client.getURL()); const auto remove_callback_from_client_at_exit = gsl::finally([&client] { client.setUploadCallback({}); diff --git a/extensions/http-curl/processors/InvokeHTTP.h b/extensions/http-curl/processors/InvokeHTTP.h index 89fd640c3..2f447d39c 100644 --- a/extensions/http-curl/processors/InvokeHTTP.h +++ b/extensions/http-curl/processors/InvokeHTTP.h @@ -67,10 +67,10 @@ class InvokeHTTP : public core::Processor { "The destination URL and HTTP Method are configurable. FlowFile attributes are converted to HTTP headers and the " "FlowFile contents are included as the body of the request (if the HTTP Method is PUT, POST or PATCH)."; - EXTENSIONAPI static constexpr auto Method = core::PropertyDefinitionBuilder<>::createProperty("HTTP Method") - .withDescription("HTTP request method (GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS). " - "Arbitrary methods are also supported. Methods other than POST, PUT and PATCH will be sent without a message body.") - .withDefaultValue("GET") + EXTENSIONAPI static constexpr auto Method = core::PropertyDefinitionBuilder<magic_enum::enum_count<utils::HttpRequestMethod>()>::createProperty("HTTP Method") + .withDescription("HTTP request method. Methods other than POST, PUT and PATCH will be sent without a message body.") + .withAllowedValues(magic_enum::enum_names<utils::HttpRequestMethod>()) + .withDefaultValue(magic_enum::enum_name(utils::HttpRequestMethod::GET)) .build(); EXTENSIONAPI static constexpr auto URL = core::PropertyDefinitionBuilder<>::createProperty("Remote URL") .withDescription("Remote URL which will be connected to, including scheme, host, port, path.") @@ -267,7 +267,7 @@ class InvokeHTTP : public core::Processor { void setupMembersFromProperties(const core::ProcessContext& context); std::unique_ptr<minifi::extensions::curl::HTTPClient> createHTTPClientFromPropertiesAndMembers(const core::ProcessContext& context) const; - std::string method_; + utils::HttpRequestMethod method_{}; std::optional<utils::Regex> attributes_to_send_; std::optional<std::string> put_response_body_in_attribute_; bool always_output_response_{false}; diff --git a/extensions/http-curl/protocols/RESTSender.cpp b/extensions/http-curl/protocols/RESTSender.cpp index 44575f734..db65ab0e7 100644 --- a/extensions/http-curl/protocols/RESTSender.cpp +++ b/extensions/http-curl/protocols/RESTSender.cpp @@ -93,7 +93,7 @@ C2Payload RESTSender::consumePayload(const C2Payload &payload, Direction directi void RESTSender::update(const std::shared_ptr<Configure> &) { } -void RESTSender::setSecurityContext(extensions::curl::HTTPClient &client, const std::string &type, const std::string &url) { +void RESTSender::setSecurityContext(extensions::curl::HTTPClient &client, utils::HttpRequestMethod type, const std::string &url) { // only use the SSL Context if we have a secure URL. auto generatedService = std::make_shared<minifi::controllers::SSLContextService>("Service", configuration_); generatedService->onEnable(); @@ -111,7 +111,7 @@ C2Payload RESTSender::sendPayload(const std::string& url, const Direction direct client.setKeepAliveProbe(extensions::curl::KeepAliveProbeData{2s, 2s}); client.setConnectionTimeout(2s); - auto setUpHttpRequest = [&](const std::string& http_method) { + auto setUpHttpRequest = [&](utils::HttpRequestMethod http_method) { client.set_request_method(http_method); if (url.find("https://";) == 0) { if (!ssl_context_service_) { @@ -122,7 +122,7 @@ C2Payload RESTSender::sendPayload(const std::string& url, const Direction direct } }; if (direction == Direction::TRANSMIT) { - setUpHttpRequest("POST"); + setUpHttpRequest(utils::HttpRequestMethod::POST); if (payload.getOperation() == Operation::transfer) { // treat nested payloads as files for (const auto& file : payload.getNestedPayloads()) { @@ -163,7 +163,7 @@ C2Payload RESTSender::sendPayload(const std::string& url, const Direction direct } else { // we do not need to set the upload callback // since we are not uploading anything on a get - setUpHttpRequest("GET"); + setUpHttpRequest(utils::HttpRequestMethod::GET); } if (payload.getOperation() == Operation::transfer) { diff --git a/extensions/http-curl/protocols/RESTSender.h b/extensions/http-curl/protocols/RESTSender.h index feb604bed..7172bdd8f 100644 --- a/extensions/http-curl/protocols/RESTSender.h +++ b/extensions/http-curl/protocols/RESTSender.h @@ -68,7 +68,7 @@ class RESTSender : public RESTProtocol, public C2Protocol { * @param type type of HTTP request * @param url HTTP url */ - void setSecurityContext(extensions::curl::HTTPClient &client, const std::string &type, const std::string &url); + void setSecurityContext(extensions::curl::HTTPClient &client, utils::HttpRequestMethod type, const std::string &url); std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service_; diff --git a/extensions/http-curl/sitetosite/HTTPProtocol.cpp b/extensions/http-curl/sitetosite/HTTPProtocol.cpp index 4e713bd45..0f938c0ef 100644 --- a/extensions/http-curl/sitetosite/HTTPProtocol.cpp +++ b/extensions/http-curl/sitetosite/HTTPProtocol.cpp @@ -16,7 +16,6 @@ */ #include "HTTPProtocol.h" -#include <cstdio> #include <chrono> #include <map> #include <string> @@ -48,7 +47,7 @@ std::shared_ptr<sitetosite::Transaction> HttpSiteToSiteClient::createTransaction std::string dir_str = direction == sitetosite::SEND ? "input-ports" : "output-ports"; std::stringstream uri; uri << getBaseURI() << "data-transfer/" << dir_str << "/" << getPortId().to_string() << "/transactions"; - auto client = create_http_client(uri.str(), "POST"); + auto client = create_http_client(uri.str(), utils::HttpRequestMethod::POST); client->setRequestHeader(PROTOCOL_VERSION_HEADER, "1"); client->setConnectionTimeout(std::chrono::milliseconds(5000)); client->setContentType("application/json"); @@ -180,7 +179,7 @@ bool HttpSiteToSiteClient::getPeerList(std::vector<sitetosite::PeerStatus> &peer std::stringstream uri; uri << getBaseURI() << "site-to-site/peers"; - auto client = create_http_client(uri.str(), "GET"); + auto client = create_http_client(uri.str(), utils::HttpRequestMethod::GET); client->setRequestHeader(PROTOCOL_VERSION_HEADER, "1"); @@ -197,7 +196,7 @@ bool HttpSiteToSiteClient::getPeerList(std::vector<sitetosite::PeerStatus> &peer std::shared_ptr<minifi::extensions::curl::HTTPClient> HttpSiteToSiteClient::openConnectionForSending(const std::shared_ptr<HttpTransaction> &transaction) { std::stringstream uri; uri << transaction->getTransactionUrl() << "/flow-files"; - std::shared_ptr<minifi::extensions::curl::HTTPClient> client = create_http_client(uri.str(), "POST"); + std::shared_ptr<minifi::extensions::curl::HTTPClient> client = create_http_client(uri.str(), utils::HttpRequestMethod::POST); client->setContentType("application/octet-stream"); client->setRequestHeader("Accept", "text/plain"); client->setRequestHeader("Transfer-Encoding", "chunked"); @@ -207,7 +206,7 @@ std::shared_ptr<minifi::extensions::curl::HTTPClient> HttpSiteToSiteClient::open std::shared_ptr<minifi::extensions::curl::HTTPClient> HttpSiteToSiteClient::openConnectionForReceive(const std::shared_ptr<HttpTransaction> &transaction) { std::stringstream uri; uri << transaction->getTransactionUrl() << "/flow-files"; - std::shared_ptr<minifi::extensions::curl::HTTPClient> client = create_http_client(uri.str(), "GET"); + std::shared_ptr<minifi::extensions::curl::HTTPClient> client = create_http_client(uri.str(), utils::HttpRequestMethod::GET); return client; } @@ -270,7 +269,7 @@ void HttpSiteToSiteClient::closeTransaction(const utils::Identifier &transaction uri << "&checksum=" << transaction->getCRC(); } - auto client = create_http_client(uri.str(), "DELETE"); + auto client = create_http_client(uri.str(), utils::HttpRequestMethod::DELETE); client->setRequestHeader(PROTOCOL_VERSION_HEADER, "1"); diff --git a/extensions/http-curl/sitetosite/HTTPProtocol.h b/extensions/http-curl/sitetosite/HTTPProtocol.h index 0e74d70e9..4cd008564 100644 --- a/extensions/http-curl/sitetosite/HTTPProtocol.h +++ b/extensions/http-curl/sitetosite/HTTPProtocol.h @@ -127,7 +127,7 @@ class HttpSiteToSiteClient : public sitetosite::SiteToSiteClient { static std::optional<utils::Identifier> parseTransactionId(const std::string &uri); - std::unique_ptr<minifi::extensions::curl::HTTPClient> create_http_client(const std::string &uri, const std::string &method = "POST", bool setPropertyHeaders = false) { + std::unique_ptr<minifi::extensions::curl::HTTPClient> create_http_client(const std::string &uri, utils::HttpRequestMethod method = utils::HttpRequestMethod::POST, bool setPropertyHeaders = false) { std::unique_ptr<minifi::extensions::curl::HTTPClient> http_client_ = std::make_unique<minifi::extensions::curl::HTTPClient>(uri, ssl_context_service_); http_client_->initialize(method, uri, ssl_context_service_); if (setPropertyHeaders) { diff --git a/extensions/http-curl/tests/unit/HTTPClientTests.cpp b/extensions/http-curl/tests/unit/HTTPClientTests.cpp index 4810cc7ef..5d4695787 100644 --- a/extensions/http-curl/tests/unit/HTTPClientTests.cpp +++ b/extensions/http-curl/tests/unit/HTTPClientTests.cpp @@ -24,6 +24,7 @@ #include "client/HTTPClient.h" #include "CivetServer.h" #include "ConnectionCountingServer.h" +#include "magic_enum.hpp" #include "utils/BaseHTTPClient.h" using namespace std::literals::chrono_literals; @@ -55,7 +56,7 @@ TEST_CASE("HTTPClientTestChunkedResponse", "[basic]") { bool handlePost(CivetServer* /*server*/, struct mg_connection* conn) override { mg_printf(conn, "HTTP/1.1 100 Continue\r\n\r\n"); - std::array<uint8_t, 16384U> buf; + std::array<uint8_t, 16384U> buf{}; while (mg_read(conn, buf.data(), buf.size()) > 0) {} send_response(conn); @@ -81,7 +82,7 @@ TEST_CASE("HTTPClientTestChunkedResponse", "[basic]") { const std::string port = std::to_string(vec.at(0)); HTTPClient client; - client.initialize("GET", "http://localhost:"; + port + "/testytesttest", nullptr); + client.initialize(utils::HttpRequestMethod::GET, "http://localhost:"; + port + "/testytesttest", nullptr); REQUIRE(client.submit()); @@ -125,13 +126,15 @@ TEST_CASE("HTTPClient should be reusable", "[basic]") { HTTPClient client; client.setKeepAliveProbe(KeepAliveProbeData{2s, 2s}); - client.initialize("GET", "http://localhost:"; + keep_alive_server_.getPort() + "/method", nullptr); + client.initialize(utils::HttpRequestMethod::GET, "http://localhost:"; + keep_alive_server_.getPort() + "/method", nullptr); - std::vector<std::string> methods = {"GET", "GET", "PUT", "GET", "GET", "POST", "POST", "PUT"}; + std::vector<utils::HttpRequestMethod> methods = { + utils::HttpRequestMethod::GET, utils::HttpRequestMethod::GET, utils::HttpRequestMethod::PUT, utils::HttpRequestMethod::GET, + utils::HttpRequestMethod::GET, utils::HttpRequestMethod::POST, utils::HttpRequestMethod::POST, utils::HttpRequestMethod::PUT }; uint64_t request_number = 0; for (const auto& method: methods) { client.set_request_method(method); - if (method != "GET") { + if (method != utils::HttpRequestMethod::GET) { auto callback = std::make_unique<org::apache::nifi::minifi::utils::HTTPUploadByteArrayInputCallback>(); std::string content = R"({ "content": "a" })"; callback->write(content); @@ -148,7 +151,7 @@ TEST_CASE("HTTPClient should be reusable", "[basic]") { REQUIRE(headers.contains("Response-number")); CHECK(std::to_string(request_number) == headers.at("Response-number")); const auto& response = client.getResponseBody(); - CHECK(method + std::to_string(request_number) == std::string(response.begin(), response.end())); + CHECK(std::string(magic_enum::enum_name(method)) + std::to_string(request_number) == std::string(response.begin(), response.end())); ++request_number; } @@ -159,7 +162,7 @@ TEST_CASE("HTTPClient should be reusable", "[basic]") { #ifdef __linux__ TEST_CASE("SSL without SSLContextService", "[HTTPClient]") { HTTPClient client; - client.initialize("GET", "https://apache.org";, nullptr); + client.initialize(utils::HttpRequestMethod::GET, "https://apache.org";, nullptr); REQUIRE(client.submit()); } #endif diff --git a/extensions/splunk/SplunkHECProcessor.cpp b/extensions/splunk/SplunkHECProcessor.cpp index 1fdc5fdeb..14dca9c45 100644 --- a/extensions/splunk/SplunkHECProcessor.cpp +++ b/extensions/splunk/SplunkHECProcessor.cpp @@ -53,7 +53,7 @@ std::shared_ptr<minifi::controllers::SSLContextService> SplunkHECProcessor::getS } void SplunkHECProcessor::initializeClient(curl::HTTPClient& client, const std::string &url, std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service) const { - client.initialize("POST", url, std::move(ssl_context_service)); + client.initialize(utils::HttpRequestMethod::POST, url, std::move(ssl_context_service)); client.setRequestHeader("Authorization", token_); client.setRequestHeader("X-Splunk-Request-Channel", request_channel_); } diff --git a/libminifi/include/utils/BaseHTTPClient.h b/libminifi/include/utils/BaseHTTPClient.h index d145a7c59..88015f232 100644 --- a/libminifi/include/utils/BaseHTTPClient.h +++ b/libminifi/include/utils/BaseHTTPClient.h @@ -178,6 +178,12 @@ namespace HTTPRequestResponse { int seek_callback(void *p, int64_t offset, int); } +#undef DELETE // this is a macro in winnt.h + +enum class HttpRequestMethod { + GET, POST, PUT, PATCH, DELETE, CONNECT, HEAD, OPTIONS, TRACE +}; + class BaseHTTPClient { public: BaseHTTPClient() = default; @@ -186,7 +192,7 @@ class BaseHTTPClient { virtual void setVerbose(bool use_stderr) = 0; - virtual void initialize(std::string method, std::string url, std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service) = 0; + virtual void initialize(HttpRequestMethod method, std::string url, std::shared_ptr<minifi::controllers::SSLContextService> ssl_context_service) = 0; virtual void setConnectionTimeout(std::chrono::milliseconds timeout) = 0; @@ -208,7 +214,7 @@ class BaseHTTPClient { virtual void setRequestHeader(std::string key, std::optional<std::string> value) = 0; - virtual void set_request_method(std::string method) = 0; + virtual void set_request_method(HttpRequestMethod method) = 0; virtual void setPeerVerification(bool peer_verification) = 0; virtual void setHostVerification(bool host_verification) = 0; diff --git a/libminifi/src/RemoteProcessorGroupPort.cpp b/libminifi/src/RemoteProcessorGroupPort.cpp index b5c1be25a..6893dc6be 100644 --- a/libminifi/src/RemoteProcessorGroupPort.cpp +++ b/libminifi/src/RemoteProcessorGroupPort.cpp @@ -20,15 +20,10 @@ #include "RemoteProcessorGroupPort.h" -#include <algorithm> -#include <cstdint> #include <memory> -#include <deque> #include <iostream> -#include <set> #include <vector> #include <string> -#include <type_traits> #include <utility> #include <cinttypes> @@ -41,9 +36,7 @@ #include "core/logging/Logger.h" #include "core/ProcessContext.h" #include "core/ProcessorNode.h" -#include "core/Relationship.h" #include "utils/BaseHTTPClient.h" -#include "utils/net/DNS.h" #undef GetObject // windows.h #defines GetObject = GetObjectA or GetObjectW, which conflicts with rapidjson @@ -289,7 +282,7 @@ std::pair<std::string, int> RemoteProcessorGroupPort::refreshRemoteSite2SiteInfo return std::make_pair("", -1); } client = std::unique_ptr<utils::BaseHTTPClient>(dynamic_cast<utils::BaseHTTPClient*>(client_ptr)); - client->initialize("GET", loginUrl.str(), ssl_service); + client->initialize(utils::HttpRequestMethod::GET, loginUrl.str(), ssl_service); // use a connection timeout. if this times out we will simply attempt re-connection // so no need for configuration parameter that isn't already defined in Processor client->setConnectionTimeout(10s); @@ -308,7 +301,7 @@ std::pair<std::string, int> RemoteProcessorGroupPort::refreshRemoteSite2SiteInfo } int siteTosite_port_ = -1; client = std::unique_ptr<utils::BaseHTTPClient>(dynamic_cast<utils::BaseHTTPClient*>(client_ptr)); - client->initialize("GET", fullUrl.str(), ssl_service); + client->initialize(utils::HttpRequestMethod::GET, fullUrl.str(), ssl_service); // use a connection timeout. if this times out we will simply attempt re-connection // so no need for configuration parameter that isn't already defined in Processor client->setConnectionTimeout(10s); diff --git a/libminifi/src/core/logging/alert/AlertSink.cpp b/libminifi/src/core/logging/alert/AlertSink.cpp index b40ebefb9..6c71f591a 100644 --- a/libminifi/src/core/logging/alert/AlertSink.cpp +++ b/libminifi/src/core/logging/alert/AlertSink.cpp @@ -199,7 +199,7 @@ void AlertSink::send(Services& services) { logger_->log_error("Could not instantiate a HTTPClient object"); return; } - client->initialize("PUT", config_.url, services.ssl_service); + client->initialize(utils::HttpRequestMethod::PUT, config_.url, services.ssl_service); rapidjson::Document doc(rapidjson::kObjectType); std::string agent_id = services.agent_id->getAgentIdentifier(); diff --git a/libminifi/src/utils/BaseHTTPClient.cpp b/libminifi/src/utils/BaseHTTPClient.cpp index 7f802efb5..ab2f9eb8c 100644 --- a/libminifi/src/utils/BaseHTTPClient.cpp +++ b/libminifi/src/utils/BaseHTTPClient.cpp @@ -65,7 +65,7 @@ std::string get_token(utils::BaseHTTPClient* client, const std::string& username client->setContentType("application/x-www-form-urlencoded"); - client->set_request_method("POST"); + client->set_request_method(HttpRequestMethod::POST); std::string payload = "username=" + username + "&" + "password=" + password;
