Add limits, input validation, and unit tests for custom signals

Author: tusharkhandelwal8

FirebaseRemoteConfig/Sources/FIRRemoteConfig.m

@@ -245,53 +245,91 @@ - (void)setCustomSignals:(nullable NSDictionary<NSString *, NSObject *> *)custom
   void (^setCustomSignalsBlock)(void) = ^{
     if (!customSignals) {
       if (completionHandler) {
-        dispatch_async(dispatch_get_main_queue(), ^{
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
           completionHandler(nil);
         });
       }
       return;
     }
 
+    // Validate value type, and key and value length
     for (NSString *key in customSignals) {
       NSObject *value = customSignals[key];
-      if (value && ![value isKindOfClass:[NSString class]] &&
+      if (![value isKindOfClass:[NSNull class]] && ![value isKindOfClass:[NSString class]] &&
           ![value isKindOfClass:[NSNumber class]]) {
         if (completionHandler) {
-          dispatch_async(dispatch_get_main_queue(), ^{
+          dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
             NSError *error =
                 [NSError errorWithDomain:FIRRemoteConfigCustomSignalsErrorDomain
                                     code:FIRRemoteConfigCustomSignalsErrorInvalidValueType
                                 userInfo:@{
                                   NSLocalizedDescriptionKey :
-                                      @"Invalid value type. Must be NSString or NSNumber."
+                                      @"Invalid value type. Must be NSString, NSNumber or NSNull"
                                 }];
             completionHandler(error);
           });
         }
         return;
       }
+
+      if (key.length > 250 ||
+          ([value isKindOfClass:[NSString class]] && [(NSString *)value length] > 500)) {
+        if (completionHandler) {
+          dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+            NSError *error = [NSError errorWithDomain:FIRRemoteConfigCustomSignalsErrorDomain
+                                                 code:FIRRemoteConfigCustomSignalsErrorLimitExceeded
+                                             userInfo:@{
+                                               NSLocalizedDescriptionKey :
+                                                   @"Custom signal keys and string values must be "
+                                                   @"250 and 500 characters or less respectively."
+                                             }];
+            completionHandler(error);
+          });
+        }
+        return;
+      }
     }
 
+    // Merge new signals with existing ones, overwriting existing keys.
+    // Also, remove entries where the new value is null.
     NSMutableDictionary<NSString *, NSObject *> *newCustomSignals =
         [[NSMutableDictionary alloc] initWithDictionary:self->_settings.customSignals];
 
     for (NSString *key in customSignals) {
       NSObject *value = customSignals[key];
-      if (value) {
+      if (![value isKindOfClass:[NSNull class]]) {
         [newCustomSignals setObject:value forKey:key];
       } else {
         [newCustomSignals removeObjectForKey:key];
       }
     }
 
-    self->_settings.customSignals = newCustomSignals;
-    if (completionHandler) {
-      dispatch_async(dispatch_get_main_queue(), ^{
-        completionHandler(nil);
-      });
+    // Check the size limit.
+    if (newCustomSignals.count > 100) {
+      if (completionHandler) {
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+          NSError *error = [NSError
+              errorWithDomain:FIRRemoteConfigCustomSignalsErrorDomain
+                         code:FIRRemoteConfigCustomSignalsErrorLimitExceeded
+                     userInfo:@{
+                       NSLocalizedDescriptionKey : @"Custom signals count exceeds the limit of 100."
+                     }];
+          completionHandler(error);
+        });
+      }
+      return;
     }
-  };
 
+    // Update only if there are changes.
+    if (![newCustomSignals isEqualToDictionary:self->_settings.customSignals]) {
+      self->_settings.customSignals = newCustomSignals;
+      if (completionHandler) {
+        dispatch_async(dispatch_get_main_queue(), ^{
+          completionHandler(nil);
+        });
+      }
+    }
+  };
   dispatch_async(_queue, setCustomSignalsBlock);
 }
 

FirebaseRemoteConfig/Sources/Public/FirebaseRemoteConfig/FIRRemoteConfig.h

@@ -105,6 +105,8 @@ typedef NS_ERROR_ENUM(FIRRemoteConfigCustomSignalsErrorDomain, FIRRemoteConfigCu
     FIRRemoteConfigCustomSignalsErrorUnknown = 8001,
     /// Invalid value type in the custom signals dictionary.
     FIRRemoteConfigCustomSignalsErrorInvalidValueType = 8002,
+    /// Limit exceeded for key length, value length, or number of signals.
+    FIRRemoteConfigCustomSignalsErrorLimitExceeded = 8003,
 } NS_SWIFT_NAME(RemoteConfigCustomSignalsError);
 
 /// Enumerated value that indicates the source of Remote Config data. Data can come from

FirebaseRemoteConfig/Tests/Unit/RCNRemoteConfigTest.m

@@ -1860,6 +1860,94 @@ - (void)testSetCustomSingals {
   [self waitForExpectationsWithTimeout:_expectationTimeout handler:nil];
 }
 
+- (void)testSetCustomSignalsMultipleTimes {
+  NSMutableArray<XCTestExpectation *> *expectations =
+      [[NSMutableArray alloc] initWithCapacity:RCNTestRCNumTotalInstances];
+
+  for (int i = 0; i < RCNTestRCNumTotalInstances; i++) {
+    expectations[i] = [self
+        expectationWithDescription:
+            [NSString stringWithFormat:@"Set custom signals multiple times - instance %d", i]];
+
+    // First set of signals
+    NSDictionary<NSString *, NSObject *> *testSignals1 = @{
+      @"signal1" : @"stringValue1",
+      @"signal2" : @"stringValue2",
+    };
+
+    // Second set of signals (overwrites, remove and adds new)
+    NSDictionary<NSString *, NSObject *> *testSignals2 = @{
+      @"signal1" : @"updatedValue1",
+      @"signal2" : [NSNull null],
+      @"signal3" : @5,
+    };
+
+    // Expected final set of signals
+    NSDictionary<NSString *, NSObject *> *expectedSignals = @{
+      @"signal1" : @"updatedValue1",
+      @"signal3" : @5,
+    };
+
+    [_configInstances[i] setCustomSignals:testSignals1
+                           WithCompletion:^(NSError *_Nullable error) {
+                             XCTAssertNil(error);
+                             [_configInstances[i]
+                                 setCustomSignals:testSignals2
+                                   WithCompletion:^(NSError *_Nullable error) {
+                                     XCTAssertNil(error);
+                                     NSMutableDictionary<NSString *, NSObject *> *retrievedSignals =
+                                         self->_configInstances[i].settings.customSignals;
+                                     XCTAssertEqualObjects(retrievedSignals, expectedSignals);
+                                     [expectations[i] fulfill];
+                                   }];
+                           }];
+  }
+  [self waitForExpectationsWithTimeout:_expectationTimeout handler:nil];
+}
+
+- (void)testSetCustomSignals_invalidInput_throwsException {
+  NSMutableArray<XCTestExpectation *> *expectations =
+      [[NSMutableArray alloc] initWithCapacity:RCNTestRCNumTotalInstances];
+
+  for (int i = 0; i < RCNTestRCNumTotalInstances; i++) {
+    expectations[i] =
+        [self expectationWithDescription:
+                  [NSString stringWithFormat:@"Set custom signals expects error - instance %d", i]];
+
+    // Invalid value type.
+    NSDictionary<NSString *, NSObject *> *invalidSignals1 = @{@"name" : [NSDate date]};
+
+    // Key length exceeds limit.
+    NSDictionary<NSString *, NSObject *> *invalidSignals2 =
+        @{[@"a" stringByPaddingToLength:251 withString:@"a" startingAtIndex:0] : @"value"};
+
+    // Value length exceeds limit.
+    NSDictionary<NSString *, NSObject *> *invalidSignals3 =
+        @{@"key" : [@"a" stringByPaddingToLength:501 withString:@"a" startingAtIndex:0]};
+
+    [_configInstances[i]
+        setCustomSignals:invalidSignals1
+          WithCompletion:^(NSError *_Nullable error) {
+            XCTAssertNotNil(error);
+            XCTAssertEqual(error.code, FIRRemoteConfigCustomSignalsErrorInvalidValueType);
+          }];
+    [_configInstances[i]
+        setCustomSignals:invalidSignals2
+          WithCompletion:^(NSError *_Nullable error) {
+            XCTAssertNotNil(error);
+            XCTAssertEqual(error.code, FIRRemoteConfigCustomSignalsErrorLimitExceeded);
+          }];
+    [_configInstances[i]
+        setCustomSignals:invalidSignals3
+          WithCompletion:^(NSError *_Nullable error) {
+            XCTAssertNotNil(error);
+            XCTAssertEqual(error.code, FIRRemoteConfigCustomSignalsErrorLimitExceeded);
+            [expectations[i] fulfill];
+          }];
+  }
+  [self waitForExpectationsWithTimeout:_expectationTimeout handler:nil];
+}
+
 #pragma mark - Test Helpers
 
 - (FIROptions *)firstAppOptions {