From 5d5f7ae6657204422ab7ca4dc9447aba0b649618 Mon Sep 17 00:00:00 2001 From: Nilesh Sarupriya Date: Wed, 11 Jun 2025 20:23:36 +0530 Subject: [PATCH] fix: enhance null handling in action parameter evaluation and rendering (#40910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This update improves the handling of null values in action parameter evaluation and rendering processes. Specifically, it ensures that null values are treated appropriately, preventing unnecessary stringification and Blob creation. This change enhances the robustness of the action execution flow and prevents potential errors related to null values. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: b869dfa397737ef46a940ed82d0f7b92d1518bc4 > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Tue, 10 Jun 2025 10:21:02 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of null and empty parameter values during action execution, ensuring that nulls are preserved and not incorrectly converted to strings or empty values. - Enhanced template rendering to better preserve original formatting when bindings are missing. - **New Features** - Added new logic to sanitize null parameter values before action execution for more consistent processing. --------- Co-authored-by: Nilesh Sarupriya <20905988+nsarupr@users.noreply.github.com> --- .../sagas/ActionExecution/PluginActionSaga.ts | 17 ++++++-- .../external/helpers/MustacheHelper.java | 15 ++++--- .../external/plugins/PluginExecutor.java | 41 +++++++++++++------ .../ce/ActionExecutionSolutionCEImpl.java | 15 ++++++- 4 files changed, 66 insertions(+), 22 deletions(-) diff --git a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts index 1205c8c0fd..be93d16047 100644 --- a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts +++ b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts @@ -489,14 +489,25 @@ function* evaluateActionParams( evaluatedParams[key] = "blob"; } - value = JSON.stringify(value); - evaluatedParams[key] = value; + // Handle null values separately to avoid stringifying them + if (value === null) { + value = null; + evaluatedParams[key] = null; + } else { + value = JSON.stringify(value); + evaluatedParams[key] = value; + } } // If there are no blob urls in the value, we can directly add it to the formData // If there are blob urls, we need to add them to the blobDataMap if (!useBlobMaps) { - value = new Blob([value], { type: "text/plain" }); + // Handle null values separately to avoid creating a Blob with "null" string + if (value === null) { + value = null; + } else { + value = new Blob([value], { type: "text/plain" }); + } } bindingsMap[key] = `k${i}`; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java index 3564e48ea1..620ba00ed9 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java @@ -341,13 +341,18 @@ public class MustacheHelper { // text // and hence reflecting the value in the rendered string as is. // Example: {{Input.text}} = "This whole string is the value of Input1.text. Even this {{one}}." - String bindingValue = keyValueMap.get(token.getValue() + String tokenSubstring = token.getValue() .substring(2, token.getValue().length() - 2) - .trim()); - if (bindingValue != null) { - rendered.append(bindingValue); - } else { + .trim(); + String bindingValue = keyValueMap.get(tokenSubstring); + if (!keyValueMap.containsKey(tokenSubstring)) { rendered.append(token.getValue()); + } else if (bindingValue != null) { + // If the binding value is not null, then append the binding value to the rendered string. + // We are using the token.getValue() here to get the original value of the binding. + // This is because we want to preserve the original formatting of the binding. + // For example, if the binding is {{Input.text}}, we want to preserve the {{}} around it. + rendered.append(bindingValue); } } else { rendered.append(token.getValue()); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java index c641bbc75f..7b7063e112 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java @@ -22,12 +22,13 @@ import reactor.core.publisher.Mono; import reactor.core.scheduler.Schedulers; import reactor.util.function.Tuple2; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; -import java.util.stream.Collectors; import static com.appsmith.external.constants.spans.ActionSpan.ACTION_EXECUTION_PLUGIN_EXECUTION; import static com.appsmith.external.helpers.PluginUtils.getHintMessageForLocalhostUrl; @@ -304,6 +305,7 @@ public interface PluginExecutor extends ExtensionPoint, CrudTemplateService { ActionConfiguration actionConfiguration, ObservationRegistry observationRegistry, Map featureFlagMap) { + this.sanitiseNullsInParams(executeActionDTO); return this.executeParameterizedWithFlags( connection, executeActionDTO, datasourceConfiguration, actionConfiguration, featureFlagMap) .tag("plugin", this.getClass().getName()) @@ -311,6 +313,17 @@ public interface PluginExecutor extends ExtensionPoint, CrudTemplateService { .tap(Micrometer.observation(observationRegistry)); } + default void sanitiseNullsInParams(ExecuteActionDTO executeActionDTO) { + String stringNull = "null"; + if (executeActionDTO != null && !isEmpty(executeActionDTO.getParams())) { + executeActionDTO.getParams().stream().forEach(param -> { + if (Objects.isNull(param.getValue())) { + param.setValue(stringNull); + } + }); + } + } + default Mono triggerWithFlags( C connection, DatasourceConfiguration datasourceConfiguration, @@ -377,17 +390,21 @@ public interface PluginExecutor extends ExtensionPoint, CrudTemplateService { // Do variable substitution // Do this only if params have been provided in the execute command if (executeActionDTO != null && !isEmpty(executeActionDTO.getParams())) { - Map replaceParamsMap = executeActionDTO.getParams().stream() - .collect(Collectors.toMap( - // Trimming here for good measure. If the keys have space on either side, - // Mustache won't be able to find the key. - // We also add a backslash before every double-quote or backslash character - // because we apply the template replacing in a JSON-stringified version of - // these properties, where these two characters are escaped. - p -> p.getKey().trim(), // .replaceAll("[\"\n\\\\]", "\\\\$0"), - Param::getValue, - // In case of a conflict, we pick the older value - (oldValue, newValue) -> oldValue)); + Map replaceParamsMap = new HashMap<>(); + for (Param p : executeActionDTO.getParams()) { + if (p != null && p.getKey() != null) { + String key = p.getKey().trim(); + // We also add a backslash before every double-quote or backslash character + // because we apply the template replacing in a JSON-stringified version of + // these properties, where these two characters are escaped. + // The original comment about .replaceAll("["\n\\]", "\\$0") for the key + // is preserved here for context, though not directly applied in this simplified key trim. + String value = p.getValue(); // This will now correctly store null if p.getValue() is null + + // In case of a conflict, putIfAbsent picks the older value (the one already in the map) + replaceParamsMap.putIfAbsent(key, value); + } + } MustacheHelper.renderFieldValues(datasourceConfiguration, replaceParamsMap); MustacheHelper.renderFieldValues(actionConfiguration, replaceParamsMap); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java index 4a51d65d06..5629e217a3 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java @@ -63,6 +63,7 @@ import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang3.ObjectUtils; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.http.HttpHeaders; +import org.springframework.http.codec.multipart.FormFieldPart; import org.springframework.http.codec.multipart.Part; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; @@ -341,8 +342,8 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE @Override public Mono executeAction( ExecuteActionDTO executeActionDTO, ExecuteActionMetaDTO executeActionMetaDTO) { - // 1. Validate input parameters which are required for mustache replacements - replaceNullWithQuotesForParamValues(executeActionDTO.getParams()); + + // Earlier, here we were replacing null with quotes for param values. AtomicReference actionName = new AtomicReference<>(); actionName.set(""); @@ -505,6 +506,16 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE protected Mono parseExecuteParameter(Part part, AtomicLong totalReadableByteCount) { final Param param = new Param(); param.setPseudoBindingName(part.name()); + + if (part instanceof FormFieldPart) { + FormFieldPart formFieldPart = (FormFieldPart) part; + if ("null".equals(formFieldPart.value())) { + param.setValue(null); + } else if (formFieldPart.value().isEmpty()) { + param.setValue(formFieldPart.value()); + } + return Mono.just(param); + } return DataBufferUtils.join(part.content()).map(dataBuffer -> { byte[] bytes = new byte[dataBuffer.readableByteCount()]; totalReadableByteCount.addAndGet(dataBuffer.readableByteCount());