Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2021-09-16 09:23:03 -0400
committerThomas Watson2021-09-20 17:03:17 -0400
commitbdc5c580de8d051932df04ed46a621d95ac06c2e (patch)
tree6c7563d54b37d0715e76e7b2511cfeefc5c22d91
parent9b837b517d9829e774b69706736d593bdef66022 (diff)
downloadrt.equinox.framework-I20210920-1800.tar.gz
rt.equinox.framework-I20210920-1800.tar.xz
rt.equinox.framework-I20210920-1800.zip
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>
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/security/BundleToJarInputStreamTest.java33
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/signedcontent/BundleToJarInputStream.java201
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);
+ }
}
}

Back to the top