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

Author: tusharkhandelwal8

firebase-config/gradle.properties

@@ -14,7 +14,7 @@
 # limitations under the License.
 #
 
-version=22.0.1
+version=22.0.2
 latestReleasedVersion=22.0.0
 android.enableUnitTestBinaryResources=true
 

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigSharedPrefsClient.java

@@ -266,6 +266,15 @@ public void setCustomSignals(Map<String, Object> newCustomSignals) {
         String key = entry.getKey();
         Object value = entry.getValue();
 
+        // Validate value type, and key and value length
+        if (value != null && !(value instanceof String || value instanceof Long)) {
+          throw new IllegalArgumentException("Custom signal values must be of type String or Long");
+        }
+        if (key.length() > 250 || (value instanceof String && ((String) value).length() > 500)) {
+          throw new IllegalArgumentException(
+              "Custom signal keys must be 250 characters or less, and string values must be 500 characters or less.");
+        }
+
         // Merge new signals with existing ones, overwriting existing keys.
         // Also, remove entries where the new value is null.
         if (value != null) {
@@ -274,6 +283,16 @@ public void setCustomSignals(Map<String, Object> newCustomSignals) {
           existingCustomSignals.remove(key);
         }
       }
+
+      // Check if the map has actually changed and the size limit
+      if (existingCustomSignals.equals(getCustomSignals())) {
+        return;
+      }
+      if (existingCustomSignals.size() > 100) {
+        throw new IllegalArgumentException(
+            "Too many custom signals provided. The maximum allowed is 100.");
+      }
+
       frcMetadata
           .edit()
           .putString(CUSTOM_SIGNALS, new JSONObject(existingCustomSignals).toString())
@@ -289,7 +308,11 @@ public Map<String, Object> getCustomSignals() {
       Iterator<String> keys = existingCustomSignalsJson.keys();
       while (keys.hasNext()) {
         String key = keys.next();
-        custom_signals.put(key, existingCustomSignalsJson.get(key));
+        Object value = existingCustomSignalsJson.get(key);
+        if (value instanceof Integer) {
+          value = existingCustomSignalsJson.getLong(key);
+        }
+        custom_signals.put(key, value);
       }
       return custom_signals;
     } catch (JSONException e) {

firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigSharedPrefsClientTest.java

@@ -25,6 +25,7 @@
 import static com.google.firebase.remoteconfig.internal.ConfigSharedPrefsClient.LAST_FETCH_TIME_NO_FETCH_YET;
 import static com.google.firebase.remoteconfig.internal.ConfigSharedPrefsClient.NO_BACKOFF_TIME;
 import static com.google.firebase.remoteconfig.internal.ConfigSharedPrefsClient.NO_FAILED_FETCHES;
+import static com.google.firebase.remoteconfig.testutil.Assert.assertThrows;
 
 import android.content.Context;
 import android.content.SharedPreferences;
@@ -37,6 +38,7 @@
 import com.google.firebase.remoteconfig.internal.ConfigSharedPrefsClient.LastFetchStatus;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
@@ -367,4 +369,43 @@ public void getCustomSignals_isSet_returnsCustomSignals() {
     metadataClient.setCustomSignals(SAMPLE_CUSTOM_SIGNALS);
     assertThat(metadataClient.getCustomSignals()).isEqualTo(SAMPLE_CUSTOM_SIGNALS);
   }
+
+  @Test
+  public void setCustomSignals_multipleTimes_addsNewSignals() {
+    Map<String, Object> signals1 = ImmutableMap.of("subscription", "premium");
+    Map<String, Object> signals2 = ImmutableMap.of("age", 20L);
+    metadataClient.setCustomSignals(signals1);
+    metadataClient.setCustomSignals(signals2);
+    Map<String, Object> expectedSignals = ImmutableMap.of("subscription", "premium", "age", 20L);
+    assertThat(metadataClient.getCustomSignals()).isEqualTo(expectedSignals);
+  }
+
+  @Test
+  public void setCustomSignals_nullValue_removesSignal() {
+    Map<String, Object> signals1 = ImmutableMap.of("subscription", "premium", "age", 20L);
+    metadataClient.setCustomSignals(signals1);
+    Map<String, Object> signals2 = new HashMap<>();
+    signals2.put("age", null);
+    metadataClient.setCustomSignals(signals2);
+    Map<String, Object> expectedSignals = ImmutableMap.of("subscription", "premium");
+    assertThat(metadataClient.getCustomSignals()).isEqualTo(expectedSignals);
+  }
+
+  @Test
+  public void setCustomSignals_invalidInput_throwsException() {
+    Map<String, Object> invalidSignals1 = new HashMap<>();
+    invalidSignals1.put("name", true);
+    assertThrows(
+        IllegalArgumentException.class, () -> metadataClient.setCustomSignals(invalidSignals1));
+
+    Map<String, Object> invalidSignals2 = new HashMap<>();
+    invalidSignals2.put("a".repeat(251), "value");
+    assertThrows(
+        IllegalArgumentException.class, () -> metadataClient.setCustomSignals(invalidSignals2));
+
+    Map<String, Object> invalidSignals3 = new HashMap<>();
+    invalidSignals3.put("key", "a".repeat(501));
+    assertThrows(
+        IllegalArgumentException.class, () -> metadataClient.setCustomSignals(invalidSignals3));
+  }
 }