Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion analytics/src/analytics_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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.
Expand All @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions analytics/src/analytics_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down
Loading