Summary Event Crash (#91)

* adds summary event tests with empty and nil tracker

* synchronize tracker access

* adds guard for nil flagKey to initSummaryEvent

* adds nil summary event guard to DataManager createSummaryEvent

* adds guard for empty flagKey to FlagConfigTracker

* removes unused defaultValue from FlagCounter logRequest

* adds guard for nil reported value in FlagCounter’s logRequest

* adds nil reportedValue handling to LDFlagValueCounter

* advance version documents

* removes access to LDFlagConfigTracker flagCounters

Too much chance to misuse it

* removes flagCounters from public access

* move selected items outside of synchronized blocks

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 All notable changes to the LaunchDarkly iOS SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org).
 
+## [2.13.3] - 2018-08-15
+### Changed
+- Synchronized summary event creation to limit thread access and protect data integrity
+- Improved the robustness of the code creating summary events to better handle unexpected data
+
 ## [2.13.2] - 2018-07-27
 ### Fixed
 - Updated `DarklyEventSource` in order to fix potential flag stream parsing issues.

Darkly.xcodeproj/project.pbxproj

@@ -1155,7 +1155,7 @@
 		690346B51E6872EA00E45133 /* Project object */ = {
 			isa = PBXProject;
 			attributes = {
-				LastUpgradeCheck = 0920;
+				LastUpgradeCheck = 0940;
 				ORGANIZATIONNAME = LaunchDarkly;
 				TargetAttributes = {
 					690346BD1E6872EA00E45133 = {

Darkly.xcodeproj/xcshareddata/xcschemes/Darkly_iOS.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "0920"
+   LastUpgradeVersion = "0940"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"
@@ -26,9 +26,8 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
-      shouldUseLaunchSchemeArgsEnv = "YES"
-      codeCoverageEnabled = "YES">
+      codeCoverageEnabled = "YES"
+      shouldUseLaunchSchemeArgsEnv = "YES">
       <Testables>
          <TestableReference
             skipped = "NO">
@@ -57,7 +56,6 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
       launchStyle = "0"
       useCustomWorkingDirectory = "NO"
       ignoresPersistentStateOnLaunch = "NO"

Darkly.xcodeproj/xcshareddata/xcschemes/Darkly_osx.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "0920"
+   LastUpgradeVersion = "0940"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"
@@ -26,7 +26,6 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
       shouldUseLaunchSchemeArgsEnv = "YES">
       <Testables>
       </Testables>
@@ -37,7 +36,6 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
       launchStyle = "0"
       useCustomWorkingDirectory = "NO"
       ignoresPersistentStateOnLaunch = "NO"

Darkly.xcodeproj/xcshareddata/xcschemes/Darkly_tvOS.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "0920"
+   LastUpgradeVersion = "0940"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"
@@ -26,7 +26,6 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
       shouldUseLaunchSchemeArgsEnv = "YES">
       <Testables>
       </Testables>
@@ -37,7 +36,6 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
       launchStyle = "0"
       useCustomWorkingDirectory = "NO"
       ignoresPersistentStateOnLaunch = "NO"

Darkly.xcodeproj/xcshareddata/xcschemes/Darkly_watchOS.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "0920"
+   LastUpgradeVersion = "0940"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"
@@ -26,7 +26,6 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
       shouldUseLaunchSchemeArgsEnv = "YES">
       <Testables>
       </Testables>
@@ -37,7 +36,6 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      language = ""
       launchStyle = "0"
       useCustomWorkingDirectory = "NO"
       ignoresPersistentStateOnLaunch = "NO"

Darkly.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>IDEDidComputeMac32BitWarning</key>
+	<true/>
+</dict>
+</plist>

Darkly/DarklyConstants.m

@@ -4,7 +4,7 @@
 
 #import "DarklyConstants.h"
 
-NSString * const kClientVersion = @"2.13.2";
+NSString * const kClientVersion = @"2.13.3";
 NSString * const kBaseUrl = @"https://app.launchdarkly.com";
 NSString * const kEventsUrl = @"https://mobile.launchdarkly.com";
 NSString * const kStreamUrl = @"https://clientstream.launchdarkly.com";

Darkly/LDDataManager.m

@@ -193,12 +193,17 @@ -(void)createIdentifyEventWithUser:(LDUserModel*)user config:(LDConfig*)config {
 }
 
 -(void)createSummaryEventWithTracker:(LDFlagConfigTracker*)tracker config:(LDConfig*)config {
-    if (tracker.flagCounters.count == 0) {
-        DEBUG_LOGX(@"Tracker has no flag counters. Discarding summary event.");
+    if (!tracker.hasTrackedEvents) {
+        DEBUG_LOGX(@"Tracker has no events to report. Discarding summary event.");
+        return;
+    }
+    LDEventModel *summaryEvent = [LDEventModel summaryEventWithTracker:tracker];
+    if (summaryEvent == nil) {
+        DEBUG_LOGX(@"Failed to create summary event. Aborting.");
         return;
     }
     DEBUG_LOGX(@"Creating summary event");
-    [self addEventDictionary:[[LDEventModel summaryEventWithTracker:tracker] dictionaryValueUsingConfig:config]];
+    [self addEventDictionary:[summaryEvent dictionaryValueUsingConfig:config]];
 }
 
 -(void)createDebugEventWithFlagKey:(NSString *)flagKey

Darkly/LDEventModel.m

@@ -149,15 +149,12 @@ +(instancetype)summaryEventWithTracker:(LDFlagConfigTracker*)tracker {
 
 -(instancetype)initSummaryEventWithTracker:(LDFlagConfigTracker*)tracker {
     if(!(self = [self init])) { return nil; }
+    if(tracker == nil) { return nil; }
 
     self.kind = kEventModelKindFeatureSummary;
     self.startDateMillis = tracker.startDateMillis;
     self.endDateMillis = [[NSDate date] millisSince1970];
-    NSMutableDictionary *flagRequestSummary = [NSMutableDictionary dictionaryWithCapacity:tracker.flagCounters.count];
-    for (NSString *flagKey in [tracker.flagCounters.allKeys copy]) {
-        flagRequestSummary[flagKey] = [tracker.flagCounters[flagKey] dictionaryValue];
-    }
-    self.flagRequestSummary = [NSDictionary dictionaryWithDictionary:flagRequestSummary];
+    self.flagRequestSummary = tracker.flagRequestSummary;
 
     return self;
 }

Darkly/LDFlagConfig/LDFlagConfigTracker.h

@@ -14,7 +14,7 @@
 
 @interface LDFlagConfigTracker : NSObject
 @property (nonatomic, assign, readonly) LDMillisecond startDateMillis;
-@property (nonnull, nonatomic, strong, readonly) NSDictionary<NSString*, LDFlagCounter*> *flagCounters;
+@property (nonatomic, assign, readonly) BOOL hasTrackedEvents;
 
 +(nonnull instancetype)tracker;
 -(nonnull instancetype)init;
@@ -23,6 +23,6 @@
           reportedFlagValue:(nonnull id)reportedFlagValue
             flagConfigValue:(nullable LDFlagConfigValue*)flagConfigValue
                defaultValue:(nullable id)defaultValue;
-
+-(nonnull NSDictionary<NSString*, NSDictionary*>*)flagRequestSummary;
 -(nonnull NSString*)description;
 @end

Darkly/LDFlagConfig/LDFlagConfigTracker.m

@@ -11,6 +11,7 @@
 #import "LDFlagCounter.h"
 #import "LDFlagConfigValue.h"
 #import "NSDate+ReferencedDate.h"
+#import "LDUtil.h"
 
 @interface LDFlagConfigTracker()
 @property (nonatomic, assign) LDMillisecond startDateMillis;
@@ -26,22 +27,45 @@ +(instancetype)tracker {
 -(instancetype)init {
     if (!(self = [super init])) { return nil; }
 
-    self.startDateMillis = [[NSDate date] millisSince1970];
-    self.mutableFlagCounters = [NSMutableDictionary dictionary];
+    @synchronized(self) {
+        self.startDateMillis = [[NSDate date] millisSince1970];
+        self.mutableFlagCounters = [NSMutableDictionary dictionary];
 
-    return self;
+        return self;
+    }
 }
 
--(NSDictionary*)flagCounters {
-    return [NSDictionary dictionaryWithDictionary:self.mutableFlagCounters];
+-(BOOL)hasTrackedEvents {
+    @synchronized(self) {
+        return self.mutableFlagCounters.count > 0;
+    }
 }
 
 -(void)logRequestForFlagKey:(NSString*)flagKey reportedFlagValue:(id)reportedFlagValue flagConfigValue:(LDFlagConfigValue*)flagConfigValue defaultValue:(id)defaultValue {
-    if (!self.mutableFlagCounters[flagKey]) {
-        self.mutableFlagCounters[flagKey] = [LDFlagCounter counterWithFlagKey:flagKey defaultValue:defaultValue];
+    if(flagKey.length == 0) {
+        DEBUG_LOGX(@"-[LDFlagConfigTracker logRequestForFlagKey:reportedFlagValue:flagConfigValue:defaultValue] called with an empty flagKey. Aborting.");
+        return;
+    }
+    @synchronized(self) {
+        if (!self.mutableFlagCounters[flagKey]) {
+            self.mutableFlagCounters[flagKey] = [LDFlagCounter counterWithFlagKey:flagKey defaultValue:defaultValue];
+        }
+
+        [self.mutableFlagCounters[flagKey] logRequestWithFlagConfigValue:flagConfigValue reportedFlagValue:reportedFlagValue];
+    }
+}
+
+-(NSDictionary<NSString*, NSDictionary*>*)flagRequestSummary {
+    @synchronized(self) {
+        NSMutableDictionary *flagRequestSummary = [NSMutableDictionary dictionaryWithCapacity:self.mutableFlagCounters.count];
+        NSDictionary<NSString*, LDFlagCounter*> *flagCounters = [NSDictionary dictionaryWithDictionary:self.mutableFlagCounters];
+        for (NSString *flagKey in flagCounters.allKeys) {
+            NSDictionary *counterDictionary = [flagCounters[flagKey] dictionaryValue];
+            if (counterDictionary == nil) { continue; }
+            flagRequestSummary[flagKey] = counterDictionary;
+        }
+        return [NSDictionary dictionaryWithDictionary:flagRequestSummary];
     }
-
-    [self.mutableFlagCounters[flagKey] logRequestWithFlagConfigValue:flagConfigValue reportedFlagValue:reportedFlagValue defaultValue:defaultValue];
 }
 
 -(NSString*)description {

Darkly/LDFlagConfig/LDFlagCounter.h

@@ -14,12 +14,12 @@
 @interface LDFlagCounter : NSObject
 @property (nonatomic, strong, nonnull, readonly) NSString *flagKey;
 @property (nonatomic, strong, nonnull) id defaultValue;
-@property (nonatomic, strong, nonnull, readonly) NSArray<LDFlagValueCounter*> *valueCounters;
+@property (nonatomic, assign, readonly) BOOL hasLoggedRequests;
 
 +(nonnull instancetype)counterWithFlagKey:(nonnull NSString*)flagKey defaultValue:(nonnull id)defaultValue;
 -(nonnull instancetype)initWithFlagKey:(nonnull NSString*)flagKey defaultValue:(nonnull id)defaultValue;
 
--(void)logRequestWithFlagConfigValue:(nullable LDFlagConfigValue*)flagConfigValue reportedFlagValue:(nonnull id)reportedFlagValue defaultValue:(nullable id)defaultValue;
+-(void)logRequestWithFlagConfigValue:(nullable LDFlagConfigValue*)flagConfigValue reportedFlagValue:(nonnull id)reportedFlagValue;
 
 -(nonnull NSDictionary*)dictionaryValue;
 

Darkly/LDFlagConfig/LDFlagCounter.m

@@ -27,25 +27,34 @@ +(instancetype)counterWithFlagKey:(NSString*)flagKey defaultValue:(id)defaultVal
 -(instancetype)initWithFlagKey:(NSString*)flagKey defaultValue:(id)defaultValue {
     if (!(self = [super init])) { return nil; }
 
-    self.flagKey = flagKey;
-    self.defaultValue = defaultValue;
-    self.flagValueCounters = [NSMutableArray array];
+    @synchronized(self) {
+        self.flagKey = flagKey;
+        self.defaultValue = defaultValue;
+        self.flagValueCounters = [NSMutableArray array];
 
-    return self;
+        return self;
+    }
 }
 
--(NSArray<LDFlagValueCounter*>*)valueCounters {
-    return self.flagValueCounters;
+-(BOOL)hasLoggedRequests {
+    @synchronized(self) {
+        return self.flagValueCounters.count > 0;
+    }
 }
 
--(void)logRequestWithFlagConfigValue:(LDFlagConfigValue*)flagConfigValue reportedFlagValue:(id)reportedFlagValue defaultValue:(id)defaultValue {
-    LDFlagValueCounter *selectedFlagValueCounter = [self valueCounterForFlagConfigValue:flagConfigValue];
-    if (selectedFlagValueCounter) {
-        selectedFlagValueCounter.count += 1;
-        return;
+-(void)logRequestWithFlagConfigValue:(LDFlagConfigValue*)flagConfigValue reportedFlagValue:(id)reportedFlagValue {
+    if (reportedFlagValue == nil) {
+        reportedFlagValue = [NSNull null];
     }
+    @synchronized(self) {
+        LDFlagValueCounter *selectedFlagValueCounter = [self valueCounterForFlagConfigValue:flagConfigValue];
+        if (selectedFlagValueCounter) {
+            selectedFlagValueCounter.count += 1;
+            return;
+        }
 
-    [self.flagValueCounters addObject:[LDFlagValueCounter counterWithFlagConfigValue:flagConfigValue reportedFlagValue:reportedFlagValue]];
+        [self.flagValueCounters addObject:[LDFlagValueCounter counterWithFlagConfigValue:flagConfigValue reportedFlagValue:reportedFlagValue]];
+    }
 }
 
 -(LDFlagValueCounter*)valueCounterForFlagConfigValue:(LDFlagConfigValue*)flagConfigValue {
@@ -58,24 +67,27 @@ -(LDFlagValueCounter*)valueCounterForFlagConfigValue:(LDFlagConfigValue*)flagCon
 }
 
 -(NSDictionary*)dictionaryValue {
-    NSMutableArray<NSDictionary*> *flagValueCounterDictionaries = [NSMutableArray arrayWithCapacity:self.flagValueCounters.count];
-    for (LDFlagValueCounter *flagValueCounter in self.flagValueCounters) {
-        NSMutableDictionary *flagValueCounterDictionary = [NSMutableDictionary dictionaryWithDictionary:[flagValueCounter dictionaryValue]];
-        //If the flagConfigValue.value is nil or null, the client will have served the default value
-        if (!flagValueCounter.flagConfigValue.value || [flagValueCounter.flagConfigValue.value isKindOfClass:[NSNull class]]) {
-            flagValueCounterDictionary[kLDFlagConfigValueKeyValue] = self.defaultValue ?: [NSNull null];
+    @synchronized(self) {
+        NSMutableArray<NSDictionary*> *flagValueCounterDictionaries = [NSMutableArray arrayWithCapacity:self.flagValueCounters.count];
+        NSArray<LDFlagValueCounter*> *flagValueCounters = [NSArray arrayWithArray: self.flagValueCounters];
+        for (LDFlagValueCounter *flagValueCounter in flagValueCounters) {
+            NSMutableDictionary *flagValueCounterDictionary = [NSMutableDictionary dictionaryWithDictionary:[flagValueCounter dictionaryValue]];
+            //If the flagConfigValue.value is nil or null, the client will have served the default value
+            if (!flagValueCounter.flagConfigValue.value || [flagValueCounter.flagConfigValue.value isKindOfClass:[NSNull class]]) {
+                flagValueCounterDictionary[kLDFlagConfigValueKeyValue] = self.defaultValue ?: [NSNull null];
+            }
+            [flagValueCounterDictionaries addObject:[flagValueCounterDictionary copy]];
         }
-        [flagValueCounterDictionaries addObject:[flagValueCounterDictionary copy]];
-    }
-    NSMutableDictionary *dictionary = [NSMutableDictionary dictionary];
-    if (self.defaultValue) {
-        dictionary[kLDFlagCounterKeyDefaultValue] = self.defaultValue;
-    } else {
-        dictionary[kLDFlagCounterKeyDefaultValue] = [NSNull null];
-    }
-    dictionary[kLDFlagCounterKeyCounters] = flagValueCounterDictionaries;
+        NSMutableDictionary *dictionary = [NSMutableDictionary dictionary];
+        if (self.defaultValue) {
+            dictionary[kLDFlagCounterKeyDefaultValue] = self.defaultValue;
+        } else {
+            dictionary[kLDFlagCounterKeyDefaultValue] = [NSNull null];
+        }
+        dictionary[kLDFlagCounterKeyCounters] = flagValueCounterDictionaries;
 
-    return [NSDictionary dictionaryWithDictionary:dictionary];
+        return [NSDictionary dictionaryWithDictionary:dictionary];
+    }
 }
 
 -(NSString*)description {

Darkly/LDFlagConfig/LDFlagValueCounter.m

@@ -27,8 +27,11 @@ +(instancetype)counterWithFlagConfigValue:(LDFlagConfigValue*)flagConfigValue re
     return [[LDFlagValueCounter alloc] initWithFlagConfigValue:flagConfigValue reportedFlagValue:reportedFlagValue];
 }
 
--(instancetype)initWithFlagConfigValue:(LDFlagConfigValue*)flagConfigValue reportedFlagValue:(id)reportedFlagValue  {
+-(instancetype)initWithFlagConfigValue:(LDFlagConfigValue*)flagConfigValue reportedFlagValue:(id)reportedFlagValue {
     if (!(self = [super init])) { return nil; }
+    if (reportedFlagValue == nil) {
+        reportedFlagValue = [NSNull null];
+    }
 
     self.flagConfigValue = flagConfigValue;
     self.reportedFlagValue = reportedFlagValue;
@@ -48,7 +51,7 @@ -(NSDictionary*)dictionaryValue {
     } else {
         dictionary[kLDFlagValueCounterKeyUnknown] = @(YES);
     }
-    dictionary[kLDFlagValueCounterKeyValue] = self.reportedFlagValue ?: [NSNull null];
+    dictionary[kLDFlagValueCounterKeyValue] = self.reportedFlagValue;
     dictionary[kLDFlagValueCounterKeyCount] = @(self.count);
 
     return [NSDictionary dictionaryWithDictionary:dictionary];