Improve MTA file size validation

Use commons-fileupload LimitedInputStream instead of our own wrapper of InputStream Jira: LMCROSSITXSADEPLOY-2766
cloudfoundry · Jul 3, 2023 · f351f3f · f351f3f
1 parent 139b025
commit f351f3f
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 73 deletions.
diff --git a/multiapps-mta/pom.xml b/multiapps-mta/pom.xml
@@ -20,6 +20,10 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-collections4</artifactId>
         </dependency>
+        <dependency>
+            <groupId>commons-fileupload</groupId>
+            <artifactId>commons-fileupload</artifactId>
+        </dependency>
         <dependency>
             <groupId>com.vdurmont</groupId>
             <artifactId>semver4j</artifactId>

diff --git a/multiapps-mta/src/main/java/module-info.java b/multiapps-mta/src/main/java/module-info.java
@@ -33,5 +33,6 @@
 
     requires static java.compiler;
     requires static org.immutables.value;
+    requires commons.fileupload;
 
 }
diff --git a/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/ArchiveHandler.java b/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/ArchiveHandler.java
@@ -8,8 +8,8 @@
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
+import org.apache.commons.fileupload.util.LimitedInputStream;
 import org.apache.commons.io.IOUtils;
-import org.apache.commons.io.output.StringBuilderWriter;
 import org.cloudfoundry.multiapps.common.ContentException;
 import org.cloudfoundry.multiapps.common.SLException;
 import org.cloudfoundry.multiapps.mta.Messages;
@@ -27,11 +27,8 @@ public static Manifest getManifest(InputStream archiveStream, long maxManifestSi
     }
 
     public static String getDescriptor(InputStream archiveStream, long maxMtaDescriptorSize) throws SLException {
-        try (InputStream descriptorStream = getInputStream(archiveStream, MTA_DEPLOYMENT_DESCRIPTOR_NAME, maxMtaDescriptorSize);
-            InputStream in = new LimitedSizeInputStream(descriptorStream, maxMtaDescriptorSize, MTA_DEPLOYMENT_DESCRIPTOR_NAME);
-            final StringBuilderWriter sw = new StringBuilderWriter()) {
-            IOUtils.copy(in, sw, StandardCharsets.UTF_8);
-            return sw.toString();
+        try (InputStream descriptorStream = getInputStream(archiveStream, MTA_DEPLOYMENT_DESCRIPTOR_NAME, maxMtaDescriptorSize)) {
+            return IOUtils.toString(descriptorStream, StandardCharsets.UTF_8);
         } catch (IOException e) {
             throw new SLException(e, Messages.ERROR_RETRIEVING_MTA_MODULE_CONTENT);
         }
@@ -51,8 +48,18 @@ public static InputStream getInputStream(InputStream is, String entryName, long
             for (ZipEntry e; (e = zis.getNextEntry()) != null;) {
                 if (e.getName()
                      .equals(entryName)) {
+                    // quick and unreliable check for file size without file processing
                     validateZipEntrySize(e, maxEntrySize);
-                    return zis;
+                    // ensure processed file size indeed is lower than the configured limit
+                    return new LimitedInputStream(zis, maxEntrySize) {
+                        @Override
+                        protected void raiseError(long maxSize, long currentSize) {
+                            throw new ContentException(Messages.ERROR_PROCESSED_SIZE_OF_FILE_EXCEEDS_CONFIGURED_MAX_SIZE_LIMIT,
+                                                       currentSize,
+                                                       entryName,
+                                                       maxSize);
+                        }
+                    };
                 }
             }
             throw new ContentException(Messages.CANNOT_FIND_ARCHIVE_ENTRY, entryName);

diff --git a/...pps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/LimitedSizeInputStream.java b/...pps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/LimitedSizeInputStream.java
diff --git a/multiapps-mta/src/test/java/org/cloudfoundry/multiapps/mta/handlers/ArchiveHandlerTest.java b/multiapps-mta/src/test/java/org/cloudfoundry/multiapps/mta/handlers/ArchiveHandlerTest.java
@@ -25,7 +25,6 @@ class ArchiveHandlerTest {
     private static final String LARGE_DESCRIPTOR_MTAR = "large.mta.descriptor.mtar";
     private static final long MAX_MTA_DESCRIPTOR_SIZE = 1024 * 1024L;
     private static final long MAX_MANIFEST_SIZE = 1024 * 1024L;
-    private static final long MAX_RESOURCE_FILE_SIZE = 1024 * 1024 * 1024L;
 
     @Test
     void testGetManifest() throws Exception {
@@ -60,7 +59,7 @@ void testGetDescriptorExceedsSize() throws Exception {
     @Test
     void testGetProcessedDescriptorExceedsSize() throws Exception {
         assertThrows(ContentException.class,
-                     () -> ArchiveHandler.getDescriptor(ArchiveHandlerTest.class.getResourceAsStream(LARGE_DESCRIPTOR_MTAR), 10 * 1024L));
+                     () -> ArchiveHandler.getDescriptor(ArchiveHandlerTest.class.getResourceAsStream(LARGE_DESCRIPTOR_MTAR), 1024L));
     }
 
     @Test
@@ -97,21 +96,6 @@ void testGetDescriptorFlat() throws Exception {
         assertTrue(descriptor.contains("com.sap.mta.sample"));
     }
 
-    @Test
-    void testGetModuleContentFlat() throws Exception {
-        InputStream mtarStream = ArchiveHandlerTest.class.getResourceAsStream(SAMPLE_FLAT_MTAR);
-        try (InputStream is = ArchiveHandler.getInputStream(mtarStream, "web/", MAX_RESOURCE_FILE_SIZE)) {
-            ZipInputStream zis = (ZipInputStream) is;
-            for (ZipEntry e; (e = zis.getNextEntry()) != null;) {
-                if (e.getName()
-                     .equals("web/readme.txt")) {
-                    String readmeContent = IOUtils.toString(zis, StandardCharsets.UTF_8);
-                    assertEquals("App router code will be packaged in this archive", readmeContent);
-                }
-            }
-        }
-    }
-
     private InputStream getEntryStream(byte[] content, String entryName) throws IOException {
         ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(content));
         for (ZipEntry e; (e = zis.getNextEntry()) != null;) {

diff --git a/pom.xml b/pom.xml
@@ -20,6 +20,7 @@
         <commons-lang3.version>3.12.0</commons-lang3.version>
         <commons-collections4.version>4.4</commons-collections4.version>
         <commons-io.version>2.11.0</commons-io.version>
+        <commons-fileupload.version>1.5</commons-fileupload.version>
         <slf4j.version>2.0.6</slf4j.version>
         <snakeyaml.version>2.0</snakeyaml.version>
         <semver4j.version>3.1.0</semver4j.version>
@@ -303,6 +304,12 @@
                 <artifactId>commons-io</artifactId>
                 <version>${commons-io.version}</version>
             </dependency>
+            <!-- https://mvnrepository.com/artifact/commons-fileupload/commons-fileupload -->
+            <dependency>
+                <groupId>commons-fileupload</groupId>
+                <artifactId>commons-fileupload</artifactId>
+                <version>${commons-fileupload.version}</version>
+            </dependency>
             <!-- https://mvnrepository.com/artifact/org.yaml/snakeyaml -->
             <dependency>
                 <groupId>org.yaml</groupId>