From f351f3fdce11369d1a957156d877875c065bb3dd Mon Sep 17 00:00:00 2001 From: Velinov Date: Mon, 3 Jul 2023 16:50:45 +0300 Subject: [PATCH] Improve MTA file size validation Use commons-fileupload LimitedInputStream instead of our own wrapper of InputStream Jira: LMCROSSITXSADEPLOY-2766 --- multiapps-mta/pom.xml | 4 ++ multiapps-mta/src/main/java/module-info.java | 1 + .../mta/handlers/ArchiveHandler.java | 21 +++++--- .../mta/handlers/LimitedSizeInputStream.java | 49 ------------------- .../mta/handlers/ArchiveHandlerTest.java | 18 +------ pom.xml | 7 +++ 6 files changed, 27 insertions(+), 73 deletions(-) delete mode 100644 multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/LimitedSizeInputStream.java diff --git a/multiapps-mta/pom.xml b/multiapps-mta/pom.xml index b8000429..84b52214 100644 --- a/multiapps-mta/pom.xml +++ b/multiapps-mta/pom.xml @@ -20,6 +20,10 @@ org.apache.commons commons-collections4 + + commons-fileupload + commons-fileupload + com.vdurmont semver4j diff --git a/multiapps-mta/src/main/java/module-info.java b/multiapps-mta/src/main/java/module-info.java index 3ab88054..6394a884 100644 --- 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; } \ No newline at end of file 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 index 7df696bc..b688eaac 100644 --- 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/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/LimitedSizeInputStream.java b/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/LimitedSizeInputStream.java deleted file mode 100644 index 37cb966d..00000000 --- a/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/handlers/LimitedSizeInputStream.java +++ /dev/null @@ -1,49 +0,0 @@ -package org.cloudfoundry.multiapps.mta.handlers; - -import java.io.IOException; -import java.io.InputStream; - -import org.cloudfoundry.multiapps.common.ContentException; -import org.cloudfoundry.multiapps.mta.Messages; - -public class LimitedSizeInputStream extends InputStream { - - private final InputStream inputStream; - private final long maxSize; - private final String entryName; - private long total; - - public LimitedSizeInputStream(InputStream inputStream, long maxSize, String entryName) { - this.inputStream = inputStream; - this.maxSize = maxSize; - this.entryName = entryName; - } - - @Override - public int read() throws IOException { - int i = inputStream.read(); - if (i >= 0) - incrementCounter(1); - return i; - } - - @Override - public int read(byte b[]) throws IOException { - return read(b, 0, b.length); - } - - @Override - public int read(byte b[], int off, int len) throws IOException { - int i = inputStream.read(b, off, len); - if (i >= 0) - incrementCounter(i); - return i; - } - - private void incrementCounter(int size) throws IOException { - total += size; - if (total > maxSize) - throw new ContentException(Messages.ERROR_PROCESSED_SIZE_OF_FILE_EXCEEDS_CONFIGURED_MAX_SIZE_LIMIT, total, entryName, maxSize); - } - -} 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 index 6ce010d3..2eb257a8 100644 --- 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 index 94bd2e8b..17437b63 100644 --- a/pom.xml +++ b/pom.xml @@ -20,6 +20,7 @@ 3.12.0 4.4 2.11.0 + 1.5 2.0.6 2.0 3.1.0 @@ -303,6 +304,12 @@ commons-io ${commons-io.version} + + + commons-fileupload + commons-fileupload + ${commons-fileupload.version} + org.yaml