From d538c6949b1d807da5527c815c1726e7b66ee0bb Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Wed, 26 Nov 2025 15:21:39 +0100 Subject: [PATCH 1/2] fix exception overwriting --- .../readhelper/LazyProxyInputStream.java | 16 +++- .../readhelper/LazyProxyInputStreamTest.java | 73 +++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java index 850b588f..11016d90 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java @@ -58,11 +58,21 @@ public void close() throws IOException { } private InputStream getDelegate() { - attachmentStatusValidator.verifyStatus(status); - if (delegate == null) { logger.debug("Creating delegate input stream"); - delegate = inputStreamSupplier.get(); + try { + delegate = inputStreamSupplier.get(); + attachmentStatusValidator.verifyStatus(status); + } catch (RuntimeException originalException) { + try { + attachmentStatusValidator.verifyStatus(status); + throw originalException; + } catch (AttachmentStatusException statusException) { + // Add status validation error as suppressed exception to preserve context + originalException.addSuppressed(statusException); + throw originalException; + } + } } return delegate; } diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java index 0413e96a..e5140c95 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java @@ -16,6 +16,7 @@ import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.StatusCode; import com.sap.cds.feature.attachments.service.AttachmentService; +import com.sap.cds.services.ServiceException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; import java.io.InputStream; @@ -137,4 +138,76 @@ void noExceptionIfCorrectStatus() { assertDoesNotThrow(() -> cut.read()); } + + @Test + void originalExceptionPreservedWhenSupplierFails() { + // Test the core fix: original exception from supplier should be preserved + // even when status validation also fails + ServiceException originalException = new ServiceException("Failed to retrieve document content for file with ID: test123"); + AttachmentStatusException statusException = AttachmentStatusException.getNotCleanException(); + + // Mock supplier to throw original exception + when(attachmentService.readAttachment(any())).thenThrow(originalException); + // Mock status validator to also throw exception + doThrow(statusException).when(attachmentStatusValidator).verifyStatus(StatusCode.INFECTED); + + cut = new LazyProxyInputStream( + () -> attachmentService.readAttachment(any()), + attachmentStatusValidator, + StatusCode.INFECTED); + + // The original exception should be thrown, not the status exception + ServiceException thrownException = assertThrows(ServiceException.class, () -> cut.read()); + + // Verify the original exception is preserved + assertThat(thrownException).isEqualTo(originalException); + assertThat(thrownException.getMessage()).isEqualTo("Failed to retrieve document content for file with ID: test123"); + + // Verify the status exception is added as suppressed exception for context + assertThat(thrownException.getSuppressed()).hasSize(1); + assertThat(thrownException.getSuppressed()[0]).isInstanceOf(AttachmentStatusException.class); + } + + @Test + void originalExceptionPreservedWhenStatusValidationPasses() { + // Test edge case: supplier fails but status validation passes + // In this case, we still want the original exception, not status exception + ServiceException originalException = new ServiceException("Network connection failed"); + + // Mock supplier to throw original exception + when(attachmentService.readAttachment(any())).thenThrow(originalException); + // Status validator should pass (no exception thrown) + + cut = new LazyProxyInputStream( + () -> attachmentService.readAttachment(any()), + attachmentStatusValidator, + StatusCode.CLEAN); + + // The original exception should be thrown + ServiceException thrownException = assertThrows(ServiceException.class, () -> cut.read()); + + // Verify the original exception is preserved + assertThat(thrownException).isEqualTo(originalException); + assertThat(thrownException.getMessage()).isEqualTo("Network connection failed"); + + // No suppressed exceptions should be present since status validation passed + assertThat(thrownException.getSuppressed()).isEmpty(); + } + + @Test + void statusValidationOnlyWhenSupplierSucceeds() { + // Test that status validation happens after successful stream creation + // This ensures we don't pre-emptively block access due to status when the underlying issue might be different + + cut = new LazyProxyInputStream( + () -> attachmentService.readAttachment(any()), + attachmentStatusValidator, + StatusCode.CLEAN); + + assertDoesNotThrow(() -> cut.read()); + + // Verify status was validated after supplier was called + verify(attachmentService).readAttachment(any()); + verify(attachmentStatusValidator).verifyStatus(StatusCode.CLEAN); + } } From fe8a87d3378c1c20789a3f91b38287dede1f16c0 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Thu, 27 Nov 2025 09:46:38 +0100 Subject: [PATCH 2/2] remove comment --- .../applicationservice/readhelper/LazyProxyInputStream.java | 1 - 1 file changed, 1 deletion(-) diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java index 11016d90..0a533ad1 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java @@ -68,7 +68,6 @@ private InputStream getDelegate() { attachmentStatusValidator.verifyStatus(status); throw originalException; } catch (AttachmentStatusException statusException) { - // Add status validation error as suppressed exception to preserve context originalException.addSuppressed(statusException); throw originalException; }