fix(taskAttachment): prevent NPE for deletion of attachments with emp…

…ty or nonexisting taskId related to #3545
camunda · Nov 27, 2024 · 3859a8a · 3859a8a
1 parent 119cbf8
commit 3859a8a
Show file tree

Hide file tree

Showing 3 changed files with 2,892 additions and 2,848 deletions.
diff --git a/engine/pom.xml b/engine/pom.xml
@@ -333,6 +333,10 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-lang3</artifactId>
+    </dependency>
   </dependencies>
 
 

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/DeleteAttachmentCmd.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/DeleteAttachmentCmd.java
@@ -20,6 +20,7 @@
 
 import java.io.Serializable;
 
+import org.apache.commons.lang3.StringUtils;
 import org.camunda.bpm.engine.history.UserOperationLogEntry;
 import org.camunda.bpm.engine.impl.interceptor.Command;
 import org.camunda.bpm.engine.impl.interceptor.CommandContext;
@@ -49,39 +50,45 @@ public DeleteAttachmentCmd(String taskId, String attachmentId) {
 
   public Object execute(CommandContext commandContext) {
     AttachmentEntity attachment = null;
-    if (taskId != null) {
+    if (StringUtils.isNotEmpty(taskId)) {
       attachment = (AttachmentEntity) commandContext
           .getAttachmentManager()
           .findAttachmentByTaskIdAndAttachmentId(taskId, attachmentId);
-      ensureNotNull("No attachment exist for task id '" + taskId + " and attachmentId '" + attachmentId + "'.", "attachment", attachment);
+      ensureNotNull("No attachment exists for task id '" + taskId + " and attachmentId '" + attachmentId + "'.", "attachment", attachment);
     } else {
       attachment = commandContext
           .getDbEntityManager()
           .selectById(AttachmentEntity.class, attachmentId);
-      ensureNotNull("No attachment exist with attachmentId '" + attachmentId + "'.", "attachment", attachment);
+      ensureNotNull("No attachment exists with attachmentId '" + attachmentId + "'.", "attachment", attachment);
     }
 
     commandContext
-      .getDbEntityManager()
-      .delete(attachment);
+        .getDbEntityManager()
+        .delete(attachment);
 
     if (attachment.getContentId() != null) {
       commandContext
-        .getByteArrayManager()
-        .deleteByteArrayById(attachment.getContentId());
+          .getByteArrayManager()
+          .deleteByteArrayById(attachment.getContentId());
     }
 
-    if (attachment.getTaskId()!=null) {
+    if (StringUtils.isNotEmpty(attachment.getTaskId())) {
       TaskEntity task = commandContext
           .getTaskManager()
           .findTaskById(attachment.getTaskId());
 
+      if (task != null) {
       PropertyChange propertyChange = new PropertyChange("name", null, attachment.getName());
 
       commandContext.getOperationLogManager()
           .logAttachmentOperation(UserOperationLogEntry.OPERATION_TYPE_DELETE_ATTACHMENT, task, propertyChange);
 
       task.triggerUpdateEvent();
+      } else {
+        ensureNotNull(
+            "No task exists for attachment '" + attachmentId + "' with taskId '" + attachment.getTaskId() + "'.",
+            "task", task);
+      }
     }
 
     return null;