Fix usage of malware scanner with non-HANA storage (#557)

Co-authored-by: Markus Ofterdinger <[email protected]>

Author: lisajulia

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java

@@ -95,11 +95,10 @@ void processAfter(CdsReadEventContext context, List<CdsData> data) {
           (path, element, value) -> {
             Attachments attachment = Attachments.of(path.target().values());
             InputStream content = attachment.getContent();
-            boolean contentExists = nonNull(content);
-            if (nonNull(attachment.getContentId()) || contentExists) {
-              verifyStatus(path, attachment, contentExists);
+            if (nonNull(attachment.getContentId())) {
+              verifyStatus(path, attachment);
               Supplier<InputStream> supplier =
-                  contentExists
+                  nonNull(content)
                       ? () -> content
                       : () -> attachmentService.readAttachment(attachment.getContentId());
               return new LazyProxyInputStream(supplier, statusValidator, attachment.getStatus());
@@ -147,19 +146,20 @@ private List<String> getAttachmentAssociations(
     return associationNames;
   }
 
-  private void verifyStatus(Path path, Attachments attachment, boolean contentExists) {
+  private void verifyStatus(Path path, Attachments attachment) {
     if (areKeysEmpty(path.target().keys())) {
+      String currentStatus = attachment.getStatus();
       logger.debug(
           "In verify status for content id {} and status {}",
           attachment.getContentId(),
-          attachment.getStatus());
-      if ((StatusCode.UNSCANNED.equals(attachment.getStatus())
-              || StatusCode.SCANNING.equals(attachment.getStatus()))
-          && contentExists) {
+          currentStatus);
+      if (StatusCode.UNSCANNED.equals(currentStatus)
+          || StatusCode.SCANNING.equals(currentStatus)
+          || currentStatus == null) {
         logger.debug(
             "Scanning content with ID {} for malware, has current status {}",
             attachment.getContentId(),
-            attachment.getStatus());
+            currentStatus);
         scanExecutor.scanAsync(path.target().entity(), attachment.getContentId());
       }
       statusValidator.verifyStatus(attachment.getStatus());

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/DefaultAttachmentsServiceHandler.java

@@ -13,9 +13,11 @@
 import com.sap.cds.feature.attachments.service.model.servicehandler.AttachmentRestoreEventContext;
 import com.sap.cds.services.changeset.ChangeSetListener;
 import com.sap.cds.services.handler.EventHandler;
+import com.sap.cds.services.handler.annotations.After;
 import com.sap.cds.services.handler.annotations.HandlerOrder;
 import com.sap.cds.services.handler.annotations.On;
 import com.sap.cds.services.handler.annotations.ServiceName;
+import java.util.Objects;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -35,33 +37,43 @@ public class DefaultAttachmentsServiceHandler implements EventHandler {
   private static final Logger logger =
       LoggerFactory.getLogger(DefaultAttachmentsServiceHandler.class);
 
-  private final EndTransactionMalwareScanProvider endTransactionMalwareScanProvider;
+  private final EndTransactionMalwareScanProvider malwareScanProvider;
 
-  public DefaultAttachmentsServiceHandler(
-      EndTransactionMalwareScanProvider endTransactionMalwareScanProvider) {
-    this.endTransactionMalwareScanProvider = endTransactionMalwareScanProvider;
+  public DefaultAttachmentsServiceHandler(EndTransactionMalwareScanProvider malwareScanProvider) {
+    this.malwareScanProvider =
+        Objects.requireNonNull(malwareScanProvider, "malwareScanProvider must not be null");
   }
 
   @On
   @HandlerOrder(DEFAULT_ON)
-  public void createAttachment(AttachmentCreateEventContext context) {
+  void createAttachment(AttachmentCreateEventContext context) {
     logger.debug(
         "Default Attachment Service handler called for creating attachment for entity '{}'",
         context.getAttachmentEntity().getQualifiedName());
     String contentId = (String) context.getAttachmentIds().get(Attachments.ID);
     context.getData().setStatus(StatusCode.SCANNING);
-    ChangeSetListener listener =
-        endTransactionMalwareScanProvider.getChangeSetListener(
-            context.getAttachmentEntity(), contentId);
-    context.getChangeSetContext().register(listener);
     context.setIsInternalStored(true);
     context.setContentId(contentId);
     context.setCompleted();
   }
 
+  /**
+   * After the attachment is created, register a {@link ChangeSetListener} to perform a malware scan
+   * at the end of the transaction.
+   *
+   * @param context the attachment creation event context
+   */
+  @After
+  void afterCreateAttachment(AttachmentCreateEventContext context) {
+    ChangeSetListener listener =
+        malwareScanProvider.getChangeSetListener(
+            context.getAttachmentEntity(), context.getContentId());
+    context.getChangeSetContext().register(listener);
+  }
+
   @On
   @HandlerOrder(DEFAULT_ON)
-  public void markAttachmentAsDeleted(AttachmentMarkAsDeletedEventContext context) {
+  void markAttachmentAsDeleted(AttachmentMarkAsDeletedEventContext context) {
     logger.debug(
         "Default Attachment Service handler called for marking attachment as deleted with document id {}",
         context.getContentId());
@@ -72,7 +84,7 @@ public void markAttachmentAsDeleted(AttachmentMarkAsDeletedEventContext context)
 
   @On
   @HandlerOrder(DEFAULT_ON)
-  public void restoreAttachment(AttachmentRestoreEventContext context) {
+  void restoreAttachment(AttachmentRestoreEventContext context) {
     logger.debug(
         "Default Attachment Service handler called for restoring attachment for timestamp {}",
         context.getRestoreTimestamp());
@@ -83,7 +95,7 @@ public void restoreAttachment(AttachmentRestoreEventContext context) {
 
   @On
   @HandlerOrder(DEFAULT_ON)
-  public void readAttachment(AttachmentReadEventContext context) {
+  void readAttachment(AttachmentReadEventContext context) {
     logger.debug(
         "Default Attachment Service handler called for reading attachment with document id {}",
         context.getContentId());

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/handler/transaction/EndTransactionMalwareScanRunner.java

@@ -61,8 +61,7 @@ private void startScanning(CdsEntity attachmentEntityToScan, String contentId) {
           runner.run(
               resourceCtx -> {
                 // ensure that DB transaction is still active until the content is completely read
-                // from InputStream and
-                // scanned by malware scanner
+                // from InputStream and scanned by malware scanner
                 runtime
                     .changeSetContext()
                     .run(

cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandlerTest.java

@@ -128,9 +128,10 @@ void dataFilledWithDeepStructure() throws IOException {
       item4.setId("item id4");
       item4.setAttachments(List.of(attachmentWithStreamAsContent));
       var attachmentWithStreamContentButWithoutContentId = Attachments.create();
-      attachmentWithStreamContentButWithoutContentId.setContent(mock(InputStream.class));
+      var inputMock = mock(InputStream.class);
+      attachmentWithStreamContentButWithoutContentId.setContent(inputMock);
       var item5 = Items.create();
-      item5.setId("item id4");
+      item5.setId("item id5");
       item5.setAttachments(List.of(attachmentWithStreamContentButWithoutContentId));
       var root1 = RootTable.create();
       root1.setItemTable(List.of(item2, item1, item4, item5));
@@ -147,8 +148,7 @@ void dataFilledWithDeepStructure() throws IOException {
       assertThat(attachmentWithoutContentField.getContent()).isNull();
       assertThat(attachmentWithStreamAsContent.getContent())
           .isInstanceOf(LazyProxyInputStream.class);
-      assertThat(attachmentWithStreamContentButWithoutContentId.getContent())
-          .isInstanceOf(LazyProxyInputStream.class);
+      assertThat(attachmentWithStreamContentButWithoutContentId.getContent()).isEqualTo(inputMock);
       verifyNoInteractions(attachmentService);
     }
   }
@@ -227,7 +227,7 @@ void scannerCalledForUnscannedAttachments() {
   }
 
   @Test
-  void scannerNotCalledForUnscannedAttachmentsIfNoContentProvided() {
+  void scannerCalledForUnscannedAttachmentsIfNoContentProvided() {
     mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class));
     var attachment = Attachments.create();
     attachment.setContentId("some ID");
@@ -236,7 +236,8 @@ void scannerNotCalledForUnscannedAttachmentsIfNoContentProvided() {
 
     cut.processAfter(readEventContext, List.of(attachment));
 
-    verifyNoInteractions(asyncMalwareScanExecutor);
+    verify(asyncMalwareScanExecutor)
+        .scanAsync(readEventContext.getTarget(), attachment.getContentId());
   }
 
   @Test

cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/service/handler/AttachmentsServiceImplHandlerTest.java

@@ -103,7 +103,7 @@ void classHasCorrectAnnotation() {
   @Test
   void createMethodHasCorrectAnnotation() throws NoSuchMethodException {
     var createMethod =
-        cut.getClass().getMethod("createAttachment", AttachmentCreateEventContext.class);
+        cut.getClass().getDeclaredMethod("createAttachment", AttachmentCreateEventContext.class);
     var onAnnotation = createMethod.getAnnotation(On.class);
     var handlerOrderAnnotation = createMethod.getAnnotation(HandlerOrder.class);
 
@@ -114,7 +114,7 @@ void createMethodHasCorrectAnnotation() throws NoSuchMethodException {
   @Test
   void restoreAttachmentMethodHasCorrectAnnotation() throws NoSuchMethodException {
     var updateMethod =
-        cut.getClass().getMethod("restoreAttachment", AttachmentRestoreEventContext.class);
+        cut.getClass().getDeclaredMethod("restoreAttachment", AttachmentRestoreEventContext.class);
     var onAnnotation = updateMethod.getAnnotation(On.class);
     var handlerOrderAnnotation = updateMethod.getAnnotation(HandlerOrder.class);
 
@@ -126,7 +126,8 @@ void restoreAttachmentMethodHasCorrectAnnotation() throws NoSuchMethodException
   void deleteMethodHasCorrectAnnotation() throws NoSuchMethodException {
     var deleteMethod =
         cut.getClass()
-            .getMethod("markAttachmentAsDeleted", AttachmentMarkAsDeletedEventContext.class);
+            .getDeclaredMethod(
+                "markAttachmentAsDeleted", AttachmentMarkAsDeletedEventContext.class);
     var onAnnotation = deleteMethod.getAnnotation(On.class);
     var handlerOrderAnnotation = deleteMethod.getAnnotation(HandlerOrder.class);
 
@@ -136,7 +137,8 @@ void deleteMethodHasCorrectAnnotation() throws NoSuchMethodException {
 
   @Test
   void readMethodHasCorrectAnnotation() throws NoSuchMethodException {
-    var readMethod = cut.getClass().getMethod("readAttachment", AttachmentReadEventContext.class);
+    var readMethod =
+        cut.getClass().getDeclaredMethod("readAttachment", AttachmentReadEventContext.class);
     var onAnnotation = readMethod.getAnnotation(On.class);
     var handlerOrderAnnotation = readMethod.getAnnotation(HandlerOrder.class);
 
@@ -156,6 +158,7 @@ void malwareScannerRegisteredForEndOfTransaction() {
     ChangeSetContextImpl.open(false);
 
     cut.createAttachment(createContext);
+    cut.afterCreateAttachment(createContext);
 
     verify(malwareScanProvider).getChangeSetListener(entity, "contentId");
   }

doc/CHANGELOG.md

@@ -14,6 +14,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
 ### Fixed
 
+- Fixed an issue where the Malware Scanning Service is not triggered after uploading an Attachment, if the target storage is other than the default DB storage. (#557)
+
 ## Version 1.2.0 - 2025-08-26
 
 ### Added

storage-targets/cds-feature-attachments-fs/src/main/java/com/sap/cds/feature/attachments/fs/handler/FSAttachmentsServiceHandler.java

@@ -56,7 +56,7 @@ void createAttachment(AttachmentCreateEventContext context) throws IOException {
     String contentId = (String) context.getAttachmentIds().get(Attachments.ID);
 
     MediaData data = context.getData();
-    data.setStatus(StatusCode.CLEAN);
+    data.setStatus(StatusCode.SCANNING);
 
     try (InputStream input = data.getContent()) {
       Path contentPath = getContentPath(context, contentId);

storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/handler/OSSAttachmentsServiceHandler.java

@@ -5,6 +5,7 @@
 
 import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.Attachments;
 import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.MediaData;
+import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.StatusCode;
 import com.sap.cds.feature.attachments.oss.client.AWSClient;
 import com.sap.cds.feature.attachments.oss.client.AzureClient;
 import com.sap.cds.feature.attachments.oss.client.GoogleClient;
@@ -124,6 +125,7 @@ void createAttachment(AttachmentCreateEventContext context)
     try {
       osClient.uploadContent(data.getContent(), contentId, data.getMimeType()).get();
       logger.info("Uploaded file {}", fileName);
+      context.getData().setStatus(StatusCode.SCANNING);
       context.setIsInternalStored(false);
       context.setContentId(contentId);
       context.setCompleted();