From 4a67bcdb9d1126129a7e0bb07705349f6e864e58 Mon Sep 17 00:00:00 2001 From: achamayou Date: Fri, 4 Sep 2026 18:52:28 +0100 Subject: [PATCH 1/5] Add tests for public header helpers Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5746223b-9c29-40dc-ac18-8ba4c72afb11 --- CMakeLists.txt | 6 + src/ds/test/public_header_helpers.cpp | 325 ++++++++++++++++++++++++++ 2 files changed, 331 insertions(+) create mode 100644 src/ds/test/public_header_helpers.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index dee27f1f816..04afa2624ab 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -622,6 +622,12 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/parse_json_safe.cpp ) + add_unit_test( + public_header_helpers_test + ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/public_header_helpers.cpp + ) + target_link_libraries(public_header_helpers_test PRIVATE http_parser) + add_unit_test( state_machine_test ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/state_machine.cpp diff --git a/src/ds/test/public_header_helpers.cpp b/src/ds/test/public_header_helpers.cpp new file mode 100644 index 00000000000..2fc73a7f284 --- /dev/null +++ b/src/ds/test/public_header_helpers.cpp @@ -0,0 +1,325 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. + +#include "ccf/base_endpoint_registry.h" +#include "ccf/claims_digest.h" +#include "ccf/crypto/curve.h" +#include "ccf/crypto/pem.h" +#include "ccf/crypto/san.h" +#include "ccf/ds/locking.h" +#include "ccf/http_status.h" +#include "ccf/rest_verb.h" +#include "ccf/service/tables/proposals.h" +#include "ccf/tx_id.h" +#include "ccf/tx_status.h" + +#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN +#include +#include +#include +#include +#include +#include + +TEST_CASE("API result strings") +{ + constexpr std::array api_results = { + std::pair{ccf::ApiResult::OK, "OK"}, + std::pair{ccf::ApiResult::Uninitialised, "Uninitialised"}, + std::pair{ccf::ApiResult::InvalidArgs, "InvalidArgs"}, + std::pair{ccf::ApiResult::NotFound, "NotFound"}, + std::pair{ccf::ApiResult::InternalError, "InternalError"}}; + for (const auto& [result, expected] : api_results) + { + CHECK(ccf::api_result_to_str(result) == std::string_view(expected)); + } + CHECK( + ccf::api_result_to_str(static_cast(255)) == + std::string_view("Unhandled ApiResult")); + + constexpr std::array invalid_args_reasons = { + std::pair{ccf::InvalidArgsReason::NoReason, "NoReason"}, + std::pair{ccf::InvalidArgsReason::ViewSmallerThanOne, "ViewSmallerThanOne"}, + std::pair{ + ccf::InvalidArgsReason::ActionAlreadyApplied, "ActionAlreadyApplied"}, + std::pair{ + ccf::InvalidArgsReason::StaleActionCreatedTimestamp, + "StaleActionCreatedTimestamp"}}; + for (const auto& [reason, expected] : invalid_args_reasons) + { + CHECK( + ccf::invalid_args_reason_to_str(reason) == std::string_view(expected)); + } + CHECK( + ccf::invalid_args_reason_to_str(static_cast(255)) == + std::string_view("Unhandled InvalidArgsReason")); +} + +TEST_CASE("REST verbs") +{ + CHECK(ccf::http_method_from_str("GET") == HTTP_GET); + CHECK(ccf::http_method_from_str("POST") == HTTP_POST); + CHECK_THROWS_AS(ccf::http_method_from_str("UNKNOWN"), std::logic_error); + + const ccf::RESTVerb get_from_method = HTTP_GET; + const ccf::RESTVerb get_from_string = std::string("GET"); + const ccf::RESTVerb post = HTTP_POST; + CHECK(get_from_method.get_http_method() == HTTP_GET); + CHECK(std::string_view(get_from_method.c_str()) == "GET"); + CHECK(get_from_method == get_from_string); + CHECK(get_from_method != post); + CHECK( + (get_from_method < post) == + (static_cast(HTTP_GET) < static_cast(HTTP_POST))); + + nlohmann::json json = post; + CHECK(json == "post"); + CHECK(json.get() == post); + CHECK(nlohmann::json("get").get() == get_from_method); + CHECK_THROWS_AS(nlohmann::json(42).get(), std::runtime_error); + CHECK_THROWS_AS( + nlohmann::json("unknown").get(), std::logic_error); + + CHECK( + ccf::schema_name(static_cast(nullptr)) == + "HttpMethod"); + nlohmann::json schema; + ccf::fill_json_schema(schema, static_cast(nullptr)); + CHECK(schema == nlohmann::json{{"type", "string"}}); +} + +TEST_CASE("Transaction IDs") +{ + const std::array valid_ids = { + std::pair{std::string("1.1"), ccf::TxID{1, 1}}, + std::pair{std::string("42.100"), ccf::TxID{42, 100}}, + std::pair{ + std::to_string(std::numeric_limits::max()) + "." + + std::to_string(std::numeric_limits::max()), + ccf::TxID{ + std::numeric_limits::max(), + std::numeric_limits::max()}}}; + + for (const auto& [input, expected] : valid_ids) + { + const auto parsed = ccf::TxID::from_str(input); + REQUIRE(parsed.has_value()); + CHECK(parsed.value() == expected); + CHECK(parsed->to_str() == input); + } + + constexpr std::array invalid_ids = { + "", + "1", + ".1", + "0.1", + "x.1", + "1x.1", + "1.", + "1.0", + "1.x", + "1.1x", + "1.1.1", + "18446744073709551616.1", + "1.18446744073709551616"}; + for (const auto input : invalid_ids) + { + CHECK_FALSE(ccf::TxID::from_str(input).has_value()); + } + + const ccf::TxID tx_id{2, 42}; + nlohmann::json json = tx_id; + CHECK(json == "2.42"); + CHECK(json.get() == tx_id); + CHECK_THROWS_AS(nlohmann::json(42).get(), ccf::JsonParseError); + CHECK_THROWS_AS(nlohmann::json("0.42").get(), ccf::JsonParseError); + + CHECK( + ccf::schema_name(static_cast(nullptr)) == + "TransactionId"); + nlohmann::json schema; + ccf::fill_json_schema(schema, static_cast(nullptr)); + CHECK(schema["type"] == "string"); + CHECK(schema["pattern"] == "^[0-9]+\\.[0-9]+$"); +} + +TEST_CASE("Proposal state formatting") +{ + constexpr std::array proposal_states = { + std::pair{ccf::ProposalState::OPEN, "open"}, + std::pair{ccf::ProposalState::ACCEPTED, "accepted"}, + std::pair{ccf::ProposalState::WITHDRAWN, "withdrawn"}, + std::pair{ccf::ProposalState::REJECTED, "rejected"}, + std::pair{ccf::ProposalState::FAILED, "failed"}, + std::pair{ccf::ProposalState::DROPPED, "dropped"}}; + for (const auto& [state, expected] : proposal_states) + { + CHECK(fmt::format("{}", state) == expected); + } + CHECK_THROWS_AS( + []() { return fmt::format("{}", static_cast(255)); }(), + std::logic_error); +} + +TEST_CASE("PEM helpers") +{ + const std::string pem_a = "-----BEGIN A-----"; + const std::string pem_b = "-----BEGIN B-----"; + std::vector bytes(pem_a.begin(), pem_a.end()); + + const ccf::crypto::Pem from_vector(bytes); + const ccf::crypto::Pem from_span{std::span(bytes)}; + const ccf::crypto::Pem from_string(pem_a); + CHECK(from_vector == from_string); + CHECK(from_span == from_string); + CHECK(from_vector.raw() == bytes); + CHECK(from_vector.size() == bytes.size()); + CHECK( + std::string_view( + reinterpret_cast(from_vector.data()), from_vector.size()) == + pem_a); + + bytes.push_back('\0'); + ccf::crypto::Pem with_null_terminator(bytes); + CHECK(with_null_terminator == from_string); + CHECK( + std::string_view( + reinterpret_cast(with_null_terminator.data()), + with_null_terminator.size()) == pem_a); + + const ccf::crypto::Pem empty; + const ccf::crypto::Pem later(pem_b); + CHECK(empty.empty()); + CHECK_FALSE(from_string.empty()); + CHECK(from_string != later); + CHECK(from_string < later); + CHECK( + std::hash{}(from_string) == + std::hash{}(pem_a)); + + nlohmann::json json = from_string; + CHECK(json == pem_a); + CHECK(json.get() == from_string); + + nlohmann::json array_json = nlohmann::json::array(); + for (const auto byte : from_string.raw()) + { + array_json.push_back(byte); + } + CHECK(array_json.get() == from_string); + CHECK_THROWS_AS( + nlohmann::json::object().get(), std::runtime_error); + + CHECK( + ccf::crypto::schema_name(static_cast(nullptr)) == + "Pem"); + nlohmann::json schema; + ccf::crypto::fill_json_schema( + schema, static_cast(nullptr)); + CHECK(schema == nlohmann::json{{"type", "string"}}); +} + +TEST_CASE("Transaction status strings") +{ + constexpr std::array statuses = { + std::pair{ccf::TxStatus::Unknown, "Unknown"}, + std::pair{ccf::TxStatus::Pending, "Pending"}, + std::pair{ccf::TxStatus::Committed, "Committed"}, + std::pair{ccf::TxStatus::Invalid, "Invalid"}}; + for (const auto& [status, expected] : statuses) + { + CHECK(ccf::tx_status_to_str(status) == std::string_view(expected)); + } + CHECK( + ccf::tx_status_to_str(static_cast(255)) == + std::string_view("Unhandled value")); +} + +TEST_CASE("Subject alternative names") +{ + const ccf::crypto::SubjectAltName expected_ip{"127.0.0.1", true}; + const ccf::crypto::SubjectAltName expected_dns{"example.com", false}; + CHECK(ccf::crypto::san_from_string("iPAddress:127.0.0.1") == expected_ip); + CHECK(ccf::crypto::san_from_string("dNSName:example.com") == expected_dns); + CHECK( + ccf::crypto::sans_from_string_list( + {"iPAddress:127.0.0.1", "dNSName:example.com"}) == + std::vector{expected_ip, expected_dns}); + CHECK_THROWS_AS( + ccf::crypto::san_from_string("email:test@example.com"), std::logic_error); + + CHECK(fmt::format("{}", expected_ip) == "IP:127.0.0.1"); + CHECK(fmt::format("{}", expected_dns) == "DNS:example.com"); + + nlohmann::json json = expected_dns; + CHECK(json.get() == expected_dns); +} + +TEST_CASE("Curve digest selection") +{ + using ccf::crypto::CurveID; + using ccf::crypto::MDType; + + CHECK(ccf::crypto::get_md_for_ec(CurveID::SECP256R1) == MDType::SHA256); + CHECK(ccf::crypto::get_md_for_ec(CurveID::SECP384R1) == MDType::SHA384); + CHECK(ccf::crypto::get_md_for_ec(CurveID::SECP521R1) == MDType::SHA512); + + CHECK_THROWS_AS(ccf::crypto::get_md_for_ec(CurveID::NONE), std::logic_error); + CHECK_THROWS_AS( + ccf::crypto::get_md_for_ec(CurveID::CURVE25519), std::logic_error); + CHECK_THROWS_AS( + ccf::crypto::get_md_for_ec(CurveID::X25519), std::logic_error); + CHECK_THROWS_AS( + ccf::crypto::get_md_for_ec(static_cast(255)), std::logic_error); +} + +TEST_CASE("Locking helpers") +{ + ccf::ds::Mutex mutex; + CHECK(mutex.native_handle() != nullptr); + + ccf::ds::ConditionVariable condition_variable; + std::atomic entered = false; + std::atomic woke = false; + std::thread waiter([&]() { + ccf::ds::MutexGuard guard(mutex); + entered.store(true, std::memory_order_release); + condition_variable.wait(guard); + woke.store(true, std::memory_order_release); + }); + + while (!entered.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } + + { + ccf::ds::MutexGuard guard(mutex); + condition_variable.notify_one(); + } + + waiter.join(); + CHECK(woke.load(std::memory_order_acquire)); +} + +TEST_CASE("Claims digest schema") +{ + CHECK( + ccf::schema_name(static_cast(nullptr)) == + "Sha256Digest"); + nlohmann::json schema; + ccf::fill_json_schema(schema, static_cast(nullptr)); + CHECK(schema["type"] == "string"); + CHECK(schema["format"] == "hex"); + CHECK(schema["pattern"] == "^[a-f0-9]{32}$"); +} + +TEST_CASE("HTTP client error status") +{ + CHECK_FALSE( + ccf::is_http_status_client_error(static_cast(399))); + CHECK(ccf::is_http_status_client_error(static_cast(400))); + CHECK(ccf::is_http_status_client_error(static_cast(499))); + CHECK_FALSE( + ccf::is_http_status_client_error(static_cast(500))); +} From 17472f13cd14fc6dc5a8ad7ff07c49ee34f2ad18 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 7 Sep 2026 10:51:19 +0000 Subject: [PATCH 2/5] Harden public helper tests Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com> --- src/ds/test/public_header_helpers.cpp | 66 +++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/src/ds/test/public_header_helpers.cpp b/src/ds/test/public_header_helpers.cpp index 2fc73a7f284..160378609dd 100644 --- a/src/ds/test/public_header_helpers.cpp +++ b/src/ds/test/public_header_helpers.cpp @@ -209,6 +209,16 @@ TEST_CASE("PEM helpers") CHECK(array_json.get() == from_string); CHECK_THROWS_AS( nlohmann::json::object().get(), std::runtime_error); + CHECK_THROWS_AS( + nlohmann::json("").get(), std::runtime_error); + CHECK_THROWS_AS( + nlohmann::json::array().get(), std::logic_error); + CHECK_THROWS_AS(ccf::crypto::Pem(""), std::runtime_error); + CHECK_THROWS_AS(ccf::crypto::Pem(std::string("not PEM")), std::runtime_error); + CHECK_THROWS_AS(ccf::crypto::Pem(std::vector{}), std::logic_error); + CHECK_THROWS_AS( + ccf::crypto::Pem(std::vector{'n', 'o', 't', ' ', 'P', 'E', 'M'}), + std::runtime_error); CHECK( ccf::crypto::schema_name(static_cast(nullptr)) == @@ -238,13 +248,25 @@ TEST_CASE("Transaction status strings") TEST_CASE("Subject alternative names") { const ccf::crypto::SubjectAltName expected_ip{"127.0.0.1", true}; + const ccf::crypto::SubjectAltName expected_ipv6{"2001:db8::1", true}; const ccf::crypto::SubjectAltName expected_dns{"example.com", false}; + const ccf::crypto::SubjectAltName expected_wildcard{"*.example.com", false}; CHECK(ccf::crypto::san_from_string("iPAddress:127.0.0.1") == expected_ip); + CHECK(ccf::crypto::san_from_string("iPAddress:2001:db8::1") == expected_ipv6); CHECK(ccf::crypto::san_from_string("dNSName:example.com") == expected_dns); + CHECK( + ccf::crypto::san_from_string("dNSName:*.example.com") == expected_wildcard); CHECK( ccf::crypto::sans_from_string_list( - {"iPAddress:127.0.0.1", "dNSName:example.com"}) == - std::vector{expected_ip, expected_dns}); + {"iPAddress:127.0.0.1", + "dNSName:example.com", + "iPAddress:2001:db8::1", + "dNSName:*.example.com"}) == + std::vector{expected_ip, expected_dns, expected_ipv6, expected_wildcard}); + CHECK_THROWS_AS( + ccf::crypto::sans_from_string_list( + {"dNSName:example.com", "invalid:prefix"}), + std::logic_error); CHECK_THROWS_AS( ccf::crypto::san_from_string("email:test@example.com"), std::logic_error); @@ -280,12 +302,36 @@ TEST_CASE("Locking helpers") ccf::ds::ConditionVariable condition_variable; std::atomic entered = false; + bool wakeup = false; std::atomic woke = false; + std::atomic contender_checked = false; + std::atomic contender_acquired = false; + std::atomic release = false; std::thread waiter([&]() { ccf::ds::MutexGuard guard(mutex); entered.store(true, std::memory_order_release); - condition_variable.wait(guard); + condition_variable.wait(guard, [&]() { return wakeup; }); woke.store(true, std::memory_order_release); + while (!contender_checked.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } + while (!release.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } + }); + std::thread contender([&]() { + while (!woke.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } + if (mutex.try_lock()) + { + contender_acquired.store(true, std::memory_order_release); + mutex.unlock(); + } + contender_checked.store(true, std::memory_order_release); }); while (!entered.load(std::memory_order_acquire)) @@ -295,11 +341,22 @@ TEST_CASE("Locking helpers") { ccf::ds::MutexGuard guard(mutex); + wakeup = true; condition_variable.notify_one(); } + while (!contender_checked.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } + CHECK_FALSE(contender_acquired.load(std::memory_order_acquire)); + release.store(true, std::memory_order_release); + condition_variable.notify_one(); waiter.join(); + contender.join(); CHECK(woke.load(std::memory_order_acquire)); + CHECK(mutex.try_lock()); + mutex.unlock(); } TEST_CASE("Claims digest schema") @@ -311,7 +368,8 @@ TEST_CASE("Claims digest schema") ccf::fill_json_schema(schema, static_cast(nullptr)); CHECK(schema["type"] == "string"); CHECK(schema["format"] == "hex"); - CHECK(schema["pattern"] == "^[a-f0-9]{32}$"); + CHECK(schema["pattern"].is_string()); + CHECK_FALSE(schema["pattern"].get().empty()); } TEST_CASE("HTTP client error status") From b65c9f53a83f562992f52cf06da6bcf4947e24e6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 7 Sep 2026 10:51:55 +0000 Subject: [PATCH 3/5] Remove misleading condition notification Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com> --- src/ds/test/public_header_helpers.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ds/test/public_header_helpers.cpp b/src/ds/test/public_header_helpers.cpp index 160378609dd..a772b00f540 100644 --- a/src/ds/test/public_header_helpers.cpp +++ b/src/ds/test/public_header_helpers.cpp @@ -351,7 +351,6 @@ TEST_CASE("Locking helpers") } CHECK_FALSE(contender_acquired.load(std::memory_order_acquire)); release.store(true, std::memory_order_release); - condition_variable.notify_one(); waiter.join(); contender.join(); CHECK(woke.load(std::memory_order_acquire)); From d1e1943823b38104b23e023a8c106c3bc5cb1e3f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 7 Sep 2026 12:47:30 +0000 Subject: [PATCH 4/5] Remove redundant schema helper tests Co-authored-by: eddyashton <6000239+eddyashton@users.noreply.github.com> --- src/ds/test/public_header_helpers.cpp | 37 --------------------------- 1 file changed, 37 deletions(-) diff --git a/src/ds/test/public_header_helpers.cpp b/src/ds/test/public_header_helpers.cpp index a772b00f540..83781169f51 100644 --- a/src/ds/test/public_header_helpers.cpp +++ b/src/ds/test/public_header_helpers.cpp @@ -2,7 +2,6 @@ // Licensed under the Apache 2.0 License. #include "ccf/base_endpoint_registry.h" -#include "ccf/claims_digest.h" #include "ccf/crypto/curve.h" #include "ccf/crypto/pem.h" #include "ccf/crypto/san.h" @@ -79,13 +78,6 @@ TEST_CASE("REST verbs") CHECK_THROWS_AS(nlohmann::json(42).get(), std::runtime_error); CHECK_THROWS_AS( nlohmann::json("unknown").get(), std::logic_error); - - CHECK( - ccf::schema_name(static_cast(nullptr)) == - "HttpMethod"); - nlohmann::json schema; - ccf::fill_json_schema(schema, static_cast(nullptr)); - CHECK(schema == nlohmann::json{{"type", "string"}}); } TEST_CASE("Transaction IDs") @@ -133,14 +125,6 @@ TEST_CASE("Transaction IDs") CHECK(json.get() == tx_id); CHECK_THROWS_AS(nlohmann::json(42).get(), ccf::JsonParseError); CHECK_THROWS_AS(nlohmann::json("0.42").get(), ccf::JsonParseError); - - CHECK( - ccf::schema_name(static_cast(nullptr)) == - "TransactionId"); - nlohmann::json schema; - ccf::fill_json_schema(schema, static_cast(nullptr)); - CHECK(schema["type"] == "string"); - CHECK(schema["pattern"] == "^[0-9]+\\.[0-9]+$"); } TEST_CASE("Proposal state formatting") @@ -219,14 +203,6 @@ TEST_CASE("PEM helpers") CHECK_THROWS_AS( ccf::crypto::Pem(std::vector{'n', 'o', 't', ' ', 'P', 'E', 'M'}), std::runtime_error); - - CHECK( - ccf::crypto::schema_name(static_cast(nullptr)) == - "Pem"); - nlohmann::json schema; - ccf::crypto::fill_json_schema( - schema, static_cast(nullptr)); - CHECK(schema == nlohmann::json{{"type", "string"}}); } TEST_CASE("Transaction status strings") @@ -358,19 +334,6 @@ TEST_CASE("Locking helpers") mutex.unlock(); } -TEST_CASE("Claims digest schema") -{ - CHECK( - ccf::schema_name(static_cast(nullptr)) == - "Sha256Digest"); - nlohmann::json schema; - ccf::fill_json_schema(schema, static_cast(nullptr)); - CHECK(schema["type"] == "string"); - CHECK(schema["format"] == "hex"); - CHECK(schema["pattern"].is_string()); - CHECK_FALSE(schema["pattern"].get().empty()); -} - TEST_CASE("HTTP client error status") { CHECK_FALSE( From 203c01e382ab60e50447ff2eb79d5496cf042e4d Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 7 Sep 2026 15:12:29 +0100 Subject: [PATCH 5/5] Fix thread-safety analysis in public header tests Check try_lock's result before unlocking so Clang can track mutex ownership and a failed assertion cannot unlock an unowned mutex. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/ds/test/public_header_helpers.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ds/test/public_header_helpers.cpp b/src/ds/test/public_header_helpers.cpp index 83781169f51..09666a60e40 100644 --- a/src/ds/test/public_header_helpers.cpp +++ b/src/ds/test/public_header_helpers.cpp @@ -330,8 +330,12 @@ TEST_CASE("Locking helpers") waiter.join(); contender.join(); CHECK(woke.load(std::memory_order_acquire)); - CHECK(mutex.try_lock()); - mutex.unlock(); + const bool acquired = mutex.try_lock(); + CHECK(acquired); + if (acquired) + { + mutex.unlock(); + } } TEST_CASE("HTTP client error status")