diff --git a/contract-tests/client-contract-tests/src/main.cpp b/contract-tests/client-contract-tests/src/main.cpp index 7b5a878ae..ae7c42446 100644 --- a/contract-tests/client-contract-tests/src/main.cpp +++ b/contract-tests/client-contract-tests/src/main.cpp @@ -47,6 +47,7 @@ int main(int argc, char* argv[]) { 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 diff --git a/libs/client-sdk/src/client_impl.cpp b/libs/client-sdk/src/client_impl.cpp index 374f92e57..82a09db35 100644 --- a/libs/client-sdk/src/client_impl.cpp +++ b/libs/client-sdk/src/client_impl.cpp @@ -247,7 +247,8 @@ template EvaluationDetail ClientImpl::VariationInternal(FlagKey const& key, Value default_value, bool check_type, - bool detailed) { + bool detailed, + std::unordered_set* visited) { auto desc = flag_manager_.Store().Get(key); events::FeatureEventParams event = { @@ -308,19 +309,35 @@ EvaluationDetail ClientImpl::VariationInternal(FlagKey const& key, // 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 current path so a cyclic prerequisite graph + // terminates instead of recursing without bound. + // // 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()) { + // refactoring the code below. + auto const prereqs = flag.Prerequisites(); + if (prereqs && !prereqs->empty()) { + std::unordered_set local_visited; + auto* ancestors = visited ? visited : &local_visited; + ancestors->insert(key); for (auto const& prereq : *prereqs) { - JsonVariation(prereq, Value::Null()); + 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. + LD_LOG(logger_, LogLevel::kWarn) + << "prerequisite cycle detected: skipping edge from '" + << key << "' to '" << prereq << "'"; + continue; + } + (void)VariationInternal(prereq, Value::Null(), + /*check_type=*/false, + /*detailed=*/false, ancestors); } + ancestors->erase(key); } if (check_type && default_value.Type() != Value::Type::kNull && diff --git a/libs/client-sdk/src/client_impl.hpp b/libs/client-sdk/src/client_impl.hpp index 78142e347..ac03a4ae3 100644 --- a/libs/client-sdk/src/client_impl.hpp +++ b/libs/client-sdk/src/client_impl.hpp @@ -23,7 +23,9 @@ #include #include #include +#include #include +#include namespace launchdarkly::client_side { class ClientImpl : public IClient { @@ -93,7 +95,8 @@ class ClientImpl : public IClient { [[nodiscard]] EvaluationDetail VariationInternal(FlagKey const& key, Value default_value, bool check_type, - bool detailed); + bool detailed, + std::unordered_set* visited = nullptr); void TrackInternal(std::string event_name, std::optional data, std::optional metric_value);