From 977c0e726a671b421b6b1f3e6ab9337b67f61fe6 Mon Sep 17 00:00:00 2001 From: NagyZoltanPeter <113987313+NagyZoltanPeter@users.noreply.github.com> Date: Wed, 25 Feb 2026 12:15:00 +0100 Subject: [PATCH 1/4] creatNode securty enhancements. Prevent multi thread creatNode call, prevent multi ininitialization. Fix creatNode callback handling for fine resource handling --- delivery_module_plugin.cpp | 104 +++++++++++++++++++++++++------------ delivery_module_plugin.h | 2 + vendor/logos-delivery | 2 +- 3 files changed, 75 insertions(+), 33 deletions(-) diff --git a/delivery_module_plugin.cpp b/delivery_module_plugin.cpp index 3cbb636..1c9c9b5 100644 --- a/delivery_module_plugin.cpp +++ b/delivery_module_plugin.cpp @@ -4,7 +4,10 @@ #include #include #include +#include +#include #include +#include #include "api_call_handler.h" // Include the liblogosdelivery header from logos-delivery @@ -116,61 +119,98 @@ void DeliveryModulePlugin::initLogos(LogosAPI* logosAPIInstance) { bool DeliveryModulePlugin::createNode(const QString &cfg) { + std::lock_guard createNodeLock(createNodeMutex); + + if (deliveryCtx != nullptr) { + qWarning() << "DeliveryModulePlugin: createNode rejected - context already initialized"; + return false; + } + qDebug() << "DeliveryModulePlugin::createNode called with cfg:" << cfg; // Convert QString to UTF-8 byte array QByteArray cfgUtf8 = cfg.toUtf8(); - // Create semaphore and callback context for synchronous operation - // Callback is only called in failure case + // Create callback context for synchronous createNode result. + // The context is kept in a pending map so late callbacks can be safely ignored. struct CallbackContext { - std::binary_semaphore* sem; - bool callbackInvoked; + std::binary_semaphore sem{0}; + int callerRet{RET_ERR}; + QString message; }; + + static std::mutex pendingMutex; + static std::unordered_map> pendingContexts; + + auto callbackCtx = std::make_shared(); + void* callbackKey = static_cast(callbackCtx.get()); + + { + std::lock_guard lock(pendingMutex); + pendingContexts[callbackKey] = callbackCtx; + } - std::binary_semaphore sem(0); - CallbackContext ctx{&sem, false}; - - // Lambda callback that will be called only on failure (when deliveryCtx is nullptr) + // Callback is expected in both success and error cases. auto callback = +[](int callerRet, const char* msg, size_t len, void* userData) { qDebug() << "DeliveryModulePlugin::createNode callback called with ret:" << callerRet; - - CallbackContext* ctx = static_cast(userData); - if (!ctx) { - qWarning() << "DeliveryModulePlugin::createNode callback: Invalid userData"; + + std::shared_ptr callbackCtx; + { + std::lock_guard lock(pendingMutex); + auto it = pendingContexts.find(userData); + if (it == pendingContexts.end()) { + return; + } + callbackCtx = it->second; + pendingContexts.erase(it); + } + + if (!callbackCtx) { return; } - + + callbackCtx->callerRet = callerRet; if (msg && len > 0) { - QString message = QString::fromUtf8(msg, len); - qDebug() << "DeliveryModulePlugin::createNode callback message:" << message; + callbackCtx->message = QString::fromUtf8(msg, len); + qDebug() << "DeliveryModulePlugin::createNode callback message:" << callbackCtx->message; } - - ctx->callbackInvoked = true; - + // Release semaphore to unblock the createNode method - ctx->sem->release(); + callbackCtx->sem.release(); }; // Call logosdelivery_create_node with the configuration - // Important: Keep deliveryCtx assignment from the call - deliveryCtx = logosdelivery_create_node(cfgUtf8.constData(), callback, &ctx); - - // If deliveryCtx is nullptr, callback will be invoked with error details - if (!deliveryCtx) { - qDebug() << "DeliveryModulePlugin: Waiting for createNode error callback..."; - - // Wait for callback to complete with timeout - if (!sem.try_acquire_for(CALLBACK_TIMEOUT)) { - qWarning() << "DeliveryModulePlugin: Timeout waiting for createNode callback"; - return false; + // Important: Keep deliveryCtx assignment from the call, + // creating of the context is immediate and not depends on the callback. + deliveryCtx = logosdelivery_create_node(cfgUtf8.constData(), callback, callbackKey); + + qDebug() << "DeliveryModulePlugin: Waiting for createNode callback..."; + + // Wait for callback result regardless of immediate pointer value. + // Callback ensures that the underlying node object is properly created. + if (!callbackCtx->sem.try_acquire_for(CALLBACK_TIMEOUT)) { + std::lock_guard lock(pendingMutex); + pendingContexts.erase(callbackKey); + + deliveryCtx = nullptr; + + qWarning() << "DeliveryModulePlugin: Timeout waiting for createNode callback"; + return false; + } + + // Any issue happened during node creation means the context is destroyed and must not be user. + if (callbackCtx->callerRet != RET_OK || deliveryCtx == nullptr) { + if (!callbackCtx->message.isEmpty()) { + qWarning() << "DeliveryModulePlugin: createNode callback error:" << callbackCtx->message; } - + + deliveryCtx = nullptr; + qWarning() << "DeliveryModulePlugin: Failed to create Messaging context"; return false; } - // Success case - deliveryCtx is valid, callback won't be called + // Success case - deliveryCtx is valid and callback returned RET_OK. qDebug() << "DeliveryModulePlugin: Messaging context created successfully"; // Set up event callback diff --git a/delivery_module_plugin.h b/delivery_module_plugin.h index 8ffc9eb..d69d26f 100644 --- a/delivery_module_plugin.h +++ b/delivery_module_plugin.h @@ -2,6 +2,7 @@ #include #include +#include #include "delivery_module_interface.h" #include "logos_api.h" #include "logos_api_client.h" @@ -34,6 +35,7 @@ class DeliveryModulePlugin : public QObject, public DeliveryModuleInterface private: void* deliveryCtx; + std::mutex createNodeMutex; // Timeout for callback operations static constexpr std::chrono::seconds CALLBACK_TIMEOUT{30}; diff --git a/vendor/logos-delivery b/vendor/logos-delivery index a7872d5..8e41a27 160000 --- a/vendor/logos-delivery +++ b/vendor/logos-delivery @@ -1 +1 @@ -Subproject commit a7872d59d1991c7f726c7845d8ac536c6bdf7fb4 +Subproject commit 8e41a27ad29fb18cc4ee7d6e8d3afa9e061a4492 From 9b48a43ccd58689531d5df684aaa439bc893e032 Mon Sep 17 00:00:00 2001 From: NagyZoltanPeter <113987313+NagyZoltanPeter@users.noreply.github.com> Date: Wed, 25 Feb 2026 22:45:27 +0100 Subject: [PATCH 2/4] move sources under ./src --- CMakeLists.txt | 8 ++++---- QExpected.h => src/QExpected.h | 0 api_call_handler.h => src/api_call_handler.h | 0 .../delivery_module_interface.h | 0 .../delivery_module_plugin.cpp | 0 delivery_module_plugin.h => src/delivery_module_plugin.h | 2 +- 6 files changed, 5 insertions(+), 5 deletions(-) rename QExpected.h => src/QExpected.h (100%) rename api_call_handler.h => src/api_call_handler.h (100%) rename delivery_module_interface.h => src/delivery_module_interface.h (100%) rename delivery_module_plugin.cpp => src/delivery_module_plugin.cpp (100%) rename delivery_module_plugin.h => src/delivery_module_plugin.h (95%) diff --git a/CMakeLists.txt b/CMakeLists.txt index c1f1eb3..ddd04fd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -142,9 +142,9 @@ find_library(LIBLOGOSDELIVERY_PATH NAMES ${LIBLOGOSDELIVERY_NAMES} PATHS ${LIBLO # Plugin sources set(PLUGIN_SOURCES - delivery_module_plugin.cpp - delivery_module_plugin.h - delivery_module_interface.h + src/delivery_module_plugin.cpp + src/delivery_module_plugin.h + src/delivery_module_interface.h ) # Add liblogos interface header @@ -203,7 +203,7 @@ endif() # Include directories target_include_directories(delivery_module_plugin PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/src ${LIBLOGOSDELIVERY_DIR} ) diff --git a/QExpected.h b/src/QExpected.h similarity index 100% rename from QExpected.h rename to src/QExpected.h diff --git a/api_call_handler.h b/src/api_call_handler.h similarity index 100% rename from api_call_handler.h rename to src/api_call_handler.h diff --git a/delivery_module_interface.h b/src/delivery_module_interface.h similarity index 100% rename from delivery_module_interface.h rename to src/delivery_module_interface.h diff --git a/delivery_module_plugin.cpp b/src/delivery_module_plugin.cpp similarity index 100% rename from delivery_module_plugin.cpp rename to src/delivery_module_plugin.cpp diff --git a/delivery_module_plugin.h b/src/delivery_module_plugin.h similarity index 95% rename from delivery_module_plugin.h rename to src/delivery_module_plugin.h index d69d26f..970914c 100644 --- a/delivery_module_plugin.h +++ b/src/delivery_module_plugin.h @@ -10,7 +10,7 @@ class DeliveryModulePlugin : public QObject, public DeliveryModuleInterface { Q_OBJECT - Q_PLUGIN_METADATA(IID DeliveryModuleInterface_iid FILE "metadata.json") + Q_PLUGIN_METADATA(IID DeliveryModuleInterface_iid FILE "../metadata.json") Q_INTERFACES(DeliveryModuleInterface PluginInterface) public: From 70cad4b7ac56fd7c57df12a7f89e86c6a36e4b8a Mon Sep 17 00:00:00 2001 From: NagyZoltanPeter <113987313+NagyZoltanPeter@users.noreply.github.com> Date: Thu, 26 Feb 2026 15:30:30 +0100 Subject: [PATCH 3/4] module test initiative --- CMakeLists.txt | 79 +++++++++++++ nix/default.nix | 1 + nix/lib.nix | 7 ++ test/test_create_node_integration.cpp | 152 ++++++++++++++++++++++++++ 4 files changed, 239 insertions(+) create mode 100644 test/test_create_node_integration.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index ddd04fd..dba10a2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -300,3 +300,82 @@ install(DIRECTORY "${PLUGINS_OUTPUT_DIR}/" DESTINATION ${CMAKE_INSTALL_DATADIR}/logos-delivery-module/generated OPTIONAL ) + +include(CTest) + +if(BUILD_TESTING) + find_package(Qt${QT_VERSION_MAJOR} REQUIRED COMPONENTS Test) + + add_executable(delivery_module_plugin_integration_test + test/test_create_node_integration.cpp + src/delivery_module_plugin.cpp + src/delivery_module_plugin.h + src/delivery_module_interface.h + src/api_call_handler.h + src/QExpected.h + ) + + target_include_directories(delivery_module_plugin_integration_test PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/src + ) + + if(EXISTS "${LOGOS_DELIVERY_ROOT}/include/liblogosdelivery.h") + target_include_directories(delivery_module_plugin_integration_test PRIVATE ${LOGOS_DELIVERY_ROOT}/include) + elseif(EXISTS "${LOGOS_DELIVERY_ROOT}/liblogosdelivery/liblogosdelivery.h") + target_include_directories(delivery_module_plugin_integration_test PRIVATE ${LOGOS_DELIVERY_ROOT}/liblogosdelivery) + endif() + + if(_liblogos_is_source) + target_include_directories(delivery_module_plugin_integration_test PRIVATE ${LOGOS_LIBLOGOS_ROOT}) + else() + target_include_directories(delivery_module_plugin_integration_test PRIVATE ${LOGOS_LIBLOGOS_ROOT}/include) + endif() + + if(_cpp_sdk_is_source) + target_include_directories(delivery_module_plugin_integration_test PRIVATE + ${LOGOS_CPP_SDK_ROOT}/cpp + ${LOGOS_CPP_SDK_ROOT}/cpp/generated + ) + else() + target_include_directories(delivery_module_plugin_integration_test PRIVATE + ${LOGOS_CPP_SDK_ROOT}/include + ${LOGOS_CPP_SDK_ROOT}/include/cpp + ${LOGOS_CPP_SDK_ROOT}/include/core + ) + endif() + + target_link_libraries(delivery_module_plugin_integration_test PRIVATE + Qt${QT_VERSION_MAJOR}::Core + Qt${QT_VERSION_MAJOR}::RemoteObjects + Qt${QT_VERSION_MAJOR}::Test + ) + + if(NOT _cpp_sdk_is_source) + target_link_libraries(delivery_module_plugin_integration_test PRIVATE ${LOGOS_SDK_LIB}) + else() + target_sources(delivery_module_plugin_integration_test PRIVATE + ${LOGOS_CPP_SDK_ROOT}/cpp/logos_api.cpp + ${LOGOS_CPP_SDK_ROOT}/cpp/logos_api_client.cpp + ${LOGOS_CPP_SDK_ROOT}/cpp/logos_api_consumer.cpp + ${LOGOS_CPP_SDK_ROOT}/cpp/logos_api_provider.cpp + ${LOGOS_CPP_SDK_ROOT}/cpp/token_manager.cpp + ${LOGOS_CPP_SDK_ROOT}/cpp/module_proxy.cpp + ) + endif() + + if(LIBLOGOSDELIVERY_PATH) + target_link_libraries(delivery_module_plugin_integration_test PRIVATE ${LIBLOGOSDELIVERY_PATH}) + + if(APPLE) + add_custom_command(TARGET delivery_module_plugin_integration_test POST_BUILD + COMMAND ${CMAKE_COMMAND} + -DTEST_BIN=$ + -DREAL_LIB=${LIBLOGOSDELIVERY_PATH} + -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/fix_liblogosdelivery_id.cmake + COMMENT "Rewriting liblogosdelivery install name for integration test runtime" + ) + endif() + endif() + + add_test(NAME delivery_module_plugin_integration_test COMMAND delivery_module_plugin_integration_test) +endif() diff --git a/nix/default.nix b/nix/default.nix index db444ed..1f78980 100644 --- a/nix/default.nix +++ b/nix/default.nix @@ -23,6 +23,7 @@ # Common CMake flags cmakeFlags = [ "-GNinja" + "-DBUILD_TESTING=ON" "-DLOGOS_CPP_SDK_ROOT=${logosSdk}" "-DLOGOS_LIBLOGOS_ROOT=${logosLiblogos}" "-DLOGOS_DELIVERY_ROOT=${logosDelivery}" diff --git a/nix/lib.nix b/nix/lib.nix index ddee49c..443462b 100644 --- a/nix/lib.nix +++ b/nix/lib.nix @@ -8,6 +8,13 @@ pkgs.stdenv.mkDerivation { inherit src; inherit (common) nativeBuildInputs buildInputs cmakeFlags meta env; + doCheck = true; + checkPhase = '' + runHook preCheck + ctest --output-on-failure + runHook postCheck + ''; + # Determine platform-specific library extension libdeliveryLib = if pkgs.stdenv.hostPlatform.isDarwin then "liblogosdelivery.dylib" else "liblogosdelivery.so"; libpqPattern = if pkgs.stdenv.hostPlatform.isDarwin then "libpq*.dylib" else "libpq.so*"; diff --git a/test/test_create_node_integration.cpp b/test/test_create_node_integration.cpp new file mode 100644 index 0000000..e206a55 --- /dev/null +++ b/test/test_create_node_integration.cpp @@ -0,0 +1,152 @@ +#include +#include +#include + +#include "delivery_module_plugin.h" + +class DeliveryModulePluginCreateNodeIntegrationTest : public QObject +{ + Q_OBJECT + +private: + static const QString kDefaultConfig; + static const QString kEdgeConfig; + static const QString kCoreConfig; + + static bool hasLibpqDependency() + { + const QStringList candidates = { + QStringLiteral("libpq"), + QStringLiteral("libpq.dylib"), + QStringLiteral("pq") + }; + + for (const QString& candidate : candidates) { + QLibrary library(candidate); + if (library.load()) { + library.unload(); + return true; + } + } + + return false; + } + +private slots: + void createNode_withDefaultConfig_succeedsOrSkips(); + void createNode_withEdgeConfig_succeedsOrSkips(); + void createNode_withCoreConfig_succeedsOrSkips(); + void createNode_secondCallRejected_afterSuccessfulInit(); + void startStop_withRealBackend_succeedsOrSkips(); +}; + +const QString DeliveryModulePluginCreateNodeIntegrationTest::kDefaultConfig = QStringLiteral("{}"); + +const QString DeliveryModulePluginCreateNodeIntegrationTest::kEdgeConfig = QStringLiteral(R"JSON({ + "mode": "Edge", + "protocolsConfig": { + "entryNodes": [], + "clusterId": 1, + "messageValidation": { + "maxMessageSize": "150 KiB", + "rlnConfig": null + } + }, + "networkingConfig": { + "listenIpv4": "127.0.0.1", + "p2pTcpPort": 61000, + "discv5UdpPort": 61001 + }, + "ethRpcEndpoints": [], + "p2pReliability": false, + "logLevel": "INFO", + "logFormat": "TEXT" +})JSON"); + +const QString DeliveryModulePluginCreateNodeIntegrationTest::kCoreConfig = QStringLiteral(R"JSON({ + "mode": "Core", + "protocolsConfig": { + "entryNodes": [], + "clusterId": 1, + "autoShardingConfig": { + "numShardsInCluster": 1 + }, + "messageValidation": { + "maxMessageSize": "150 KiB", + "rlnConfig": null + } + }, + "networkingConfig": { + "listenIpv4": "127.0.0.1", + "p2pTcpPort": 62000, + "discv5UdpPort": 62001 + }, + "ethRpcEndpoints": [], + "p2pReliability": false, + "logLevel": "INFO", + "logFormat": "TEXT" + })JSON"); + +void DeliveryModulePluginCreateNodeIntegrationTest::createNode_withDefaultConfig_succeedsOrSkips() +{ + if (!hasLibpqDependency()) { + QSKIP("libpq dynamic library is not available; skipping logosdelivery integration test."); + } + + DeliveryModulePlugin plugin; + + QVERIFY(plugin.createNode(kDefaultConfig)); +} + +void DeliveryModulePluginCreateNodeIntegrationTest::createNode_withEdgeConfig_succeedsOrSkips() +{ + if (!hasLibpqDependency()) { + QSKIP("libpq dynamic library is not available; skipping logosdelivery integration test."); + } + + DeliveryModulePlugin plugin; + + QVERIFY(plugin.createNode(kEdgeConfig)); +} + +void DeliveryModulePluginCreateNodeIntegrationTest::createNode_withCoreConfig_succeedsOrSkips() +{ + if (!hasLibpqDependency()) { + QSKIP("libpq dynamic library is not available; skipping logosdelivery integration test."); + } + + DeliveryModulePlugin plugin; + QElapsedTimer timer; + timer.start(); + + QVERIFY(plugin.createNode(kCoreConfig)); +} + +void DeliveryModulePluginCreateNodeIntegrationTest::createNode_secondCallRejected_afterSuccessfulInit() +{ + if (!hasLibpqDependency()) { + QSKIP("libpq dynamic library is not available; skipping logosdelivery integration test."); + } + + DeliveryModulePlugin plugin; + QVERIFY(plugin.createNode(kEdgeConfig)); + + QVERIFY(!plugin.createNode(kEdgeConfig)); +} + +void DeliveryModulePluginCreateNodeIntegrationTest::startStop_withRealBackend_succeedsOrSkips() +{ + if (!hasLibpqDependency()) { + QSKIP("libpq dynamic library is not available; skipping logosdelivery integration test."); + } + + DeliveryModulePlugin plugin; + QVERIFY(plugin.createNode(kEdgeConfig)); + + QVERIFY(plugin.start()); + + QVERIFY(plugin.stop()); +} + +QTEST_MAIN(DeliveryModulePluginCreateNodeIntegrationTest) +#include "test_create_node_integration.moc" From fb2d3b7651fd5bf9b324a06d660145fd9333dd31 Mon Sep 17 00:00:00 2001 From: NagyZoltanPeter <113987313+NagyZoltanPeter@users.noreply.github.com> Date: Thu, 26 Feb 2026 15:58:21 +0100 Subject: [PATCH 4/4] update flake lock for corret logos-delivery --- flake.lock | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/flake.lock b/flake.lock index 8ecbbbf..08392c7 100644 --- a/flake.lock +++ b/flake.lock @@ -122,14 +122,12 @@ "zerokit": "zerokit" }, "locked": { - "lastModified": 1771543191, - "narHash": "sha256-gvPc2Q28zSm3KffJZCRjQ+bT6UGpqYPynMvbC/MB1gs=", - "ref": "refs/heads/master", - "rev": "a7872d59d1991c7f726c7845d8ac536c6bdf7fb4", - "revCount": 2225, - "submodules": true, - "type": "git", - "url": "https://github.com/logos-messaging/logos-delivery" + "lastModified": 1772017969, + "narHash": "sha256-bOaVBZz+3OpmSM+p3421VsmOekZi9+w7ybwXuMcscF4=", + "owner": "logos-messaging", + "repo": "logos-delivery", + "rev": "8e41a27ad29fb18cc4ee7d6e8d3afa9e061a4492", + "type": "github" }, "original": { "submodules": true,