Fix: Prevent SetDefaultEventParameters from clearing params with all-… (#1747)

* Fix: Prevent SetDefaultEventParameters from clearing params with all-invalid input

Addresses code review feedback regarding the behavior of SetDefaultEventParameters when all provided parameters are of invalid types.

Previously, if all parameters in the C++ map were invalid, an empty NSDictionary/Bundle would be passed to the native iOS/Android setDefaultEventParameters method. According to documentation, passing nil/null (for iOS/Android respectively) clears all default parameters. While passing an empty dictionary/bundle is a valid operation, it could lead to unintentionally clearing parameters if that was the state before, or it might be a no-op if parameters were already set.

This change modifies the iOS and Android implementations to explicitly check if the processed native parameter collection (NSDictionary for iOS, Bundle for Android) is empty after filtering for valid types. If it is empty, the call to the native SDK's setDefaultEventParameters method is skipped.

This ensures that providing a C++ map containing exclusively invalid parameters (or an empty map) to SetDefaultEventParameters will be a true no-op from the C++ SDK's perspective, preventing any accidental modification or clearing of existing default parameters on the native side.

* Fix: Prevent SetDefaultEventParameters from clearing params with all-invalid input

Addresses code review feedback regarding the behavior of SetDefaultEventParameters when all provided parameters are of invalid types.

Previously, if all parameters in the C++ map were invalid, an empty NSDictionary/Bundle would be passed to the native iOS/Android setDefaultEventParameters method. According to documentation, passing nil/null (for iOS/Android respectively) clears all default parameters. While passing an empty dictionary/bundle is a valid operation, it could lead to unintentionally clearing parameters if that was the state before, or it might be a no-op if parameters were already set.

This change modifies the iOS and Android implementations to explicitly check if the processed native parameter collection (NSDictionary for iOS, Bundle for Android) is effectively empty after filtering for valid types. If it is, the call to the native SDK's setDefaultEventParameters method is skipped.

For iOS, this is done by checking `[ns_default_parameters count] == 0`.
For Android, this is done by tracking if any parameter was successfully added to the Bundle using a boolean flag, as a direct `bundle.isEmpty()` check before the native call could miss JNI exceptions during bundle population.

This ensures that providing a C++ map containing exclusively invalid parameters (or an empty map) to SetDefaultEventParameters will be a true no-op from the C++ SDK's perspective, preventing any accidental modification or clearing of existing default parameters on the native side.

---------

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

Author: jonsimantov

analytics/src/analytics_android.cc

@@ -627,6 +627,7 @@ void SetDefaultEventParameters(
     return;
   }
 
+  bool any_parameter_added = false;
   for (const auto& pair : default_parameters) {
     const Variant& value = pair.second;
     const char* key_cstr = pair.first.c_str();
@@ -648,14 +649,18 @@ void SetDefaultEventParameters(
         LogError(
             "SetDefaultEventParameters: Failed to put null string for key: %s",
             key_cstr);
+      } else {
+        any_parameter_added = true;
       }
       env->DeleteLocalRef(key_jstring);
     } else if (value.is_string() || value.is_int64() || value.is_double() ||
                value.is_bool()) {
       // AddVariantToBundle handles these types and their JNI conversions.
       // It also logs if an individual AddToBundle within it fails or if a type
       // is unsupported by it.
-      if (!AddVariantToBundle(env, bundle, key_cstr, value)) {
+      if (AddVariantToBundle(env, bundle, key_cstr, value)) {
+        any_parameter_added = true;
+      } else {
         // This specific log gives context that the failure happened during
         // SetDefaultEventParameters for a type that was expected to be
         // supported by AddVariantToBundle.
@@ -681,6 +686,14 @@ void SetDefaultEventParameters(
     }
   }
 
+  if (!any_parameter_added) {
+    LogDebug(
+        "SetDefaultEventParameters: No valid parameters were processed, "
+        "skipping native call to avoid clearing existing parameters.");
+    env->DeleteLocalRef(bundle);
+    return;
+  }
+
   env->CallVoidMethod(
       g_analytics_class_instance,
       analytics::GetMethodId(analytics::kSetDefaultEventParameters), bundle);

analytics/src/analytics_ios.mm

@@ -403,6 +403,15 @@ void SetDefaultEventParameters(const std::map<std::string, Variant>& default_par
                pair.first.c_str(), Variant::TypeName(value.type()));
     }
   }
+  // If ns_default_parameters is empty at this point, it means all input
+  // parameters were invalid or the input map itself was empty.
+  // In this case, we should not call the native SDK, as passing an empty
+  // NSDictionary might clear existing parameters, which is not the intent
+  // if all inputs were simply invalid.
+  if ([ns_default_parameters count] == 0) {
+    LogDebug("SetDefaultEventParameters: No valid parameters to set, skipping native call.");
+    return;
+  }
   [FIRAnalytics setDefaultEventParameters:ns_default_parameters];
 }