fix: add defensive cycle guard to prerequisite evaluation

contract-tests/client-contract-tests/src/main.cpp

@@ -1,4 +1,4 @@
 #include "server.hpp"
 
 #include <launchdarkly/logging/console_backend.hpp>
 
@@ -18,7 +18,7 @@
 using launchdarkly::LogLevel;
 
 int main(int argc, char* argv[]) {
     launchdarkly::Logger logger{
         std::make_unique<ConsoleBackend>("client-contract-tests")};
 
     std::string const default_port = "8123";
@@ -31,8 +31,8 @@
     try {
         net::io_context ioc{1};
 
         auto p = boost::lexical_cast<unsigned short>(port);
         server srv(ioc, "0.0.0.0", p, logger);
 
         srv.add_capability("client-side");
         srv.add_capability("mobile");
@@ -47,6 +47,7 @@
         srv.add_capability("tls:skip-verify-peer");
         srv.add_capability("tls:custom-ca");
         srv.add_capability("client-prereq-events");
+        srv.add_capability("client-prereq-cycle-detection");
         srv.add_capability("wrapper");
         // Proxies are supported only with CURL networking.
 #ifdef LD_CURL_NETWORKING

libs/client-sdk/src/client_impl.cpp

@@ -1,3 +1,3 @@
 #include "client_impl.hpp"
 #include "data_sources/null_data_source.hpp"
 #include "data_sources/polling_data_source.hpp"
@@ -211,7 +211,7 @@
     std::unordered_map<Client::FlagKey, Value> result;
     for (auto& [key, descriptor] : flag_manager_.Store().GetAll()) {
         if (descriptor->item) {
             result.try_emplace(key, descriptor->item->Detail().Value());
         }
     }
     return result;
@@ -244,10 +244,11 @@
 }
 
 template <typename T>
 EvaluationDetail<T> ClientImpl::VariationInternal(FlagKey const& key,
                                                   Value default_value,
                                                   bool check_type,
-                                                  bool detailed) {
+                                                  bool detailed,
+                                                  std::unordered_set<std::string>* visited) {
     auto desc = flag_manager_.Store().Get(key);
 
     events::FeatureEventParams event = {
@@ -300,7 +301,7 @@
 
     LD_ASSERT(desc->item);
 
     auto const& flag = *(desc->item);
     auto const& detail = flag.Detail();
 
     // The Prerequisites vector represents the evaluated prerequisites of
@@ -308,19 +309,41 @@
     // prerequisites (recursively), which is necessary to ensure LaunchDarkly
     // analytics functions properly.
     //
-    // We're using JsonVariation because the type of the
-    // prerequisite is both unknown and irrelevant to emitting the events.
-    //
-    // We're passing Value::Null() to match a server-side SDK's behavior when
-    // evaluating prerequisites.
+    // `visited` tracks the chain of prerequisite dependencies from the
+    // top-level evaluation to (but not including) the current flag. It is
+    // allocated lazily: variation calls on prereq-less flags allocate no
+    // set. Once created it is shared for the rest of the walk via
+    // insert-before-recurse / erase-after-recurse (guarded by a scope-exit
+    // erase so an exception below cannot leave a stale ancestor entry
+    // visible to a sibling branch). A prerequisite already in `visited`
+    // closes a cycle; descent is skipped and the loop continues with the
+    // remaining prerequisites at the current level.
     //
     // NOTE: if "hooks" functionality is implemented into this SDK, take care
     // that evaluating prerequisites does not trigger hooks. This may require
-    // refactoring the code below to not use JsonVariation.
-    if (auto const prereqs = flag.Prerequisites()) {
-        for (auto const& prereq : *prereqs) {
-            JsonVariation(prereq, Value::Null());
+    // refactoring the code below.
+    if (auto const prereqs = flag.Prerequisites();
+        prereqs && !prereqs->empty()) {
+        std::unordered_set<std::string> local_visited;
+        auto* ancestors = visited ? visited : &local_visited;
+        ancestors->insert(key);
+        try {
+            for (auto const& prereq : *prereqs) {
+                if (ancestors->count(prereq) != 0) {
+                    // Cyclic edge: skip descent, continue with remaining
+                    // prerequisites at this level. The requested flag's value
+                    // and reason (below) are unaffected.
+                    continue;
+                }
+                (void)VariationInternal<Value>(prereq, Value::Null(),
+                                               /*check_type=*/false,
+                                               /*detailed=*/false, ancestors);
+            }
+        } catch (...) {
+            ancestors->erase(key);
+            throw;
         }
+        ancestors->erase(key);
     }
 
     if (check_type && default_value.Type() != Value::Type::kNull &&

libs/client-sdk/src/client_impl.hpp

@@ -1,3 +1,3 @@
 #pragma once
 
 #include "data_sources/data_source.hpp"
@@ -23,7 +23,9 @@
 #include <memory>
 #include <optional>
 #include <shared_mutex>
+#include <string>
 #include <thread>
+#include <unordered_set>
 
 namespace launchdarkly::client_side {
 class ClientImpl : public IClient {
@@ -84,7 +86,7 @@
 
     flag_manager::IFlagNotifier& FlagNotifier() override;
 
     ~ClientImpl();
 
     std::future<bool> StartAsync() override;
 
@@ -93,13 +95,14 @@
     [[nodiscard]] EvaluationDetail<T> VariationInternal(FlagKey const& key,
                                                         Value default_value,
                                                         bool check_type,
-                                                        bool detailed);
+                                                        bool detailed,
+                                                        std::unordered_set<std::string>* visited = nullptr);
     void TrackInternal(std::string event_name,
                        std::optional<Value> data,
                        std::optional<double> metric_value);
 
     template <typename F>
     auto ReadContextSynchronized(F fn) const
         -> std::invoke_result_t<F, Context const&> {
         std::shared_lock lock(context_mutex_);
         return fn(context_);