Bug 576000 - Avoid keeping entry content in memory while verifyingI20210921-1800I20210920-1800

content

The use of byte[] to store the complete content of an entry for a
directory must be avoided otherwise the risk of running out of memory is
high for really large resources.

Change-Id: Ib8b44b41143fc6c64967d07d0674f5ccdc19b588
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/185526
Tested-by: Equinox Bot <equinox-bot@eclipse.org>

2 files changed, 222 insertions, 12 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/security/BundleToJarInputStreamTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/security/BundleToJarInputStreamTest.java
index faddec75f..641b34428 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/security/BundleToJarInputStreamTest.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/security/BundleToJarInputStreamTest.java
@@ -34,6 +34,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.jar.JarFile;
+import java.util.jar.JarInputStream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 import java.util.zip.ZipInputStream;
@@ -51,6 +52,9 @@ public class BundleToJarInputStreamTest {
 			"signed_with_metadata", "signed_with_missing_digest", "signed_with_sf_corrupted", "signed", "signedJava16",
 			"test.bug252098", "unsigned");
 
+	static private final List<String> corruptedJarNames = Arrays.asList("signed_with_corrupt",
+			"signed_with_metadata_corrupt", "signed_with_sf_corrupted");
+
 	@Test
 	public void testInputStreamEquality() throws IOException {
 		for (String testJarName : testJarNames) {
@@ -58,6 +62,35 @@ public class BundleToJarInputStreamTest {
 		}
 	}
 
+	@Test
+	public void testSignedContentCorruption() throws Exception {
+		for (String testJarName : testJarNames) {
+			readJarContent(testJarName);
+		}
+	}
+
+	private void readJarContent(String testJarName) throws Exception {
+		File jar = getEntryFile(getTestJarPath(testJarName));
+		File extracted = extract(jar);
+		try {
+			verify(extracted);
+		} catch (Exception e) {
+			if (!corruptedJarNames.contains(testJarName)) {
+				throw e;
+			}
+		}
+	}
+
+	private void verify(File extracted) throws IOException {
+		BundleToJarInputStream inputToJar = new BundleToJarInputStream(new DirBundleFile(extracted, false));
+		try (JarInputStream jarInput = new JarInputStream(inputToJar)) {
+			for (ZipEntry extractedEntry = jarInput.getNextEntry(); extractedEntry != null; extractedEntry = jarInput
+					.getNextEntry()) {
+				getBytes(jarInput);
+			}
+		}
+	}
+
 	private void compareContent(String testJarName) throws IOException {
 		File jar = getEntryFile(getTestJarPath(testJarName));
 		File extracted = extract(jar);
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/signedcontent/BundleToJarInputStream.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/signedcontent/BundleToJarInputStream.java
index cb70bdbe5..b323eaa61 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/signedcontent/BundleToJarInputStream.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/signedcontent/BundleToJarInputStream.java
@@ -14,10 +14,13 @@
 
 package org.eclipse.osgi.internal.signedcontent;
 
+import java.io.BufferedInputStream;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.Iterator;
@@ -25,20 +28,39 @@ import java.util.List;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.jar.JarOutputStream;
+import java.util.zip.CRC32;
+import java.util.zip.Deflater;
+import java.util.zip.ZipEntry;
 import org.eclipse.osgi.storage.bundlefile.BundleEntry;
 import org.eclipse.osgi.storage.bundlefile.BundleFile;
 
-/*
- * Converts a BundleFile into an input stream that is appropriate for
- * creating a JarInputStream.
+/**
+ * Converts a BundleFile into an input stream that is appropriate for creating a
+ * JarInputStream. This class is intended to only be used for directory bundles
+ * in order to parse out the singer information. For Jar'ed bundles the JarFile
+ * class should be used to read the singer information.
+ * <p>
+ * To do this a JarOutStream is used to write entries from the bundle file one
+ * at a time and the output of that is then feed into the input stream which can
+ * be used with a JarInputStream to read the directory as if it is a JarFile.
+ * <p>
+ * Unfortunately to do this without holding the complete content of an entry in
+ * memory the content of each entry must be read 3 times. 1) for calculating the
+ * CRC. 2) for writing to the JarOutputStream so it can also calculate the CRC
+ * 3) for the actual content that will be read by the JarInputStream as it is
+ * iterating over the entries.
+ * <p>
+ * This is required to use the STORED method for each entry to avoid holding the
+ * complete content in memory.
  */
 public final class BundleToJarInputStream extends InputStream {
+	static final ByteArrayInputStream directoryInputStream = new ByteArrayInputStream(new byte[0]);
 	private final BundleFile bundlefile;
 	private final Iterator<String> entryPaths;
 	private final JarOutputStream jarOutput;
-	private final ByteArrayOutputStream nextEntryOutput = new ByteArrayOutputStream();
+	private final NextEntryOutputStream nextEntryOutput = new NextEntryOutputStream();
 
-	private ByteArrayInputStream nextEntryInput = null;
+	private InputStream nextEntryInput = null;
 
 	public BundleToJarInputStream(BundleFile bundleFile) throws IOException {
 		this.bundlefile = bundleFile;
@@ -63,6 +85,7 @@ public final class BundleToJarInputStream extends InputStream {
 		entryPaths = entries.iterator();
 
 		jarOutput = new JarOutputStream(nextEntryOutput);
+		jarOutput.setLevel(Deflater.NO_COMPRESSION);
 	}
 
 	private boolean isSignatureFile(String entry) {
@@ -84,9 +107,9 @@ public final class BundleToJarInputStream extends InputStream {
 			}
 
 			// this entry is done force a new one to be read if there is a next
+			nextEntryInput.close();
 			nextEntryInput = null;
 			return read();
-
 		}
 
 		if (entryPaths.hasNext()) {
@@ -108,26 +131,180 @@ public final class BundleToJarInputStream extends InputStream {
 		if (found == null) {
 			throw new IOException("No entry found: " + path); //$NON-NLS-1$
 		}
+		nextEntryOutput.setCurrent(found);
+
+		// create the JarEntry, this will read non-directory entries to calculate the
+		// CRC
+		JarEntry entry = getJarEntry(path, found);
+		nextEntryOutput.setCurrentLen(entry.getSize());
 
-		nextEntryOutput.reset();
-		JarEntry entry = new JarEntry(path);
 		jarOutput.putNextEntry(entry);
+		nextEntryOutput.prefixWritten();
+
 		if (!entry.isDirectory()) {
-			try (InputStream source = found.getInputStream()) {
+			// Have to read the non-directory entry again so that the JarOutputStream
+			// can confirm the CRC we calculated above is correct.
+			try (InputStream source = new BufferedInputStream(found.getInputStream())) {
 				byte[] buf = new byte[8192];
 				int length;
 				while ((length = source.read(buf)) > 0) {
 					jarOutput.write(buf, 0, length);
 				}
 			}
+		} else {
+			nextEntryOutput.setCurrentLen(0);
 		}
 
-
 		jarOutput.closeEntry();
 		jarOutput.flush();
 
-		// now save off the entry we just wrote
-		nextEntryInput = new ByteArrayInputStream(nextEntryOutput.toByteArray());
+		// now save off the entry we just wrote, this is the 3rd time
+		// we will read the content of non-directory entries!
+		nextEntryInput = nextEntryOutput.getNextInputStream();
+	}
+
+	private JarEntry getJarEntry(String path, BundleEntry found) throws IOException {
+		JarEntry entry = new JarEntry(path);
+		if (!entry.isDirectory()) {
+			entry.setMethod(ZipEntry.STORED);
+			entry.setCompressedSize(-1);
+
+			CRC32 crc = new CRC32();
+			long entryLen = 0;
+			try (InputStream source = new BufferedInputStream(found.getInputStream())) {
+				byte[] buf = new byte[8192];
+				int length;
+				while ((length = source.read(buf)) > 0) {
+					entryLen += length;
+					crc.update(buf, 0, length);
+				}
+			}
+			entry.setCrc(crc.getValue());
+			entry.setSize(entryLen);
+		}
+		return entry;
+	}
+
+	@Override
+	public void close() throws IOException {
+		nextEntryOutput.close();
+	}
+
+	static class NextEntryOutputStream extends OutputStream {
+		private BundleEntry current = null;
+		private long currentLen = -1;
+		private long currentWritten = 0;
+		private boolean writingPrefix = true;
+		final ByteArrayOutputStream currentPrefix = new ByteArrayOutputStream();
+		final ByteArrayOutputStream currentPostfix = new ByteArrayOutputStream();
+
+		void setCurrent(BundleEntry entry) {
+			this.current = entry;
+			this.currentWritten = 0;
+			this.currentPrefix.reset();
+			this.currentPostfix.reset();
+			this.writingPrefix = true;
+		}
+
+		void prefixWritten() {
+			writingPrefix = false;
+		}
+
+		void setCurrentLen(long currentLen) {
+			this.currentLen = currentLen;
+		}
+
+		InputStream getCurrentInputStream() throws IOException {
+			if (current.getName().endsWith("/")) { //$NON-NLS-1$
+				return directoryInputStream;
+			}
+			return new BufferedInputStream(current.getInputStream());
+		}
+
+		InputStream getNextInputStream() throws IOException {
+			return new FilterInputStream(getCurrentInputStream()) {
+				ByteArrayInputStream prefix = new ByteArrayInputStream(currentPrefix.toByteArray());
+				ByteArrayInputStream postfix = new ByteArrayInputStream(currentPostfix.toByteArray());
+
+				@Override
+				public int read() throws IOException {
+					int read = prefix.read();
+					if (read != -1) {
+						return read;
+					}
+					read = super.read();
+					if (read != -1) {
+						return read;
+					}
+					return postfix.read();
+				}
+
+				@Override
+				public int read(byte[] b) throws IOException {
+					int read = prefix.read(b);
+					if (read != -1) {
+						return read;
+					}
+					read = super.read(b);
+					if (read != -1) {
+						return read;
+					}
+					return postfix.read(b);
+				}
+
+				@Override
+				public int read(byte[] b, int off, int len) throws IOException {
+					int read = prefix.read(b, off, len);
+					if (read != -1) {
+						return read;
+					}
+					read = super.read(b, off, len);
+					if (read != -1) {
+						return read;
+					}
+					return postfix.read(b, off, len);
+				}
+			};
+		}
+
+		@Override
+		public void write(int b) throws IOException {
+			if (writingPrefix) {
+				currentPrefix.write(b);
+				return;
+			}
+			if (currentWritten < currentLen) {
+				currentWritten++;
+				return;
+			}
+			currentPostfix.write(b);
+		}
+
+		@Override
+		public void write(byte[] b) throws IOException {
+			if (writingPrefix) {
+				currentPrefix.write(b);
+				return;
+			}
+			if (currentWritten < currentLen) {
+				currentWritten += b.length;
+				return;
+			}
+			currentPostfix.write(b);
+		}
+
+		@Override
+		public void write(byte[] b, int off, int len) throws IOException {
+			if (writingPrefix) {
+				currentPrefix.write(b, off, len);
+				return;
+			}
+			if (currentWritten < currentLen) {
+				currentWritten += len;
+				return;
+			}
+			currentPostfix.write(b, off, len);
+		}
 	}
 
 }