prepare 2.8.3 release (#62)

LaunchDarklyReleaseBot · hroederld · bwoskow-ld · web-flow · commit 2a5d3f89c66d · 2022-08-09T10:52:09.000-07:00
* ensure subevents always get freed * cleanup leaks in error conditions and generally reduce allocations * remove allocation from parse path * use new detach function instead of duplicate * rename FeatureCollection to FeatureCollectionItem * convert ldstore interface to use static strings instead of enum * update streaming to rename flags to features * replace two instance of free with LDFree * replace two unrelated instances of malloc with LDAlloc * rename flags to features in polling update * DRY up with featureKindToString * more logically organize ldstore * update doxygen header for RC related functions * correctly return NULL when RC lock construction fails * add sanity check assertion that current reference count is > 0 when decrementing * make some routines static and some parameters const * replace free with LDFree in store destroy * add const to ptr for addHidden * rename and make jsonrc destruction static * makeFeatureCollection -> makeFeatureCollectionItem, replacementcollection -> replacementcollectionitem * 1.0.0 beta.1 (#8) prepare 1.0.0-beta.1 release * update readme to refer to better distinguish between server-side and client-side * Add jitter to networking backoff (#9) * Add jitter to network retry backoff * Add prng * Remove GMP dep (#12) * Add support for debugEventsUntilDate (#11) * Windows C++ compatibility (#13) * Window specific time parsing (#14) * Windows CI (#15) * Dynamic library symbol hygiene (#16) * Prepare 1.0.0 beta.2 (#17) * bump version variable * update changelog * add back ifdef cplusplus (#18) * Extra build documentation (#19) * add much more detailed build instructions * update PCRE url text * remove GMP from linux CI * reorganize headers (#21) * Fix doxygen and add docgen infra (#22) * cmake cleanup (#24) * add pedantic flag back * add back wall and wextra * make more idiomatic * Complete doxygen coverage (#23) * applying markdown templates (#25) * bump version and update changelog (#26) * free subevents as append does not transfer ownership (#27) * bump version and add memory leak info to changelog (#28) * Update to handle index events (#29) * switch to using milliseconds everywhere internally (#30) * adding a circleci badge to the readme (#31) * adding a circleci badge to the readme * fix casing * switch from checking for header end within size (usually one) to total (#32) * bump version constant, document changes, remove beta warning (#33) * add /usr/local/include for osx (#34) * bump version, append changelog (#35) * memset time struct (#36) * bump version, document changes (#37) * update type comparison to work with object OR array detection (#38) * Experimention track metric (#40) * prepare 1.1.0 * bump changelog date before release * Fix rule match explanation encoding (#42) * fix json serialization for rule match case * add this fix to the changelog (have not actually released yet) * Double to int variation tests, and document behavior. (#43) * [ch49806] redis store (#44) * [ch58854] Update cJSON to 1.7.12 * [ch57214] Store API v2 (#46) * [ch55939] add Releaser scripts, CircleCI Windows build, use custom Docker image (#45) * [ch59994] add code coverage flag * [ch43307] put user in last bucket for a bucketing fallthrough * [ch60038] dont assert on polling put failure * [ch54986] old osx clock gettime (#51) * [ch60053] fix redis race detection * [ch61092] Event payload ID. (#54) * [ch60589] Cache external store values (#53) * [ch61935] Releaser for redis (#55) * [ch69694] reconnect consistency (#57) * [ch69694] streaming status codes (#58) * [ch71379] create more ideal concurrency h (#59) * [ch72167] api level assert (#61) * [ch72078] remove higher stack assertions for system ops (#60) * [ch72307] optional asserts compilation (#62) * [ch72310] cmake cleanup (#63) * [ch72310] cleanup redis cmake * [ch72763] correct find package (#65) * [ch72838] detangle headers (#66) * [ch73235] enable mock concurrency (#68) * [ch72863] event processing cleanup and optimization (#67) * [ch73704] dedup test utils (#69) * [ch73828] null user key ban (#70) * [ch73828] null user key fix (#71) * [ch73236] detach sse (#72) * [ch73698] mock backend server and polling test (#73) * [ch74213] release bundle (#75) * [ch73853] start of defensive mode (#74) * [ch75034] enable and fix more warnings (#76) * [ch73853] add logging to existing defensive checks (#77) * [ch75137] double for large milliseconds (#85) * [ch75136] c89 fix for loop initialization * [ch75131] add time.h on posix * [ch75126] Fix event generation for NULL fallback * [ch73853] remove extra error channels * [ch73853] Defensive JSON (#78) * [ch73853] enable defensive api by default (#81) * [ch75201] custom bool type for public api (#86) * [ch74995] null fallback tests (#87) * [ch74995] basic stream mock (#88) * undelete docs directory (#91) * [ch76282] wrapper metadata config (#92) * [ch78853] Add 32 bit linux CI (#93) * [ch80280] usleep error (#94) * [ch80280] skip sleep on EINTR * [ch80617] c sdk common (#96) * [ch80617] add common headers to doxygen * [ch81023] accept header (#98) * [ch81049] add common/include to public header export * [ch81355] fix experimentation events (#100) * [ch94759] bump xcode to next circleci supported version * [ch94757] cleanup staticGetValue after use in test #101 * [ch94758] run valgrind memcheck in linux ci (#103) * [ch94776] fix helgrind warning #104 * [ch94759] releaser upgrade xcode version #105 * [ch95201] Update to use LDBasicLoggerThreadSafe in tests (#106) * [ch97247] more coverage and bug fixes (#107) * [ch97472] use Findhiredis (#108) * [ch99403] Add Alias method (#110) * [ch99260] Add O3 build and cleanup warnings (#109) * Update c-sdk-common (#111) * Removed the guides link * [ch100420] Hook in c-sdk-common LDUser (#112) * run clang format (#113) * Forgotten clang-format and C standard audit. (#114) * [ch109467] fix bucketBy (#115) * [ch111014] remove stdbool.h references (#117) * [ch111014] Ensure modern clang passes (#118) * [ch111482] fix OSX CI (#119) * [ch111482] fix osx releaser (#120) * merge experimentation allocation changes * [ch114396] improve flag / segment validation (#121) * ch121458 Ensure that LDAllFlags returns all valid flags in the case where some flags are malformed. * ch121458 Handle the case where there are not any flags in the store. * ch121458 Remove unused eval status. * ch121458 Run test before shutting down logger. * ch121458 Address style feedback. Space after if. * ch121458 Check that result is empty in malformed fallthrough case. * ch121458 Allow for the rollout kind to be empty. * ch121458 Summarize events with EVAL_SCHEMA errors. * ch121458 Re-enable tests. Fix formatting. * ch121458 Remove comment about updating test. * Remove duplicate line. * ch123139 Allow the c-server-sdk to build from any directory on windows. * Rlamb/ch122102/add unit testing framework (#123) * ch122102 Implement google test framework and start implementing tests. * ch122102 Migrate tests for eval, config, and the event-processor. * ch122102 Add events tests and update other tests with better assertions. * ch122102 Add tests for json, lru, and misc. * ch122101 Add offline and mock tests. * ch122102 Add operators, store backend, and segments tests. * ch122102 Add tests for streaming and variations. * ch122102 Implement stores tests for redis and memory. Rename files all back to start with test-. * ch122102 Allow CTest to discover google tests. * ch122102 Add missing include(GoogleTest) * ch122102 Re-order dependencies to ensure google-test present when needed. * ch122102 Resolve testing issue. * ch122102 Make it clear which stores the tests are running on. * ch122102 Add git dependency. * ch122102 Simplify conditional to please windows. * ch122102 Add missing alogrithm header from test-operators. * ch122102 Change clang C++ compiler * ch122102 Make google test use the same CRT * ch122102 Teardown threads after tearing down other resources. * ch122102 Revert change cause by over aggressive refactor command. * ch122102 Revert collateral files impacted by bad refactor. * ch122102 Remove more refactoring colateral * ch122102 Intermediate step in getting things to appear as moves. * ch122102 Test files are now renames. * ch122102 Reformat all test code. * ch122102 Additional comment cleanup. * ch122102 Rename stores test file. * ch122102 Resolve test memory leak. * ch122102 Pack testing struct to prevent valgrind issue with parameterized tests in google test. * Update fixture comment to make more sense. * ch122102 Add blank lines to end of files. Re-enable tests. * ch122102 Make all CMakeLists.txt have the same minimum required version. * ch122102 Start addressing review feedback. * ch122102 Use the wrapper header thread. It will assert if the conditions are not met. * ch122102 Use the correct define to enable the additional redis tests. * ch123169 Add unit test reporting for circleci. (#125) * ch123169 Add unit test reporting for circleci. * ch123169 End files with blank line. * ch123169 Switch to absolute GTEST_OUTPUT directory. * ch123169 Update to use pwd * Updates docs URLs * ch124211 Make google test makefile not relative. (#127) * update Releaser config and scripts to v2 model (#128) * update Releaser config and scripts to v2 format * fix config syntax * use newer version of ld-c-sdk-ubuntu image that includes git * move copying of artifacts to build step * go back to using CircleCI for the Linux build because otherwise the linux-specific build-docs script doesn't get run * fix script to not do Releaser-dependent things unless it's being called in that context * rm unused * sc-126967 Build for windows in release. (#129) * SC-130264: Update PCRE download URL [Windows] (#131) * Update PCRE download URL to Sourceforge to fix Windows build The original FTP server hosted by pcre.org is defunct. This commit updates the build to download the PCRE package from Sourceforge instead. Additionally, a basic SHA256 check has been added to ensure the remote package does not silently change out from under us. * Remove unused headers from various source files (#130) * Fix batch of spelling issues (#133) * Add dedicated valgrind & helgrind analysis jobs (#136) * Add dedicated valgrind & helgrind analysis jobs * Minor updates to language (#140) * Fix language 'API Interface' -> 'API' (#141) * SC-121933: Add additional JSON helpers (#142) * Add additional JSON helpers for directly setting strings, bools, numbers * Add JSON helper for setting a non-owned object reference * Add JSON helper for setting & creating a child object * SC-131685: Add LDVersion API (#145) * Add LDVersion() API * SC-121933: Add LDAllFlagsState API (#143) * Adds missing LDAllFlagsState functionality. * Use CircleCI macOS Gen2 resource class. (#148) * SC-138968: SDK Test Harness contract implementation (#146) * Implement contract test server * Add initial test failure suppression file * Adds singleton info (#153) * Allow LDJSONVariation to accept any type (#149) LDJSONVariation should not have an expectation on type, which was previously "array or object". It should function with any representable type, including NULL. This commit updates the method to always pass the type check. * Do not return variation index when user requests wrong flag type (#152) The SDK correctly reported an error when user requested the wrong type for a flag, but it also returned a variation index of 0. This is incorrect. Now, it returns no variation (hasVariation = false), which aligns with our docs on the subject. * Don't emit warnings when optional flag data is missing (#150) SDK would emit a warning such as "LDGetNumber value is not a number" when trying to parse optional flag data, like debugEventsUntilDate. Fixed to only parse such values if they are present. If null, they remain at their default values assigned by the parser. * Remove trailing slash from streaming URI (#151) * Remove trailing slash from streaming, base, and events URI * Add additional tests for trailing slashes * SC-134551: Fix LDStoreInitialized double-free & add concurrency test helper (#139) * Switch to drd from helgrind (which had false positives in curl) & add relevant suppressions for drd * Add concurrency helper test fixture * Add concurrency test for LDClientFlush * Add test for double-free in LDStoreInitialized * Fix double-free in LDStoreInitialized with complete refactorization * Add documentation of $initChecked * Remove embedded nlohmann/json and replace with FetchContent (#157) * Add new suppressions to handle v1.3 of SDK test harness (#158) * FileData source (#155) * Add initial work to get FileData data source support * Init store with FileData; support flags, segments, and flagValues * Add datafiles; file data tests and missing h file * Flip object merge order to give precedence to earlier flags. Add more file tests * fix memory leaks * Fix 32 bit operating systems. Only support 4gb files. * convert ssize_t file_count to int fileCount in hopes of making MSVC happy * Add FileData documentation to integrations/file_data.h * Fixes to style per PR; Fix tests to use LDSetDataSource; Add lots of defensive checks to JSON allocation; Added blocks to file loading function to narrow the scope of temporary variables * Extract readFile helper function to cleanup loadFile; rename loadFile to loadJSONFile to better reflect its return type; Cleanup tests by extracting memory management and client initialization into helper classes * Add error checks to JSON functions that were missed in original. Remember to free context and dataSource in failure case * Add CommonFixture SetUp and TearDown calls to FileDataFixture SetUp and TearDown * Convert FileDataInit from a variadic function to accepting an array as a second argument * Remove the references to a new version number; Remove assertion about filecount, just initialize with empty flags; Add test to ensure that empty file array doesnt explode; * Remove @SInCE from file_data.h Co-authored-by: Casey Waldren <cwaldren@launchdarkly.com> * Fix AllFlagsState behavior when flag evaluation fails (#159) * Fix AllFlagsState behavior when flag fails to evaluate Do not add invalid flags to the AllFlagsState object. Instead, skip them, like the AllFlags function. This commit also ports over the AllFlags unit tests to AllFlagsState. * Re-generate test harness suppressions file. * Update testharness-suppressions to handle latest additions to upstream contract tests * Merge TestData source (#160) * Implement TestData data source integration Co-authored-by: Ben Levy <benjaminlevy007@gmail.com> * Implement service-endpoints capability for contract tests (#161) * Implement service-endpoints capability for contract tests * Add events/disabling/* to testharness-suppressions These are expected to fail, since the SDK in fact does not implement this feature. * Increase EventProcessor's encapsulation and add some documentation (#164) * Refactored EventProcessor's APIs for better clarity; made it an opaque struct; added documentation * Implement config option to disable analytic events (#165) * Implement disabling analytic events * Call floor() on computed milliseconds values (#167) LaunchDarkly services expect milliseconds in JSON to be representable by an integer. Right now that's not the case, since calls to timespec divide the nanosecond value by 1e6. Update the getUnixMilliseconds API to return whole numbers. * Do not generate identify events for users with empty keys (#168) * Do not generate identify events for users with empty keys * Adds link (#166) * Update SDK contract test suppressions (#170) * Remove new payload ID test from suppressions * Add new test failures from 1.6.0 contract test release * Rename master to main in PR template & update c-sdk-common subtree (#172) * Update c-sdk-common subtree * Rename master->main in PR template * Set private user attributes in contract-test service (#171) * Redundant index event should not be generated after identify event (#169) * Use wall time instead of monotonic in EventProcessor EventProcessor was previously configured with a monotonic timestamp for lastUserKeyFlush when it was constructed. On Mac, this uses the system boot clock. This caused user keys to be incorrectly flushed the first time maybeMakeIndexEvent was called, since the expression "now [wall time] > [computations involving monotonic time]" always evaluates to true. Once the cache is cleared, lastUserKeyFlush is assigned to the wall time, which allows the logic to work correctly for subsequent maybeMakeIndexEvents, since now the time quantities are measured from the same domain. * Introducing the new LDTimestamp and LDTimer data types, making clear where wall time or monotonic time is used. * Remove suppression for redundant index event * Pin SDK test harness version so that fixing the suppressions is not a moving target * Rules: null user attributes should never match (#174) * Change debugEventsUntilDate in flag model to double (#180) Unsigned int is not large enough to store millisecond precision timestamps! (Which right now, are about 1.6 trillion ms.) * Add file manifest to Releaser configuration (#179) * releaser: modify build.sh to output a 'file manifest' for comparison between builds This manifest can be used to compare a build to a previous build, in order to validate any assumptions about what headers and library files are installed by 'make install'. * circleci: stop using sourceforge mirror for downloading Windows pcre dependency The existing mirror disappeared. We can use sourceforge's download URL instead of a direct mirror, as long as the -UserAgent "NativeHost" flag is passed to the Invoke-WebRequest commandlet (otherwise it downloads a web page.) * Fetch third-party libraries using FetchContent (#178) * cmake: Fetch most 3rd party libraries via FetchContent, and delete sources from repo. * cmake: remove unused base64 lib * Update Release CircleCI image to ld-c-sdk-ubunut:3 to pull in newer cmake * Update README with basic CMake integration instructions * releaser: add hashes for release artifacts (#182) * cmake: build 3rd party dependencies as OBJECT libraries (#181) Building 3rd party dependencies as object libraries allows for their inclusion directly in the ldserverapi target sources. This will produce a single release artifact as was previously the case before the 2.7.0 release, which started linking the 3rd party dependencies as static libraries. Linking against 3rd party dependencies as static libs "works" only for external projects that use this project's CMake targets; it doesn't work if the static lib release artifact is directly consumed. This is because target_link_libraries, when used on a static target with static dependency, does not actually "link" the file, but only forms a logical link on the CMake target. This change physically includes the 3rd party dependencies directly into ldserverapi's source as was previously the case. * cmake: set PROJECT_VERSION based on version found in launchdarkly/api.h (#187) Extract the SDK's version (set by Releaser) into the cmake variable PROJECT_VERSION. The configuration step now writes out the variable as a status message. This is setting the stage for future changes to write a package config file containing this version. * Allow for unrecognized patch paths in streaming events (#198) * Allow for unrecognized patch paths in SSE events The SDK treats patch paths that aren't for '/segments/' or '/flags/' as an error. They should instead be ignored to allow for future expansion of the spec. This commit introduces a new ParsePathStatus status code for the LDi_parsePath function, allowing callers to discriminate between a failure to parse the path, vs a failure because the path was recognised. * Update unit tests to allow for unrecognized paths in patch data * Add find_package support & integration tests (#199) * Find package support - installable base SDK [1/3] (#195) * cmake: huge overhaul to export namespaced targets and provide package configuration - Allows users to call find_package(ldserverapi REQUIRED). - Link with target_link_libraries(yourtarget ldserverapi::ldserverapi) - Alternatively, supports inclusion with add_subdirectory * Find package support - integration tests [2/3] (#196) * Adds tests to check that the SDK can be integrated into other CMake projects easily. * Find package support - find pthreads [3/3] (#197) * cmake: link Threads::Threads instead of pthread directly * cmake: remove special case for apple include directory * Updates to main README and cmake_project README * releaser: configure windows timeout to 20 minutes (#200) * releaser: configure windows timeout to 20 minutes * integrations/file_data.c: use int instead of char for fgetc (#202) fgetc may return EOF, which will be negative. The char type used to hold the result is usually signed, but may not be on some platforms (implementation defined.) Switch to holding the return value as int, and then casting to char once determined that it is not EOF. * Compile 3rd party object libs as position-independent code when building shared library It appears that CMake doesn't add the -fPIC flag to object libraries that are added to shared libraries (i.e. ldserverapi when BUILD_SHARED_LIBS=ON). See https://gitlab.kitware.com/cmake/cmake/-/issues/17356. This should theoretically cause build failures - and it does - on certain platforms, namely the Lua SDKs. I'm not clear why the cmake tests pass, and Releaser is able to generate shared libraries for the project without issue. It's possible that older version of CMake + GCC cause an interaction where the flag needs to be manually added. More investigation is needed to get to the bottom of this. * contract-tests: remove custom macro for defining types with optional fields (#204) New nlohmann/json release (v3.11.0) contains native support for the existing macro that was added to this codebase. This commit removes the macro definition. * Update evaluation to handle the offVariation being unspecified or null. Add test to make sure there are not logs generated during an eval containing an unspecified/null offVariation. (#205) Co-authored-by: Harpo Roeder <hroeder@launchdarkly.com> Co-authored-by: Ben Woskow <bwoskow@launchdarkly.com> Co-authored-by: Ben Woskow <48036130+bwoskow-ld@users.noreply.github.com> Co-authored-by: Eli Bishop <eli@launchdarkly.com> Co-authored-by: LaunchDarklyCI <dev@launchdarkly.com> Co-authored-by: LaunchDarklyReleaseBot <launchdarklyreleasebot@launchdarkly.com> Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Co-authored-by: Ember Stevens <ember.stevens@launchdarkly.com> Co-authored-by: ember-stevens <79482775+ember-stevens@users.noreply.github.com> Co-authored-by: Casey Waldren <cwaldren@launchdarkly.com> Co-authored-by: Gavin Whelan <gwhelan@launchdarkly.com> Co-authored-by: Ben Levy <benjaminlevy007@gmail.com>
diff --git a/src/evaluate.c b/src/evaluate.c
@@ -231,6 +231,7 @@ LDi_evaluate(
     /* on */
     {
         const struct LDJSON *on;
+        struct LDJSON* offVariationValue = NULL;
 
         if (!lookupOptionalValueOfType(flag, "flag", "on", LDBool, &on)) {
             return EVAL_SCHEMA;
@@ -239,11 +240,16 @@ LDi_evaluate(
         if (on == NULL || !LDGetBool(on)) {
             details->reason = LD_OFF;
 
-            if (!(addValue(
+            offVariationValue = LDObjectLookup(flag, "offVariation");
+            /* It is valid for the offVariation to either be unspecified or to be null. */
+            if(offVariationValue == NULL || LDJSONGetType(offVariationValue) == LDNull) {
+                *o_value = NULL;
+            }
+            else if (!(addValue(
                     flag,
                     o_value,
                     details,
-                    LDObjectLookup(flag, "offVariation")))) {
+                    offVariationValue))) {
                 LD_LOG(LD_LOG_ERROR, "failed to add value");
 
                 return EVAL_MEM;
diff --git a/src/variations.c b/src/variations.c
@@ -243,7 +243,6 @@ variation(
     flagrc    = NULL;
     value     = NULL;
     store     = NULL;
-    value     = NULL;
     subEvents = NULL;
 
     LDDetailsInit(&details);
diff --git a/tests/test-eval.cpp b/tests/test-eval.cpp
@@ -1,5 +1,7 @@
 #include "gtest/gtest.h"
 #include "commonfixture.h"
+#include <vector>
+#include <mutex>
 
 extern "C" {
 #include <float.h>
@@ -15,9 +17,30 @@ extern "C" {
 #include "utility.h"
 }
 
+static std::vector<std::string> logMessages;
+static std::mutex logMessagesMutex;
+
 // Inherit from the CommonFixture to give a reasonable name for the test output.
 // Any custom setup and teardown would happen in this derived class.
 class EvalFixture : public CommonFixture {
+    void SetUp() override {
+        CommonFixture::SetUp();
+        LDConfigureGlobalLogger(LD_LOG_TRACE, [](const LDLogLevel level, const char *const message){
+            std::lock_guard<std::mutex> guard(logMessagesMutex);
+            logMessages.push_back(std::string(message));
+        });
+    }
+
+public:
+    void ResetLogs() {
+        std::lock_guard<std::mutex> guard(logMessagesMutex);
+        logMessages.clear();
+    }
+
+    size_t LogCount() {
+        std::lock_guard<std::mutex> guard(logMessagesMutex);
+        return logMessages.size();
+    }
 };
 
 static struct LDStore *
@@ -127,6 +150,50 @@ TEST_F(EvalFixture, ReturnsOffVariationIfFlagIsOff) {
     LDDetailsClear(&details);
 }
 
+TEST_F(EvalFixture, ReturnsCorrectReasonWhenOffAndOffVariationNull) {
+    struct LDUser *user;
+    struct LDJSON *flag, *result, *events;
+    struct LDDetails details;
+
+    events = NULL;
+    result = NULL;
+    LDDetailsInit(&details);
+
+    ASSERT_TRUE(user = LDUserNew("userKeyA"));
+
+    /* flag */
+    ASSERT_TRUE(flag = LDNewObject());
+    ASSERT_TRUE(LDObjectSetKey(flag, "key", LDNewText("feature0")));
+    ASSERT_TRUE(LDObjectSetKey(flag, "offVariation", LDNewNull()));
+    ASSERT_TRUE(LDObjectSetKey(flag, "on", LDNewBool(LDBooleanFalse)));
+    ASSERT_TRUE(LDObjectSetKey(flag, "salt", LDNewText("abc")));
+    addVariations1(flag);
+    setFallthrough(flag, 0);
+
+    ResetLogs();
+    /* run */
+    ASSERT_EQ(
+            LDi_evaluate(
+                    NULL,
+                    flag,
+                    user,
+                    (struct LDStore *) 1,
+                    &details,
+                    &events,
+                    &result,
+                    LDBooleanFalse), EVAL_MISS);
+
+    /* validation */
+    ASSERT_EQ(LogCount(), 0); // No messages should have been logged.
+    ASSERT_EQ(details.reason, LD_OFF);
+    ASSERT_FALSE(events);
+
+    LDJSONFree(flag);
+    LDJSONFree(result);
+    LDUserFree(user);
+    LDDetailsClear(&details);
+}
+
 TEST_F(EvalFixture, FlagReturnsNilIfFlagIsOffAndOffVariantIsUnspecified) {
     struct LDUser *user;
     struct LDJSON *flag, *result, *events;
@@ -146,6 +213,7 @@ TEST_F(EvalFixture, FlagReturnsNilIfFlagIsOffAndOffVariantIsUnspecified) {
     setFallthrough(flag, 0);
     addVariations1(flag);
 
+    ResetLogs();
     /* run */
     ASSERT_EQ(
             LDi_evaluate(
@@ -163,6 +231,7 @@ TEST_F(EvalFixture, FlagReturnsNilIfFlagIsOffAndOffVariantIsUnspecified) {
     ASSERT_FALSE(details.hasVariation);
     ASSERT_EQ(details.reason, LD_OFF);
     ASSERT_FALSE(events);
+    ASSERT_EQ(LogCount(), 0); // Unspecified is valid, there should be no logs.
 
     LDJSONFree(flag);
     LDJSONFree(result);
@@ -247,6 +316,7 @@ TEST_F(EvalFixture, FlagReturnsErrorForFallthroughWithNoVariationAndNoRollout) {
     ASSERT_TRUE(fallthrough = LDNewObject());
     ASSERT_TRUE(LDObjectSetKey(flag, "fallthrough", fallthrough));
 
+    ResetLogs();
     /* run */
     ASSERT_EQ(
             LDi_evaluate(
@@ -263,7 +333,7 @@ TEST_F(EvalFixture, FlagReturnsErrorForFallthroughWithNoVariationAndNoRollout) {
     ASSERT_EQ(details.reason, LD_FALLTHROUGH);
     ASSERT_FALSE(events);
     ASSERT_FALSE(result);
-
+    ASSERT_GE(LogCount(), 1);
     LDJSONFree(flag);
     LDUserFree(user);
     LDDetailsClear(&details);
diff --git a/tests/test-variations.cpp b/tests/test-variations.cpp
@@ -314,3 +314,74 @@ TEST_F(VariationsFixture, FallthroughWithNoVariationOrRollout) {
     LDClientClose(client);
     LDDetailsClear(&details);
 }
+
+TEST_F(VariationsFixture, OffWithNullOffVariation) {
+    struct LDJSON *flag, *fallthrough, *variation, *rollout, *variations;
+    struct LDClient *client;
+    struct LDUser *user;
+    char *actual;
+    struct LDDetails details;
+
+    struct LDJSON *events;
+
+    /* setup */
+    ASSERT_TRUE(client = makeTestClient());
+    ASSERT_TRUE(user = LDUserNew("userkey"));
+    /* flag */
+    ASSERT_TRUE(flag = LDNewObject());
+    ASSERT_TRUE(LDObjectSetKey(flag, "key", LDNewText("feature0")));
+    ASSERT_TRUE(LDObjectSetKey(flag, "offVariation", LDNewNull()));
+    ASSERT_TRUE(LDObjectSetKey(flag, "on", LDNewBool(LDBooleanFalse)));
+    ASSERT_TRUE(LDObjectSetKey(flag, "salt", LDNewText("123123")));
+    ASSERT_TRUE(LDObjectSetKey(flag, "version", LDNewNumber(3)));
+
+    ASSERT_TRUE(LDStoreInitEmpty(client->store));
+    LDStoreUpsert(client->store, LD_FLAG, flag);
+    /* run */
+    actual = LDStringVariation(client, user, "feature0", "test", &details);
+    /* It is valid to have a null off variation, and we should return the default value. */
+    /* validate */
+    ASSERT_STREQ("test", actual);
+    LDFree(actual);
+    ASSERT_EQ(details.reason, LD_OFF);
+
+    /* cleanup */
+    LDUserFree(user);
+    LDClientClose(client);
+    LDDetailsClear(&details);
+}
+
+TEST_F(VariationsFixture, OffWithUndefinedOffVariation) {
+    struct LDJSON *flag, *fallthrough, *variation, *rollout, *variations;
+    struct LDClient *client;
+    struct LDUser *user;
+    char *actual;
+    struct LDDetails details;
+
+    struct LDJSON *events;
+
+    /* setup */
+    ASSERT_TRUE(client = makeTestClient());
+    ASSERT_TRUE(user = LDUserNew("userkey"));
+    /* flag */
+    ASSERT_TRUE(flag = LDNewObject());
+    ASSERT_TRUE(LDObjectSetKey(flag, "key", LDNewText("feature0")));
+    ASSERT_TRUE(LDObjectSetKey(flag, "on", LDNewBool(LDBooleanFalse)));
+    ASSERT_TRUE(LDObjectSetKey(flag, "salt", LDNewText("123123")));
+    ASSERT_TRUE(LDObjectSetKey(flag, "version", LDNewNumber(3)));
+
+    ASSERT_TRUE(LDStoreInitEmpty(client->store));
+    LDStoreUpsert(client->store, LD_FLAG, flag);
+    /* run */
+    actual = LDStringVariation(client, user, "feature0", "test", &details);
+    /* It is valid to have an undefined off variation, and we should return the default value. */
+    /* validate */
+    ASSERT_STREQ("test", actual);
+    LDFree(actual);
+    ASSERT_EQ(details.reason, LD_OFF);
+
+    /* cleanup */
+    LDUserFree(user);
+    LDClientClose(client);
+    LDDetailsClear(&details);
+}
