Merge pull request #359 from pagopa/develop

* fix: [RTD-540] use nio library to create temp file (#356) * fix: [RTD-540] solve bomb zip security hotspot (#357) * [RTD-540] solve zip bomb by checking uncompressed file size * [RTD-540] solve zip bomb by checking uncompressed file size * [RTD-540] solve zip slip vulnerability * [RTD-540] add compression ratio check * [RTD-540] typo * [RTD-540] utilize copyLarge of apache common * [RTD-540] remove common io library usage * [RTD-540] reduce cognitive complexity * [RTD-540] remove codeql action * fix: [RTD-1655] reduce log severity on ack confirm failure (#358) * fix: [RTD-540] use nio library to create temp file (#356) * fix: [RTD-540] solve bomb zip security hotspot (#357) * [RTD-540] solve zip bomb by checking uncompressed file size * [RTD-540] solve zip bomb by checking uncompressed file size * [RTD-540] solve zip slip vulnerability * [RTD-540] add compression ratio check * [RTD-540] typo * [RTD-540] utilize copyLarge of apache common * [RTD-540] remove common io library usage * [RTD-540] reduce cognitive complexity * [RTD-540] remove codeql action * [RTD-1655] reduce log severity on ack confirm failure * [RTD-1655] bump pom version to 2.0.3 * fix: [RTD-1669] fix conditional flow after ack list retrieve fails (#360) * [RTD-1669] fix conditional flow on ade-ack step failure * [RTD-1669] improve failure handling and logging
pagopa · May 12, 2023 · 9b9940a · 9b9940a
2 parents 84d8459 + 592799c
commit 9b9940a
Show file tree

Hide file tree

Showing 22 changed files with 422 additions and 153 deletions.
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
diff --git a/acceptance_tests/splitting_ade_batch_service/script_splitting.bash b/acceptance_tests/splitting_ade_batch_service/script_splitting.bash
@@ -76,7 +76,7 @@ else
 fi
 
 # cli atm do not support chunk generation, it can be arranged like this:
-LOCAL_OUTPUT_DIFF=$(diff <(cat cstar-cli/generated/ADE.*expected | sort) <(cat workdir/output/ADE.*.csv | sort | tail -n +$N_CHUNKS))
+LOCAL_OUTPUT_DIFF=$(diff <(cat cstar-cli/generated/ADE.*expected | sort) <(cat workdir/output/ADE.*.csv | sort | tail -n +$((N_CHUNKS+1))))
 
 if [ -z "$LOCAL_OUTPUT_DIFF" ]
 then

diff --git a/acceptance_tests/splitting_rtd_batch_service/script_splitting_rtd.bash b/acceptance_tests/splitting_rtd_batch_service/script_splitting_rtd.bash
@@ -75,4 +75,7 @@ else
 	exit 2
 fi
 
+rm -rf cstar-cli
+rm -rf workdir
+
 exit 0
diff --git a/api/batch/pom.xml b/api/batch/pom.xml
@@ -4,11 +4,11 @@
     <parent>
         <artifactId>rtd-ms-transaction-filter-api</artifactId>
         <groupId>it.gov.pagopa.rtd.ms.transaction_filter.api</groupId>
-        <version>2.0.2</version>
+        <version>2.0.3</version>
     </parent>
 
     <artifactId>rtd-ms-transaction-filter-api-batch</artifactId>
-    <version>2.0.2</version>
+    <version>2.0.3</version>
 
     <dependencies>
         <dependency>

diff --git a/...atch/src/main/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatch.java b/...atch/src/main/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatch.java
@@ -333,7 +333,7 @@ public FlowJobBuilder transactionJobBuilder() {
                 .on(FAILED).to(fileManagementTask())
                 .from(transactionFilterStep.transactionSenderRtdMasterStep(this.hpanConnectorService, this.storeService))
                 .on("*").to(senderAdeAckFilesRecoveryTask())
-                .next(pendingStepDecider(sendPendingFilesStepEnabled))
+                .on("*").to(pendingStepDecider(sendPendingFilesStepEnabled))
                 .on(TRUE).to(transactionFilterStep.transactionSenderPendingMasterStep(this.hpanConnectorService))
                 .from(pendingStepDecider(sendPendingFilesStepEnabled))
                 .on(FALSE).to(fileManagementTask())

diff --git a/.../src/test/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatchTest.java b/.../src/test/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatchTest.java
@@ -47,6 +47,7 @@
 import org.junit.runner.RunWith;
 import org.mockito.BDDMockito;
 import org.mockito.Mockito;
+import org.springframework.batch.core.BatchStatus;
 import org.springframework.batch.core.ExitStatus;
 import org.springframework.batch.core.JobExecution;
 import org.springframework.batch.core.JobParameters;
@@ -348,6 +349,29 @@ public void whenSenderAdeAckTaskletIsSetThenMatchExpectedOutputFiles() {
             .hasContent("7890123;stringdefault");
     }
 
+    @SneakyThrows
+    @Test
+    public void whenSenderAdeAckTaskletFailsThenExecuteFileManagementStep() {
+        String publicKey = createPublicKey();
+        createPanPGP();
+        createTrnOutputFile();
+        createAdeOutputFile();
+        BDDMockito.doReturn(publicKey).when(storeServiceSpy).getKey("pagopa");
+        BDDMockito.when(senderAdeAckRestClient.getSenderAdeAckFiles()).thenThrow(RuntimeException.class);
+
+        JobExecution jobExecution = jobLauncherTestUtils.launchJob(new JobParametersBuilder()
+            .addDate("startDateTime", new Date())
+            .toJobParameters());
+        assertThat(jobExecution.getExitStatus()).isEqualTo(ExitStatus.FAILED);
+        assertThat(jobExecution.getStatus()).isEqualTo(BatchStatus.COMPLETED);
+
+        Collection<String> stepNamesExecuted = jobExecution.getStepExecutions().stream()
+            .map(StepExecution::getStepName)
+            .collect(Collectors.toSet());
+
+        assertThat(stepNamesExecuted).contains("transaction-filter-file-management-step");
+    }
+
     @SneakyThrows
     @Test
     public void jobExecutionFails() {

diff --git a/api/pom.xml b/api/pom.xml
@@ -4,12 +4,12 @@
     <parent>
         <artifactId>rtd-ms-transaction-filter</artifactId>
         <groupId>it.gov.pagopa.rtd.ms</groupId>
-        <version>2.0.2</version>
+        <version>2.0.3</version>
     </parent>
 
     <groupId>it.gov.pagopa.rtd.ms.transaction_filter.api</groupId>
     <artifactId>rtd-ms-transaction-filter-api</artifactId>
-    <version>2.0.2</version>
+    <version>2.0.3</version>
 
     <packaging>pom</packaging>
 

diff --git a/app/pom.xml b/app/pom.xml
@@ -4,12 +4,12 @@
     <parent>
         <artifactId>rtd-ms-transaction-filter</artifactId>
         <groupId>it.gov.pagopa.rtd.ms</groupId>
-        <version>2.0.2</version>
+        <version>2.0.3</version>
     </parent>
 
     <groupId>it.gov.pagopa.rtd.ms.transaction_filter</groupId>
     <artifactId>transaction-filter-app</artifactId>
-    <version>2.0.2</version>
+    <version>2.0.3</version>
 
     <dependencies>
         <dependency>

diff --git a/app/src/main/resources/config/application.yml b/app/src/main/resources/config/application.yml
@@ -116,7 +116,7 @@ batchConfiguration:
 rest-client:
   user-agent:
     prefix: BatchService
-    version: 2.0.2
+    version: 2.0.3
   hpan:
     serviceCode: hpan-service
     base-url: ${HPAN_SERVICE_URL:https://bpd-dev.azure-api.net:${HPAN_SERVICE_PORT:443}}

diff --git a/core/pom.xml b/core/pom.xml
@@ -4,12 +4,12 @@
     <parent>
         <artifactId>rtd-ms-transaction-filter</artifactId>
         <groupId>it.gov.pagopa.rtd.ms</groupId>
-        <version>2.0.2</version>
+        <version>2.0.3</version>
      </parent>
 
     <groupId>it.gov.pagopa.rtd.ms.transaction_filter</groupId>
     <artifactId>rtd-ms-transaction-filter-core</artifactId>
-    <version>2.0.2</version>
+    <version>2.0.3</version>
 
     <dependencies>
         <dependency>

diff --git a/integration/jpa/pom.xml b/integration/jpa/pom.xml
@@ -4,12 +4,12 @@
     <parent>
         <artifactId>rtd-ms-transaction-filter-integration</artifactId>
         <groupId>it.gov.pagopa.rtd.ms.transaction_filter</groupId>
-        <version>2.0.2</version>
+        <version>2.0.3</version>
     </parent>
 
     <groupId>it.gov.pagopa.rtd.ms.transaction_filter.integration</groupId>
     <artifactId>rtd-ms-transaction-filter-integration-jpa</artifactId>
-    <version>2.0.2</version>
+    <version>2.0.3</version>
 
     <dependencies>
         <dependency>

diff --git a/integration/pom.xml b/integration/pom.xml
@@ -4,12 +4,12 @@
     <parent>
         <artifactId>rtd-ms-transaction-filter</artifactId>
         <groupId>it.gov.pagopa.rtd.ms</groupId>
-        <version>2.0.2</version>
+        <version>2.0.3</version>
     </parent>
 
     <groupId>it.gov.pagopa.rtd.ms.transaction_filter</groupId>
     <artifactId>rtd-ms-transaction-filter-integration</artifactId>
-    <version>2.0.2</version>
+    <version>2.0.3</version>
 
     <packaging>pom</packaging>
 

diff --git a/integration/rest/pom.xml b/integration/rest/pom.xml
@@ -4,12 +4,12 @@
     <parent>
         <groupId>it.gov.pagopa.rtd.ms.transaction_filter</groupId>
         <artifactId>rtd-ms-transaction-filter-integration</artifactId>
-        <version>2.0.2</version>
+        <version>2.0.3</version>
     </parent>
 
     <groupId>it.gov.pagopa.rtd.ms.transaction_filter.integration</groupId>
     <artifactId>rtd-ms-transaction-filter-integration-rest</artifactId>
-    <version>2.0.2</version>
+    <version>2.0.3</version>
 
     <dependencies>
         <dependency>

diff --git a/...rest/src/main/java/it/gov/pagopa/rtd/transaction_filter/connector/HpanRestClientImpl.java b/...rest/src/main/java/it/gov/pagopa/rtd/transaction_filter/connector/HpanRestClientImpl.java
@@ -1,12 +1,29 @@
 package it.gov.pagopa.rtd.transaction_filter.connector;
 
 import it.gov.pagopa.rtd.transaction_filter.connector.config.HpanRestConnectorConfig;
+import it.gov.pagopa.rtd.transaction_filter.utils.HpanUnzipper;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.InvalidParameterException;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import javax.net.ssl.SSLContext;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.IOUtils;
 import org.apache.http.Header;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
@@ -25,25 +42,6 @@
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Service;
 import org.springframework.util.StreamUtils;
-import javax.net.ssl.SSLContext;
-
-import java.io.*;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.security.InvalidParameterException;
-import java.time.LocalDateTime;
-import java.time.ZoneId;
-import java.time.ZonedDateTime;
-import java.time.format.DateTimeFormatter;
-import java.time.temporal.ChronoUnit;
-import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.List;
-import java.util.Objects;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipFile;
 
 /**
  * Implementation for {@link HpanRestClient}
@@ -127,7 +125,7 @@ class HpanRestClientImpl implements HpanRestClient {
   @Override
   public File getList() {
 
-    tempFile = File.createTempFile("hpanDownloadFile", "");
+    tempFile = Files.createTempFile("hpanDownloadFile", "").toFile();
     File localTempFile = tempFile;
     ResponseEntity<Resource> responseEntity = hpanRestConnector.getList(apiKey);
 
@@ -194,41 +192,20 @@ private void copyZippedFileIntoTempFile(Resource zipFile) {
   @SneakyThrows
   private File extractZipFile(File tempFile) {
 
-    File localTempFile = null;
-
-    try (ZipFile zipFile = new ZipFile(tempFile)) {
-
-      tempDirWithPrefix = Files.createTempDirectory("hpanTempFolder");
-      Enumeration<? extends ZipEntry> enumeration = zipFile.entries();
-
-      while (enumeration.hasMoreElements()) {
-
-        ZipEntry zipEntry = enumeration.nextElement();
-        File newFile = new File(
-            tempDirWithPrefix.toFile().getAbsolutePath() +
-                File.separator + zipEntry.getName());
-
-        if (!isFilenameValidInZipFile(zipEntry.getName())) {
-          throw new IOException("Illegal filename in archive: " + zipEntry.getName());
-        }
-
-        try (InputStream zipEntryIS = zipFile.getInputStream(zipEntry);
-            FileOutputStream tempFileFOS = new FileOutputStream(newFile)) {
-
-          new File(newFile.getParent()).mkdirs();
-
-          IOUtils.copy(zipEntryIS, tempFileFOS);
-
-          if (zipEntry.getName().matches(listFilePattern)) {
-            localTempFile = newFile;
-          }
-        }
-      }
-    }
-    return localTempFile;
+    tempDirWithPrefix = Files.createTempDirectory("hpanTempFolder");
+
+    return HpanUnzipper.builder()
+        .fileToUnzip(tempFile)
+        .zipThresholdEntries(1000)
+        .thresholdSizeUncompressed(20_000_000L * 64)
+        .thresholdRatio(10)
+        .outputDirectory(tempDirWithPrefix)
+        .isFilenameValidPredicate(this::isFilenameValidInZipFile)
+        .listFilePattern(listFilePattern)
+        .build()
+        .extractZipFile();
   }
 
-
   @Override
   public String getSalt() {
     return hpanRestConnector.getSalt(apiKey);
@@ -302,7 +279,8 @@ private HttpHost createProxy() {
 
   private void handleErrorStatus(int statusCode, String filename) throws IOException {
     if (statusCode == HttpStatus.SC_CONFLICT) {
-      log.error("Upload failed for file {} (status was {}: File with same name has already been uploaded)",
+      log.error(
+          "Upload failed for file {} (status was {}: File with same name has already been uploaded)",
           filename, HttpStatus.SC_CONFLICT);
     } else {
       throw new IOException("Upload failed for file " + filename + " (status was: "