Skip to content

Commit e2995db

Browse files
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.
1 parent f24f016 commit e2995db

File tree

1 file changed

+9
-18
lines changed

1 file changed

+9
-18
lines changed

analytics/src/analytics_android.cc

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,7 @@ void SetDefaultEventParameters(
627627
return;
628628
}
629629

630+
bool any_parameter_added = false;
630631
for (const auto& pair : default_parameters) {
631632
const Variant& value = pair.second;
632633
const char* key_cstr = pair.first.c_str();
@@ -648,14 +649,18 @@ void SetDefaultEventParameters(
648649
LogError(
649650
"SetDefaultEventParameters: Failed to put null string for key: %s",
650651
key_cstr);
652+
} else {
653+
any_parameter_added = true;
651654
}
652655
env->DeleteLocalRef(key_jstring);
653656
} else if (value.is_string() || value.is_int64() || value.is_double() ||
654657
value.is_bool()) {
655658
// AddVariantToBundle handles these types and their JNI conversions.
656659
// It also logs if an individual AddToBundle within it fails or if a type
657660
// is unsupported by it.
658-
if (!AddVariantToBundle(env, bundle, key_cstr, value)) {
661+
if (AddVariantToBundle(env, bundle, key_cstr, value)) {
662+
any_parameter_added = true;
663+
} else {
659664
// This specific log gives context that the failure happened during
660665
// SetDefaultEventParameters for a type that was expected to be
661666
// supported by AddVariantToBundle.
@@ -681,24 +686,10 @@ void SetDefaultEventParameters(
681686
}
682687
}
683688

684-
// Check if the bundle is empty. If so, it means all input parameters were
685-
// invalid or the input map itself was empty. In this case, we should not
686-
// call the native SDK, as passing an empty Bundle might clear existing
687-
// parameters, which is not the intent if all inputs were simply invalid.
688-
jboolean is_bundle_empty = env->CallBooleanMethod(
689-
bundle, util::bundle::GetMethodId(util::bundle::kIsEmpty));
690-
if (util::CheckAndClearJniExceptions(env)) {
691-
LogError(
692-
"SetDefaultEventParameters: Failed to check if Bundle is empty. "
693-
"Skipping native call to avoid potential issues.");
694-
env->DeleteLocalRef(bundle);
695-
return;
696-
}
697-
698-
if (is_bundle_empty) {
689+
if (!any_parameter_added) {
699690
LogDebug(
700-
"SetDefaultEventParameters: No valid parameters to set, skipping "
701-
"native call.");
691+
"SetDefaultEventParameters: No valid parameters were processed, "
692+
"skipping native call to avoid clearing existing parameters.");
702693
env->DeleteLocalRef(bundle);
703694
return;
704695
}

0 commit comments

Comments
 (0)