diff --git a/kmipclient/CMakeLists.txt b/kmipclient/CMakeLists.txt index 7ecf46f..d02fdb6 100644 --- a/kmipclient/CMakeLists.txt +++ b/kmipclient/CMakeLists.txt @@ -5,6 +5,68 @@ project(kmipclient) set(CMAKE_CXX_STANDARD 20) find_package(OpenSSL REQUIRED) + +option(KMIP_MAINTAINER_MODE "Enable strict compiler warnings for kmipclient targets" OFF) +option(WARNINGS_AS_ERRORS "Treat compiler warnings as errors" OFF) + +function(kmip_enable_maintainer_warnings target_name) + if(NOT KMIP_MAINTAINER_MODE) + return() + endif() + + if(MSVC) + target_compile_options(${target_name} PRIVATE /W4) + if(WARNINGS_AS_ERRORS) + target_compile_options(${target_name} PRIVATE /WX) + endif() + return() + endif() + + if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + target_compile_options( + ${target_name} + PRIVATE + -Weverything + -Wno-c++98-compat + -Wno-c++98-compat-pedantic + -Wno-pre-c++20-compat + -Wno-c++20-compat + -Wno-padded + -Wno-switch-enum + -Wno-unsafe-buffer-usage + -Wno-covered-switch-default + -Wno-documentation + -Wno-exit-time-destructors + -Wno-global-constructors + -Wno-missing-prototypes + -Wno-newline-eof + -Wno-nrvo + -Wno-weak-vtables + ) + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + target_compile_options( + ${target_name} + PRIVATE + -Wall + -Wextra + -Wpedantic + -Wcast-qual + -Wconversion + -Wdouble-promotion + -Wformat=2 + -Wnull-dereference + -Woverloaded-virtual + -Wshadow + -Wsign-conversion + -Wundef + -Wuseless-cast + ) + endif() + + if(WARNINGS_AS_ERRORS) + target_compile_options(${target_name} PRIVATE -Werror) + endif() +endfunction() find_package(Threads REQUIRED) add_library( @@ -77,6 +139,7 @@ install( macro(add_example name) add_executable(example_${name} examples/example_${name}.cpp) target_link_libraries(example_${name} PRIVATE kmipclient) + kmip_enable_maintainer_warnings(example_${name}) endmacro() add_example(create_aes) @@ -115,6 +178,15 @@ if(BUILD_TESTS) FetchContent_MakeAvailable(googletest) endif() + if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + if(TARGET gtest) + target_compile_options(gtest PRIVATE -Wno-character-conversion) + endif() + if(TARGET gtest_main) + target_compile_options(gtest_main PRIVATE -Wno-character-conversion) + endif() + endif() + enable_testing() add_executable( @@ -131,6 +203,7 @@ if(BUILD_TESTS) GTest::gtest_main kmipclient ) + kmip_enable_maintainer_warnings(kmipclient_test) include(GoogleTest) gtest_discover_tests(kmipclient_test) diff --git a/kmipclient/examples/example_activate.cpp b/kmipclient/examples/example_activate.cpp index d6054f8..b44d693 100644 --- a/kmipclient/examples/example_activate.cpp +++ b/kmipclient/examples/example_activate.cpp @@ -42,7 +42,7 @@ int main(int argc, char **argv) { } catch (const std::exception &e) { std::cerr << "Can not activate key with id:" << argv[6] << " Cause: " << e.what() << std::endl; - }; + } return -1; } diff --git a/kmipclient/examples/example_get.cpp b/kmipclient/examples/example_get.cpp index 03ef4bd..4e67436 100644 --- a/kmipclient/examples/example_get.cpp +++ b/kmipclient/examples/example_get.cpp @@ -58,7 +58,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get key with id:" << argv[6] << " Cause: " << e.what() << std::endl; return 1; - }; + } return 0; } diff --git a/kmipclient/examples/example_get_all_ids.cpp b/kmipclient/examples/example_get_all_ids.cpp index 6ff502a..d4ef041 100644 --- a/kmipclient/examples/example_get_all_ids.cpp +++ b/kmipclient/examples/example_get_all_ids.cpp @@ -44,7 +44,7 @@ int main(int argc, char **argv) { } } catch (const std::exception &e) { std::cerr << "Can not get keys." << " Cause: " << e.what() << std::endl; - }; + } try { const auto opt_ids_s = client.op_all(object_type::KMIP_OBJTYPE_SECRET_DATA); @@ -54,7 +54,7 @@ int main(int argc, char **argv) { } } catch (const std::exception &e) { std::cerr << "Can not get id-s. Cause: " << e.what() << std::endl; - }; + } return 0; } diff --git a/kmipclient/examples/example_get_attributes.cpp b/kmipclient/examples/example_get_attributes.cpp index 9ade5f2..bd21748 100644 --- a/kmipclient/examples/example_get_attributes.cpp +++ b/kmipclient/examples/example_get_attributes.cpp @@ -74,7 +74,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get key with id:" << argv[6] << " Cause: " << e.what() << std::endl; return -1; - }; + } return 0; } diff --git a/kmipclient/examples/example_get_name.cpp b/kmipclient/examples/example_get_name.cpp index 256bac4..ff43036 100644 --- a/kmipclient/examples/example_get_name.cpp +++ b/kmipclient/examples/example_get_name.cpp @@ -53,7 +53,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get name or group for id:" << argv[6] << " Cause: " << e.what() << std::endl; return -1; - }; + } return 0; } diff --git a/kmipclient/examples/example_get_secret.cpp b/kmipclient/examples/example_get_secret.cpp index 5a2b884..8431938 100644 --- a/kmipclient/examples/example_get_secret.cpp +++ b/kmipclient/examples/example_get_secret.cpp @@ -57,7 +57,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get secret with id:" << argv[6] << " Cause: " << e.what() << std::endl; return -1; - }; + } return 0; } diff --git a/kmipclient/examples/example_get_tls_verify.cpp b/kmipclient/examples/example_get_tls_verify.cpp index b7dfd6c..687bac4 100644 --- a/kmipclient/examples/example_get_tls_verify.cpp +++ b/kmipclient/examples/example_get_tls_verify.cpp @@ -114,7 +114,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get key with id:" << argv[6] << " Cause: " << e.what() << std::endl; return 1; - }; + } return 0; } diff --git a/kmipclient/examples/example_locate.cpp b/kmipclient/examples/example_locate.cpp index 9a65981..6a355e7 100644 --- a/kmipclient/examples/example_locate.cpp +++ b/kmipclient/examples/example_locate.cpp @@ -51,7 +51,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get keys with name:" << argv[6] << " Cause: " << e.what() << std::endl; return 1; - }; + } try { const auto opt_ids_s = client.op_locate_by_name( @@ -65,7 +65,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get secrets with name:" << argv[6] << " Cause: " << e.what() << std::endl; return 1; - }; + } return 0; } diff --git a/kmipclient/examples/example_locate_by_group.cpp b/kmipclient/examples/example_locate_by_group.cpp index ed2e975..75812a5 100644 --- a/kmipclient/examples/example_locate_by_group.cpp +++ b/kmipclient/examples/example_locate_by_group.cpp @@ -51,7 +51,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get keys with group name:" << argv[6] << " Cause: " << e.what() << std::endl; return -1; - }; + } try { const auto opt_ids_s = client.op_locate_by_group( @@ -65,7 +65,7 @@ int main(int argc, char **argv) { std::cerr << "Can not get secrets with group name:" << argv[6] << " Cause: " << e.what() << std::endl; return -1; - }; + } return 0; } diff --git a/kmipclient/examples/example_pool.cpp b/kmipclient/examples/example_pool.cpp index 3fb8b2d..856a38d 100644 --- a/kmipclient/examples/example_pool.cpp +++ b/kmipclient/examples/example_pool.cpp @@ -75,6 +75,7 @@ int main(int argc, char **argv) { .client_cert = client_cert, .client_key = client_key, .server_ca_cert = server_ca_cert, + .logger = nullptr, .timeout_ms = 5000, .max_connections = static_cast(max_pool_size), } @@ -84,7 +85,7 @@ int main(int argc, char **argv) { // Spawn threads – each borrows a connection, uses it, returns it. // ------------------------------------------------------------------ std::vector threads; - threads.reserve(num_threads); + threads.reserve(static_cast(num_threads)); for (int i = 0; i < num_threads; ++i) { threads.emplace_back([&pool, &key_name_prefix, i]() { diff --git a/kmipclient/examples/example_register_key.cpp b/kmipclient/examples/example_register_key.cpp index a30c873..10dd253 100644 --- a/kmipclient/examples/example_register_key.cpp +++ b/kmipclient/examples/example_register_key.cpp @@ -86,7 +86,7 @@ int main(int argc, char **argv) { std::cerr << "Can not register key:" << argv[6] << " Cause: " << e.what() << std::endl; return -1; - }; + } return 0; } diff --git a/kmipclient/examples/example_revoke.cpp b/kmipclient/examples/example_revoke.cpp index 451aa84..e0a0389 100644 --- a/kmipclient/examples/example_revoke.cpp +++ b/kmipclient/examples/example_revoke.cpp @@ -48,6 +48,6 @@ int main(int argc, char **argv) { std::cerr << "Can not get key with id:" << argv[6] << " Cause: " << e.what() << std::endl; return -1; - }; + } return 0; } diff --git a/kmipclient/include/kmipclient/Kmip.hpp b/kmipclient/include/kmipclient/Kmip.hpp index bf53028..9059141 100644 --- a/kmipclient/include/kmipclient/Kmip.hpp +++ b/kmipclient/include/kmipclient/Kmip.hpp @@ -79,7 +79,7 @@ namespace kmipclient { )), m_client(m_net_client, logger, version, close_on_destroy) { m_net_client->connect(); - }; + } /** * @brief Destroys the facade, closing the transport if close_on_destroy is true. @@ -99,23 +99,25 @@ namespace kmipclient { * @brief Returns the initialized high-level KMIP client. * @return Mutable reference to the owned @ref KmipClient. */ - KmipClient &client() { return m_client; }; + KmipClient &client() { return m_client; } /** * @brief Returns const reference to the client. */ - [[nodiscard]] const KmipClient &client() const { return m_client; }; + [[nodiscard]] const KmipClient &client() const { return m_client; } /** * @brief Returns reference to the underlying transport. * Use with care; generally prefer client() for KMIP operations. */ - NetClientOpenSSL &transport() { return *m_net_client; }; + NetClientOpenSSL &transport() { return *m_net_client; } /** * @brief Returns const reference to the underlying transport. */ - [[nodiscard]] const NetClientOpenSSL &transport() const { return *m_net_client; }; + [[nodiscard]] const NetClientOpenSSL &transport() const { + return *m_net_client; + } /** * @brief Queries the close_on_destroy setting. diff --git a/kmipclient/include/kmipclient/NetClient.hpp b/kmipclient/include/kmipclient/NetClient.hpp index 91585a9..5fadbdf 100644 --- a/kmipclient/include/kmipclient/NetClient.hpp +++ b/kmipclient/include/kmipclient/NetClient.hpp @@ -68,15 +68,15 @@ namespace kmipclient { m_clientCertificateFn(clientCertificateFn), m_clientKeyFn(clientKeyFn), m_serverCaCertificateFn(serverCaCertFn), - m_timeout_ms(timeout_ms) {}; + m_timeout_ms(timeout_ms) {} /** @brief Virtual destructor for interface-safe cleanup. */ virtual ~NetClient() = default; // no copy, no move NetClient(const NetClient &) = delete; - virtual NetClient &operator=(const NetClient &) = delete; + NetClient &operator=(const NetClient &) = delete; NetClient(NetClient &&) = delete; - virtual NetClient &operator=(NetClient &&) = delete; + NetClient &operator=(NetClient &&) = delete; /** * @brief Establishes network/TLS connection to the KMIP server. * Must honor @ref m_timeout_ms for connect + handshake phases. diff --git a/kmipclient/include/kmipclient/kmipclient_version.hpp b/kmipclient/include/kmipclient/kmipclient_version.hpp index b912b94..63f43f1 100644 --- a/kmipclient/include/kmipclient/kmipclient_version.hpp +++ b/kmipclient/include/kmipclient/kmipclient_version.hpp @@ -25,7 +25,7 @@ /** @brief kmipclient semantic version minor component. */ #define KMIPCLIENT_VERSION_MINOR 2 /** @brief kmipclient semantic version patch component. */ -#define KMIPCLIENT_VERSION_PATCH 1 +#define KMIPCLIENT_VERSION_PATCH 3 /** @brief Internal helper for macro-stringification. */ #define KMIPCLIENT_STRINGIFY_I(x) #x diff --git a/kmipclient/src/IOUtils.hpp b/kmipclient/src/IOUtils.hpp index 283db77..48ab3b2 100644 --- a/kmipclient/src/IOUtils.hpp +++ b/kmipclient/src/IOUtils.hpp @@ -34,7 +34,7 @@ namespace kmipclient { explicit IOUtils( NetClient &nc, const std::shared_ptr &logger = {} ) - : net_client(nc), logger_(logger) {}; + : net_client(nc), logger_(logger) {} void do_exchange( const std::vector &request_bytes, diff --git a/kmipclient/src/KmipClient.cpp b/kmipclient/src/KmipClient.cpp index 99e0db8..55a53ef 100644 --- a/kmipclient/src/KmipClient.cpp +++ b/kmipclient/src/KmipClient.cpp @@ -72,24 +72,24 @@ namespace kmipclient { } KmipClient::KmipClient( - NetClient &net_client, + NetClient &transport, const std::shared_ptr &logger, kmipcore::ProtocolVersion version, bool close_on_destroy ) - : net_client(&net_client), - io(std::make_unique(net_client, logger)), + : net_client(&transport), + io(std::make_unique(transport, logger)), version_(version), close_on_destroy_(close_on_destroy) {}; KmipClient::KmipClient( - std::shared_ptr net_client, + std::shared_ptr transport, const std::shared_ptr &logger, kmipcore::ProtocolVersion version, bool close_on_destroy ) - : net_client(net_client.get()), - net_client_owner_(std::move(net_client)), + : net_client(transport.get()), + net_client_owner_(std::move(transport)), io(), version_(version), close_on_destroy_(close_on_destroy) { diff --git a/kmipclient/src/NetClientOpenSSL.cpp b/kmipclient/src/NetClientOpenSSL.cpp index 75cdabc..97d64c4 100644 --- a/kmipclient/src/NetClientOpenSSL.cpp +++ b/kmipclient/src/NetClientOpenSSL.cpp @@ -428,8 +428,8 @@ namespace kmipclient { configure_tls_verification(new_ctx.get(), ssl, m_host, m_tls_verification); SSL_set_mode(ssl, SSL_MODE_AUTO_RETRY); - BIO_set_conn_hostname(new_bio.get(), m_host.c_str()); - BIO_set_conn_port(new_bio.get(), m_port.c_str()); + BIO_set_conn_hostname(new_bio.get(), m_host.data()); + BIO_set_conn_port(new_bio.get(), m_port.data()); if (m_timeout_ms > 0) { if (BIO_set_nbio(new_bio.get(), 1) != 1) { @@ -443,7 +443,7 @@ namespace kmipclient { std::chrono::milliseconds(m_timeout_ms); for (;;) { ERR_clear_error(); - const int connect_ret = BIO_do_connect(new_bio.get()); + const auto connect_ret = BIO_do_connect(new_bio.get()); if (connect_ret == 1) { break; } diff --git a/kmipclient/src/PEMReader.cpp b/kmipclient/src/PEMReader.cpp index c318d53..d4c0ed7 100644 --- a/kmipclient/src/PEMReader.cpp +++ b/kmipclient/src/PEMReader.cpp @@ -53,7 +53,7 @@ namespace kmipclient { OPENSSL_free(der); kmipcore::Attributes attrs; - attrs.set(KMIP_ATTR_NAME_NAME, "certificate"); + attrs.set(KMIP_ATTR_NAME_NAME, std::string("certificate")); return X509Certificate(cert_bytes, std::move(attrs)); } @@ -78,7 +78,7 @@ namespace kmipclient { OPENSSL_free(der); kmipcore::Attributes attrs; - attrs.set(KMIP_ATTR_NAME_NAME, "private_key"); + attrs.set(KMIP_ATTR_NAME_NAME, std::string("private_key")); return PrivateKey(key_bytes, std::move(attrs)); } @@ -103,7 +103,7 @@ namespace kmipclient { OPENSSL_free(der); kmipcore::Attributes attrs; - attrs.set(KMIP_ATTR_NAME_NAME, "public_key"); + attrs.set(KMIP_ATTR_NAME_NAME, std::string("public_key")); return PublicKey(key_bytes, std::move(attrs)); } diff --git a/kmipclient/src/StringUtils.cpp b/kmipclient/src/StringUtils.cpp index 7d86213..0dfe8c3 100644 --- a/kmipclient/src/StringUtils.cpp +++ b/kmipclient/src/StringUtils.cpp @@ -26,13 +26,13 @@ namespace kmipclient { unsigned char char2int(const char input) { if (input >= '0' && input <= '9') { - return input - '0'; + return static_cast(input - '0'); } if (input >= 'A' && input <= 'F') { - return input - 'A' + 10; + return static_cast(input - 'A' + 10); } if (input >= 'a' && input <= 'f') { - return input - 'a' + 10; + return static_cast(input - 'a' + 10); } throw kmipcore::KmipException{"Invalid hex character."}; } @@ -57,8 +57,11 @@ namespace kmipclient { std::array l{}; l.fill(-1); for (int i = 0; i < 64; ++i) { - l["ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/" - [i]] = i; + const auto idx = static_cast( + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/" + [static_cast(i)] + ); + l[idx] = i; } return l; }(); @@ -67,7 +70,8 @@ namespace kmipclient { out.reserve((base64.size() / 4) * 3); int val = 0, val_b = -8; - for (unsigned char c : base64) { + for (const char ch : base64) { + const auto c = static_cast(ch); if (lookup[c] == -1) { if (c == '=') { break; // Padding reached diff --git a/kmipclient/tests/IOUtilsTest.cpp b/kmipclient/tests/IOUtilsTest.cpp index c720181..f9ac5a2 100644 --- a/kmipclient/tests/IOUtilsTest.cpp +++ b/kmipclient/tests/IOUtilsTest.cpp @@ -63,7 +63,7 @@ namespace { int send(std::span data) override { ++send_calls; - const int desired = send_plan_index < static_cast(send_plan.size()) + const int desired = send_plan_index < send_plan.size() ? send_plan[send_plan_index++] : static_cast(data.size()); if (desired <= 0) { @@ -93,7 +93,7 @@ namespace { int send_calls = 0; private: - int send_plan_index = 0; + size_t send_plan_index = 0; size_t recv_offset = 0; }; diff --git a/kmipclient/tests/KmipClientPoolIntegrationTest.cpp b/kmipclient/tests/KmipClientPoolIntegrationTest.cpp index 7c08a2c..06cd2f0 100644 --- a/kmipclient/tests/KmipClientPoolIntegrationTest.cpp +++ b/kmipclient/tests/KmipClientPoolIntegrationTest.cpp @@ -187,6 +187,7 @@ class KmipClientPoolIntegrationTest : public ::testing::Test { .client_cert = config.kmip_client_ca, .client_key = config.kmip_client_key, .server_ca_cert = config.kmip_server_ca, + .logger = nullptr, .timeout_ms = config.timeout_ms, .max_connections = max_connections, .tls_verification = { diff --git a/kmipcore/CMakeLists.txt b/kmipcore/CMakeLists.txt index 3747aff..50fb243 100644 --- a/kmipcore/CMakeLists.txt +++ b/kmipcore/CMakeLists.txt @@ -5,11 +5,74 @@ project(kmipcore LANGUAGES CXX) set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) +option(KMIP_MAINTAINER_MODE "Enable strict compiler warnings for kmipcore targets" OFF) +option(WARNINGS_AS_ERRORS "Treat compiler warnings as errors" OFF) + +function(kmip_enable_maintainer_warnings target_name) + if(NOT KMIP_MAINTAINER_MODE) + return() + endif() + + if(MSVC) + target_compile_options(${target_name} PRIVATE /W4) + if(WARNINGS_AS_ERRORS) + target_compile_options(${target_name} PRIVATE /WX) + endif() + return() + endif() + + if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + target_compile_options( + ${target_name} + PRIVATE + -Weverything + -Wno-c++98-compat + -Wno-c++98-compat-pedantic + -Wno-pre-c++20-compat + -Wno-c++20-compat + -Wno-padded + -Wno-switch-enum + -Wno-unsafe-buffer-usage + -Wno-covered-switch-default + -Wno-documentation + -Wno-exit-time-destructors + -Wno-global-constructors + -Wno-missing-prototypes + -Wno-newline-eof + -Wno-nrvo + -Wno-weak-vtables + ) + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + target_compile_options( + ${target_name} + PRIVATE + -Wall + -Wextra + -Wpedantic + -Wcast-qual + -Wconversion + -Wdouble-promotion + -Wformat=2 + -Wnull-dereference + -Woverloaded-virtual + -Wshadow + -Wsign-conversion + -Wundef + -Wuseless-cast + ) + endif() + + if(WARNINGS_AS_ERRORS) + target_compile_options(${target_name} PRIVATE -Werror) + endif() +endfunction() + file(GLOB_RECURSE SOURCES CONFIGURE_DEPENDS "src/*.cpp") file(GLOB_RECURSE HEADERS CONFIGURE_DEPENDS "include/kmipcore/*.hpp") add_library(kmipcore STATIC ${SOURCES} ${HEADERS}) +kmip_enable_maintainer_warnings(kmipcore) target_include_directories(kmipcore PUBLIC $ @@ -27,12 +90,15 @@ install( add_executable(kmip_core_test tests/test_core.cpp) target_link_libraries(kmip_core_test PRIVATE kmipcore) +kmip_enable_maintainer_warnings(kmip_core_test) add_test(NAME kmip_core_test COMMAND kmip_core_test) add_executable(kmip_parser_test tests/test_parsers.cpp) target_link_libraries(kmip_parser_test PRIVATE kmipcore) +kmip_enable_maintainer_warnings(kmip_parser_test) add_test(NAME kmip_parser_test COMMAND kmip_parser_test) add_executable(kmip_serialization_buffer_test tests/test_serialization_buffer.cpp) target_link_libraries(kmip_serialization_buffer_test PRIVATE kmipcore) +kmip_enable_maintainer_warnings(kmip_serialization_buffer_test) add_test(NAME kmip_serialization_buffer_test COMMAND kmip_serialization_buffer_test) diff --git a/kmipcore/include/kmipcore/kmip_enums.hpp b/kmipcore/include/kmipcore/kmip_enums.hpp index defc224..95381af 100644 --- a/kmipcore/include/kmipcore/kmip_enums.hpp +++ b/kmipcore/include/kmipcore/kmip_enums.hpp @@ -951,7 +951,7 @@ namespace kmipcore { KMIP_REVOKE_SUSPENDED = 0x05, KMIP_REVOKE_CESSATION_OF_OPERATION = 0x06, KMIP_REVOKE_PRIVILEDGE_WITHDRAWN = 0x07, - KMIP_REVOKE_EXTENSIONS = static_cast(0x80000000u) + KMIP_REVOKE_EXTENSIONS = 0x80000000u }; /** @brief KMIP secret payload data type identifiers. */ @@ -959,7 +959,7 @@ namespace kmipcore { // KMIP 1.0 KMIP_SECDATA_PASSWORD = 0x01, KMIP_SECDATA_SEED = 0x02, - KMIP_SECDATA_EXTENSIONS = static_cast(0x80000000u) + KMIP_SECDATA_EXTENSIONS = 0x80000000u }; // --------------------------------------------------------------------------- diff --git a/kmipcore/include/kmipcore/kmip_formatter.hpp b/kmipcore/include/kmipcore/kmip_formatter.hpp index d69b434..974816d 100644 --- a/kmipcore/include/kmipcore/kmip_formatter.hpp +++ b/kmipcore/include/kmipcore/kmip_formatter.hpp @@ -8,7 +8,7 @@ namespace kmipcore { - class Element; + struct Element; class RequestMessage; class ResponseMessage; diff --git a/kmipcore/include/kmipcore/kmipcore_version.hpp b/kmipcore/include/kmipcore/kmipcore_version.hpp index 78f5ba9..c19cb37 100644 --- a/kmipcore/include/kmipcore/kmipcore_version.hpp +++ b/kmipcore/include/kmipcore/kmipcore_version.hpp @@ -23,7 +23,7 @@ /** @brief kmipcore semantic version minor component. */ #define KMIPCORE_VERSION_MINOR 1 /** @brief kmipcore semantic version patch component. */ -#define KMIPCORE_VERSION_PATCH 2 +#define KMIPCORE_VERSION_PATCH 3 /** @brief Internal helper for macro stringification. */ #define KMIPCORE_STRINGIFY_I(x) #x diff --git a/kmipcore/src/kmip_basics.cpp b/kmipcore/src/kmip_basics.cpp index e093900..d9ab74a 100644 --- a/kmipcore/src/kmip_basics.cpp +++ b/kmipcore/src/kmip_basics.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include namespace kmipcore { @@ -15,10 +16,19 @@ namespace kmipcore { return htonl(v); } static std::uint64_t to_be64(std::uint64_t v) { - std::uint32_t high = htonl(v >> 32); - std::uint32_t low = htonl(v & 0xFFFFFFFF); + std::uint32_t high = htonl(static_cast(v >> 32)); + std::uint32_t low = htonl(static_cast(v & 0xFFFFFFFFULL)); return (static_cast(low) << 32) | high; } + + static std::uint32_t to_u32_size(std::size_t value, const char *field_name) { + if (value > std::numeric_limits::max()) { + throw KmipException( + std::string("TTLV ") + field_name + " exceeds uint32_t range" + ); + } + return static_cast(value); + } // Safe big-endian decoders from raw byte spans. static std::uint32_t read_be_u32(std::span data, std::size_t off) { @@ -77,7 +87,7 @@ namespace kmipcore { for (const auto &item : s.items) { item->serialize(content_buf); // Recursive call } - payload_length = content_buf.size(); + payload_length = to_u32_size(content_buf.size(), "payload length"); } else if (std::holds_alternative(value)) { std::uint32_t wire = to_be32(static_cast(std::get(value).value)); @@ -92,7 +102,7 @@ namespace kmipcore { } else if (std::holds_alternative(value)) { const auto &v = std::get(value).value; content_buf.writeBytes(std::as_bytes(std::span(v.data(), v.size()))); - payload_length = v.size(); + payload_length = to_u32_size(v.size(), "BigInteger length"); } else if (std::holds_alternative(value)) { std::uint32_t wire = to_be32( static_cast(std::get(value).value) @@ -107,11 +117,11 @@ namespace kmipcore { } else if (std::holds_alternative(value)) { const auto &v = std::get(value).value; content_buf.writePadded(std::as_bytes(std::span(v.data(), v.size()))); - payload_length = v.size(); + payload_length = to_u32_size(v.size(), "TextString length"); } else if (std::holds_alternative(value)) { const auto &v = std::get(value).value; content_buf.writePadded(std::as_bytes(std::span(v.data(), v.size()))); - payload_length = v.size(); + payload_length = to_u32_size(v.size(), "ByteString length"); } else if (std::holds_alternative(value)) { std::uint64_t wire = to_be64(static_cast(std::get(value).value)); @@ -133,10 +143,10 @@ namespace kmipcore { } // Write Length (4 bytes, big-endian) - buf.writeByte((payload_length >> 24) & 0xFF); - buf.writeByte((payload_length >> 16) & 0xFF); - buf.writeByte((payload_length >> 8) & 0xFF); - buf.writeByte(payload_length & 0xFF); + buf.writeByte(static_cast((payload_length >> 24) & 0xFF)); + buf.writeByte(static_cast((payload_length >> 16) & 0xFF)); + buf.writeByte(static_cast((payload_length >> 8) & 0xFF)); + buf.writeByte(static_cast(payload_length & 0xFF)); // Write content (already padded from content_buf) if (content_buf.size() > 0) { diff --git a/kmipcore/src/kmip_protocol.cpp b/kmipcore/src/kmip_protocol.cpp index 0734de5..73bca27 100644 --- a/kmipcore/src/kmip_protocol.cpp +++ b/kmipcore/src/kmip_protocol.cpp @@ -316,7 +316,7 @@ namespace kmipcore { !request.header_.getBatchOrderOption().has_value()) { request.header_.setBatchOrderOption(true); } - request.header_.setTimeStamp(static_cast(time(nullptr))); + request.header_.setTimeStamp(time(nullptr)); // Use SerializationBuffer for efficient serialization SerializationBuffer buf(request.getMaxResponseSize()); @@ -348,8 +348,8 @@ namespace kmipcore { validate_element_types_for_version( element, rm.header_.getProtocolVersion() ); - const auto *s = std::get_if(&element->value); - for (const auto &child : s->items) { + const auto &s = std::get(element->value); + for (const auto &child : s.items) { if (child->tag == tag::KMIP_TAG_BATCH_ITEM) { rm.batchItems_.push_back(RequestBatchItem::fromElement(child)); } @@ -510,8 +510,8 @@ namespace kmipcore { validate_element_types_for_version( element, rm.header_.getProtocolVersion() ); - const auto *s = std::get_if(&element->value); - for (const auto &child : s->items) { + const auto &s = std::get(element->value); + for (const auto &child : s.items) { if (child->tag == tag::KMIP_TAG_BATCH_ITEM) { rm.batchItems_.push_back(ResponseBatchItem::fromElement(child)); } diff --git a/kmipcore/tests/test_core.cpp b/kmipcore/tests/test_core.cpp index 7b2b63e..b9788fa 100644 --- a/kmipcore/tests/test_core.cpp +++ b/kmipcore/tests/test_core.cpp @@ -99,13 +99,11 @@ void test_date_time_extended_invalid_length() { }; size_t offset = 0; - bool threw = false; try { (void) Element::deserialize(invalid, offset); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); std::cout << "DateTimeExtended invalid-length test passed" << std::endl; } @@ -132,13 +130,11 @@ void test_non_zero_padding_is_rejected() { }; size_t offset = 0; - bool threw = false; try { (void) Element::deserialize(invalid, offset); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); std::cout << "Non-zero padding validation test passed" << std::endl; } @@ -158,13 +154,11 @@ void test_date_time_extended_requires_kmip_2_0_for_requests() { RequestMessage request_14; request_14.add_batch_item(item); - bool threw = false; try { (void) request_14.serialize(); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); RequestMessage request_20(KMIP_VERSION_2_0); assert(request_20.getHeader().getProtocolVersion().getMajor() == 2); @@ -184,9 +178,8 @@ void test_date_time_extended_requires_kmip_2_0_for_requests() { request_header->getChild(tag::KMIP_TAG_PROTOCOL_VERSION); assert(protocol_version != nullptr); - auto parsed_version = ProtocolVersion::fromElement(protocol_version); - assert(parsed_version.getMajor() == 2); - assert(parsed_version.getMinor() == 0); + assert(ProtocolVersion::fromElement(protocol_version).getMajor() == 2); + assert(ProtocolVersion::fromElement(protocol_version).getMinor() == 0); std::cout << "DateTimeExtended KMIP 2.0 request-version test passed" << std::endl; @@ -221,13 +214,11 @@ void test_date_time_extended_requires_kmip_2_0_for_responses() { batch_item->asStructure()->add(payload); response->asStructure()->add(batch_item); - bool threw = false; try { (void) ResponseMessage::fromElement(response); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); header.setProtocolVersion(ProtocolVersion(2, 0)); auto response_20 = Element::createStructure(tag::KMIP_TAG_RESPONSE_MESSAGE); @@ -256,7 +247,7 @@ void test_request_message() { Element::createInteger(tag::KMIP_TAG_ACTIVATION_DATE, 999) ); item.setRequestPayload(payload); - auto first_id = req.add_batch_item(item); + req.add_batch_item(item); RequestBatchItem item2; item2.setOperation(KMIP_OP_GET_ATTRIBUTE_LIST); @@ -265,11 +256,7 @@ void test_request_message() { Element::createInteger(tag::KMIP_TAG_ACTIVATION_DATE, 111) ); item2.setRequestPayload(payload2); - auto second_id = req.add_batch_item(item2); - - assert(first_id == 1); - assert(second_id == 2); - assert(first_id != second_id); + req.add_batch_item(item2); auto bytes = req.serialize(); std::cout << "Serialized RequestMessage size: " << bytes.size() << std::endl; @@ -613,13 +600,11 @@ void test_response_required_fields() { ); response_message->asStructure()->add(batch_item); - bool threw = false; try { (void) ResponseMessage::fromElement(response_message); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); } { @@ -647,13 +632,11 @@ void test_response_required_fields() { response_message->asStructure()->add(batch_item); // Must NOT throw: under-delivery (fewer items than declared) is accepted. - bool threw = false; try { (void) ResponseMessage::fromElement(response_message); } catch (const KmipException &) { - threw = true; + assert(false); } - assert(!threw); } { @@ -685,13 +668,11 @@ void test_response_required_fields() { make_success_item(KMIP_OP_GET_ATTRIBUTES) ); - bool threw = false; try { (void) ResponseMessage::fromElement(response_message); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); } { @@ -712,13 +693,11 @@ void test_response_required_fields() { ); response_message->asStructure()->add(batch_item); - bool threw = false; try { (void) ResponseMessage::fromElement(response_message); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); } { @@ -744,13 +723,11 @@ void test_response_required_fields() { ); response_message->asStructure()->add(batch_item); - bool threw = false; try { (void) ResponseMessage::fromElement(response_message); + assert(false); } catch (const KmipException &) { - threw = true; } - assert(threw); } { diff --git a/kmipcore/tests/test_parsers.cpp b/kmipcore/tests/test_parsers.cpp index aecece0..cb71274 100644 --- a/kmipcore/tests/test_parsers.cpp +++ b/kmipcore/tests/test_parsers.cpp @@ -93,15 +93,12 @@ void test_response_parser_failure_preserves_reason_code() { ); ResponseParser parser(bytes); - bool threw = false; try { - [[maybe_unused]] auto resp = parser.getResponse(0); + (void) parser.getResponse(0); + assert(false); } catch (const KmipException &e) { - threw = true; assert(e.code().value() == KMIP_REASON_ITEM_NOT_FOUND); } - - assert(threw); std::cout << "ResponseParser failure reason-code preservation test passed" << std::endl; } @@ -145,17 +142,15 @@ void test_response_parser_operation_hint_when_operation_absent() { // the operation in the thrown error is 0 ("Unknown Operation"). { ResponseParser parser(bytes); - bool threw = false; try { - [[maybe_unused]] auto resp = parser.getResponse(0); + (void) parser.getResponse(0); + assert(false); } catch (const KmipException &e) { - threw = true; assert(e.code().value() == KMIP_REASON_ITEM_NOT_FOUND); // Operation string should be the "unknown" fallback. const std::string msg = e.what(); assert(msg.find("Operation: Unknown Operation") != std::string::npos); } - assert(threw); } // ── Case 2: parser WITH hints → error message should show the correct op. @@ -165,18 +160,16 @@ void test_response_parser_operation_hint_when_operation_absent() { request.add_batch_item(GetRequest("fake-id-abc")); ResponseParser parser(bytes, request); - bool threw = false; try { - [[maybe_unused]] auto resp = parser.getResponse(0); + (void) parser.getResponse(0); + assert(false); } catch (const KmipException &e) { - threw = true; assert(e.code().value() == KMIP_REASON_ITEM_NOT_FOUND); const std::string msg = e.what(); // With the hint the error should now say "Operation: Get". assert(msg.find("Operation: Get") != std::string::npos); assert(msg.find("Result reason:") != std::string::npos); } - assert(threw); } std::cout << "ResponseParser operation-hint fallback test passed" @@ -243,11 +236,19 @@ void test_response_parser_discover_versions() { ResponseParser parser(bytes); auto discover_resp = parser.getResponse(0); - const auto &versions = discover_resp.getProtocolVersions(); - assert(versions.size() == 3); - assert(versions[0].getMajor() == 2 && versions[0].getMinor() == 1); - assert(versions[1].getMajor() == 2 && versions[1].getMinor() == 0); - assert(versions[2].getMajor() == 1 && versions[2].getMinor() == 4); + assert(discover_resp.getProtocolVersions().size() == 3); + assert( + discover_resp.getProtocolVersions()[0].getMajor() == 2 && + discover_resp.getProtocolVersions()[0].getMinor() == 1 + ); + assert( + discover_resp.getProtocolVersions()[1].getMajor() == 2 && + discover_resp.getProtocolVersions()[1].getMinor() == 0 + ); + assert( + discover_resp.getProtocolVersions()[2].getMajor() == 1 && + discover_resp.getProtocolVersions()[2].getMinor() == 4 + ); std::cout << "ResponseParser Discover Versions test passed" << std::endl; } @@ -536,8 +537,10 @@ void test_attributes_parser_extended() { auto parsed_attrs = AttributesParser::parse(attributes); assert(parsed_attrs.has_attribute("Activation Date")); - const auto &date_str = parsed_attrs.get("Activation Date"); - assert(date_str.find("2023-03-15") != std::string::npos); + assert( + parsed_attrs.get("Activation Date").find("2023-03-15") != + std::string::npos + ); // Cryptographic Algorithm is now a typed field. assert( @@ -768,10 +771,9 @@ void test_get_attribute_list_response_supports_v2_attribute_reference() { ResponseParser parser(bytes); auto response = parser.getResponse(0); - const auto &names = response.getAttributeNames(); - assert(names.size() == 2); - assert(names[0] == KMIP_ATTR_NAME_STATE); - assert(names[1] == "Vendor Custom Attr"); + assert(response.getAttributeNames().size() == 2); + assert(response.getAttributeNames()[0] == KMIP_ATTR_NAME_STATE); + assert(response.getAttributeNames()[1] == "Vendor Custom Attr"); std::cout << "GetAttributeListResponse KMIP 2.0 Attribute Reference test passed" diff --git a/kmippp/kmippp.cpp b/kmippp/kmippp.cpp index b19b06b..d8f3503 100644 --- a/kmippp/kmippp.cpp +++ b/kmippp/kmippp.cpp @@ -228,9 +228,6 @@ context::op_activate (context::id_t id) bool context::op_destroy (context::id_t id) { - - int key_len = 0; - char *keyp = nullptr; int result = kmip_bio_destroy_symmetric_key (bio_, const_cast (id.c_str ()), id.length ()); return result == KMIP_OK; @@ -338,10 +335,6 @@ context::op_locate_by_group (context::name_t group) a[1].type = KMIP_ATTR_OBJECT_GROUP; a[1].value = &ts2; - TemplateAttribute ta = { 0 }; - ta.attributes = a; - ta.attribute_count = ARRAY_LENGTH (a); - int upto = 0; int all = 1; // TMP ids_t ret; @@ -399,9 +392,6 @@ context::op_locate_secrets_by_group (context::name_t group) a[1].type = KMIP_ATTR_OBJECT_GROUP; a[1].value = &ts2; - TemplateAttribute ta = { 0 }; - ta.attributes = a; - ta.attribute_count = ARRAY_LENGTH (a); int upto = 0; int all = 1; // TMP