You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by ab...@apache.org on 2020/06/29 17:02:36 UTC
[nifi-minifi-cpp] branch master updated: MINIFICPP-1261 - Refactor
non-trivial usages of ScopeGuard class
This is an automated email from the ASF dual-hosted git repository.
aboda pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
The following commit(s) were added to refs/heads/master by this push:
new 852bb9d MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
852bb9d is described below
commit 852bb9d69cf12e6fba27cff136a4a44109327b7b
Author: Adam Hunyadi <hu...@gmail.com>
AuthorDate: Tue Jun 16 10:55:24 2020 +0200
MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
Signed-off-by: Arpad Boda <ab...@apache.org>
This closes #824
---
extensions/rocksdb-repos/FlowFileRepository.cpp | 17 ++++++-------
libminifi/include/io/tls/TLSSocket.h | 11 ++++-----
libminifi/include/utils/ScopeGuard.h | 3 ++-
libminifi/src/controllers/SSLContextService.cpp | 24 +++++++++---------
libminifi/src/io/tls/TLSSocket.cpp | 33 +++++++++++--------------
5 files changed, 41 insertions(+), 47 deletions(-)
diff --git a/extensions/rocksdb-repos/FlowFileRepository.cpp b/extensions/rocksdb-repos/FlowFileRepository.cpp
index f775703..428db97 100644
--- a/extensions/rocksdb-repos/FlowFileRepository.cpp
+++ b/extensions/rocksdb-repos/FlowFileRepository.cpp
@@ -122,27 +122,26 @@ void FlowFileRepository::run() {
}
void FlowFileRepository::prune_stored_flowfiles() {
- rocksdb::DB* stored_database_ = nullptr;
- utils::ScopeGuard db_guard([&stored_database_]() {
- delete stored_database_;
- });
+ rocksdb::DB* used_database;
+ std::unique_ptr<rocksdb::DB> stored_database;
bool corrupt_checkpoint = false;
if (nullptr != checkpoint_) {
rocksdb::Options options;
options.create_if_missing = true;
options.use_direct_io_for_flush_and_compaction = true;
options.use_direct_reads = true;
- rocksdb::Status status = rocksdb::DB::OpenForReadOnly(options, FLOWFILE_CHECKPOINT_DIRECTORY, &stored_database_);
- if (!status.ok()) {
- stored_database_ = db_;
- db_guard.disable();
+ rocksdb::Status status = rocksdb::DB::OpenForReadOnly(options, FLOWFILE_CHECKPOINT_DIRECTORY, &used_database);
+ if (status.ok()) {
+ stored_database.reset(used_database);
+ } else {
+ used_database = db_;
}
} else {
logger_->log_trace("Could not open checkpoint as object doesn't exist. Likely not needed or file system error.");
return;
}
- rocksdb::Iterator* it = stored_database_->NewIterator(rocksdb::ReadOptions());
+ rocksdb::Iterator* it = used_database->NewIterator(rocksdb::ReadOptions());
for (it->SeekToFirst(); it->Valid(); it->Next()) {
std::shared_ptr<FlowFileRecord> eventRead = std::make_shared<FlowFileRecord>(shared_from_this(), content_repo_);
std::string key = it->key().ToString();
diff --git a/libminifi/include/io/tls/TLSSocket.h b/libminifi/include/io/tls/TLSSocket.h
index e1acb11..8b4bd5c 100644
--- a/libminifi/include/io/tls/TLSSocket.h
+++ b/libminifi/include/io/tls/TLSSocket.h
@@ -64,13 +64,10 @@ class TLSContext : public SocketContext {
public:
TLSContext(const std::shared_ptr<Configure> &configure, std::shared_ptr<minifi::controllers::SSLContextService> ssl_service = nullptr); // NOLINT
- virtual ~TLSContext() {
- if (nullptr != ctx)
- SSL_CTX_free(ctx);
- }
+ virtual ~TLSContext() = default;
SSL_CTX *getContext() {
- return ctx;
+ return ctx.get();
}
int16_t getError() {
@@ -80,10 +77,12 @@ class TLSContext : public SocketContext {
int16_t initialize(bool server_method = false);
private:
+ static void deleteContext(SSL_CTX* ptr) { SSL_CTX_free(ptr); }
+
std::shared_ptr<logging::Logger> logger_;
std::shared_ptr<Configure> configure_;
std::shared_ptr<minifi::controllers::SSLContextService> ssl_service_;
- SSL_CTX *ctx;
+ std::unique_ptr<SSL_CTX, decltype(&deleteContext)> ctx;
int16_t error_value;
};
diff --git a/libminifi/include/utils/ScopeGuard.h b/libminifi/include/utils/ScopeGuard.h
index cc20288..01fa314 100644
--- a/libminifi/include/utils/ScopeGuard.h
+++ b/libminifi/include/utils/ScopeGuard.h
@@ -24,6 +24,7 @@
#include <functional>
#include "gsl.h"
+#include "core/Deprecated.h"
namespace org {
namespace apache {
@@ -34,7 +35,7 @@ namespace utils {
struct ScopeGuard : ::gsl::final_action<std::function<void()>> {
using ::gsl::final_action<std::function<void()>>::final_action;
- void disable() noexcept {
+ DEPRECATED(/*deprecated in*/ 0.8.0, /*will remove in */ 1.0) void disable() noexcept {
dismiss();
}
};
diff --git a/libminifi/src/controllers/SSLContextService.cpp b/libminifi/src/controllers/SSLContextService.cpp
index d21d681..07a21a4 100644
--- a/libminifi/src/controllers/SSLContextService.cpp
+++ b/libminifi/src/controllers/SSLContextService.cpp
@@ -31,7 +31,7 @@
#include "core/Property.h"
#include "io/validation.h"
#include "properties/Configure.h"
-#include "utils/ScopeGuard.h"
+#include "utils/gsl.h"
namespace org {
namespace apache {
@@ -56,30 +56,30 @@ void SSLContextService::initialize() {
bool SSLContextService::configure_ssl_context(SSL_CTX *ctx) {
if (!IsNullOrEmpty(certificate)) {
if (isFileTypeP12(certificate)) {
- BIO* fp = BIO_new(BIO_s_file());
+ auto fp_deleter = [](BIO* ptr) { BIO_free(ptr); };
+ std::unique_ptr<BIO, decltype(fp_deleter)> fp(BIO_new(BIO_s_file()), fp_deleter);
if (fp == nullptr) {
logging::LOG_ERROR(logger_) << "Failed create new file BIO, " << getLatestOpenSSLErrorString();
return false;
}
- utils::ScopeGuard fp_guard([fp]() { BIO_free(fp); });
- if (BIO_read_filename(fp, certificate.c_str()) <= 0) {
+ if (BIO_read_filename(fp.get(), certificate.c_str()) <= 0) {
logging::LOG_ERROR(logger_) << "Failed to read certificate file " << certificate << ", " << getLatestOpenSSLErrorString();
return false;
}
- PKCS12* p12 = d2i_PKCS12_bio(fp, nullptr);
+ auto p12_deleter = [](PKCS12* ptr) { PKCS12_free(ptr); };
+ std::unique_ptr<PKCS12, decltype(p12_deleter)> p12(d2i_PKCS12_bio(fp.get(), nullptr), p12_deleter);
if (p12 == nullptr) {
logging::LOG_ERROR(logger_) << "Failed to DER decode certificate file " << certificate << ", " << getLatestOpenSSLErrorString();
return false;
}
- utils::ScopeGuard p12_guard([p12]() { PKCS12_free(p12); });
EVP_PKEY* pkey = nullptr;
X509* cert = nullptr;
STACK_OF(X509)* ca = nullptr;
- if (!PKCS12_parse(p12, passphrase_.c_str(), &pkey, &cert, &ca)) {
+ if (!PKCS12_parse(p12.get(), passphrase_.c_str(), &pkey, &cert, &ca)) {
logging::LOG_ERROR(logger_) << "Failed to parse certificate file " << certificate << " as PKCS#12, " << getLatestOpenSSLErrorString();
return false;
}
- utils::ScopeGuard certs_guard([pkey, cert, ca]() {
+ const auto certs_guard = gsl::finally([pkey, cert, ca]() {
EVP_PKEY_free(pkey);
X509_free(cert);
sk_X509_pop_free(ca, X509_free);
@@ -89,13 +89,13 @@ bool SSLContextService::configure_ssl_context(SSL_CTX *ctx) {
return false;
}
while (ca != nullptr && sk_X509_num(ca) > 0) {
- X509 *cacert = sk_X509_pop(ca);
- utils::ScopeGuard cacert_guard([cacert]() { X509_free(cacert); });
- if (SSL_CTX_add_extra_chain_cert(ctx, cacert) != 1) {
+ auto x509_deleter = [](X509* ptr) { X509_free(ptr); };
+ std::unique_ptr<X509, decltype(x509_deleter)> cacert(sk_X509_pop(ca), x509_deleter);
+ if (SSL_CTX_add_extra_chain_cert(ctx, cacert.get()) != 1) {
logging::LOG_ERROR(logger_) << "Failed to set additional certificate from " << certificate << ", " << getLatestOpenSSLErrorString();
return false;
}
- cacert_guard.disable();
+ cacert.release();
}
if (SSL_CTX_use_PrivateKey(ctx, pkey) != 1) {
logging::LOG_ERROR(logger_) << "Failed to set private key from " << certificate << ", " << getLatestOpenSSLErrorString();
diff --git a/libminifi/src/io/tls/TLSSocket.cpp b/libminifi/src/io/tls/TLSSocket.cpp
index 66c004e..ce4796f 100644
--- a/libminifi/src/io/tls/TLSSocket.cpp
+++ b/libminifi/src/io/tls/TLSSocket.cpp
@@ -45,7 +45,7 @@ namespace io {
TLSContext::TLSContext(const std::shared_ptr<Configure> &configure, std::shared_ptr<minifi::controllers::SSLContextService> ssl_service)
: SocketContext(configure),
error_value(TLS_GOOD),
- ctx(nullptr),
+ ctx(nullptr, deleteContext),
logger_(logging::LoggerFactory<TLSContext>::getLogger()),
configure_(configure),
ssl_service_(std::move(ssl_service)) {
@@ -72,18 +72,13 @@ int16_t TLSContext::initialize(bool server_method) {
}
const SSL_METHOD *method;
method = server_method ? TLSv1_2_server_method() : TLSv1_2_client_method();
- ctx = SSL_CTX_new(method);
- if (ctx == nullptr) {
+ auto local_context = std::unique_ptr<SSL_CTX, decltype(&deleteContext)>(SSL_CTX_new(method), deleteContext);
+ if (local_context == nullptr) {
logger_->log_error("Could not create SSL context, error: %s.", std::strerror(errno));
error_value = TLS_ERROR_CONTEXT;
return error_value;
}
- utils::ScopeGuard ctxGuard([this]() {
- SSL_CTX_free(ctx);
- ctx = nullptr;
- });
-
if (needClientCert) {
std::string certificate;
std::string privatekey;
@@ -91,11 +86,11 @@ int16_t TLSContext::initialize(bool server_method) {
std::string caCertificate;
if (ssl_service_ != nullptr) {
- if (!ssl_service_->configure_ssl_context(ctx)) {
+ if (!ssl_service_->configure_ssl_context(local_context.get())) {
error_value = TLS_ERROR_CERT_ERROR;
return error_value;
}
- ctxGuard.disable();
+ ctx = std::move(local_context);
error_value = TLS_GOOD;
return 0;
}
@@ -106,8 +101,8 @@ int16_t TLSContext::initialize(bool server_method) {
return error_value;
}
// load certificates and private key in PEM format
- if (SSL_CTX_use_certificate_chain_file(ctx, certificate.c_str()) <= 0) {
- logger_->log_error("Could not load certificate %s, for %X and %X error : %s", certificate, this, ctx, std::strerror(errno));
+ if (SSL_CTX_use_certificate_chain_file(local_context.get(), certificate.c_str()) <= 0) {
+ logger_->log_error("Could not load certificate %s, for %X and %X error : %s", certificate, this, local_context.get(), std::strerror(errno));
error_value = TLS_ERROR_CERT_MISSING;
return error_value;
}
@@ -120,25 +115,25 @@ int16_t TLSContext::initialize(bool server_method) {
file.close();
passphrase = password;
}
- SSL_CTX_set_default_passwd_cb(ctx, io::tls::pemPassWordCb);
- SSL_CTX_set_default_passwd_cb_userdata(ctx, &passphrase);
+ SSL_CTX_set_default_passwd_cb(local_context.get(), io::tls::pemPassWordCb);
+ SSL_CTX_set_default_passwd_cb_userdata(local_context.get(), &passphrase);
}
- int retp = SSL_CTX_use_PrivateKey_file(ctx, privatekey.c_str(), SSL_FILETYPE_PEM);
+ int retp = SSL_CTX_use_PrivateKey_file(local_context.get(), privatekey.c_str(), SSL_FILETYPE_PEM);
if (retp != 1) {
logger_->log_error("Could not create load private key,%i on %s error : %s", retp, privatekey, std::strerror(errno));
error_value = TLS_ERROR_KEY_ERROR;
return error_value;
}
// verify private key
- if (!SSL_CTX_check_private_key(ctx)) {
+ if (!SSL_CTX_check_private_key(local_context.get())) {
logger_->log_error("Private key does not match the public certificate, error : %s", std::strerror(errno));
error_value = TLS_ERROR_KEY_ERROR;
return error_value;
}
// load CA certificates
if (ssl_service_ != nullptr || configure_->get(Configure::nifi_security_client_ca_certificate, caCertificate)) {
- retp = SSL_CTX_load_verify_locations(ctx, caCertificate.c_str(), 0);
+ retp = SSL_CTX_load_verify_locations(local_context.get(), caCertificate.c_str(), 0);
if (retp == 0) {
logger_->log_error("Can not load CA certificate, Exiting, error : %s", std::strerror(errno));
error_value = TLS_ERROR_CERT_ERROR;
@@ -146,9 +141,9 @@ int16_t TLSContext::initialize(bool server_method) {
}
}
- logger_->log_debug("Load/Verify Client Certificate OK. for %X and %X", this, ctx);
+ logger_->log_debug("Load/Verify Client Certificate OK. for %X and %X", this, local_context.get());
}
- ctxGuard.disable();
+ ctx = std::move(local_context);
error_value = TLS_GOOD;
return 0;
}