diff options
author | Thomas Watson | 2021-09-16 13:23:03 +0000 |
---|---|---|
committer | Thomas Watson | 2021-09-20 21:03:17 +0000 |
commit | bdc5c580de8d051932df04ed46a621d95ac06c2e (patch) | |
tree | 6c7563d54b37d0715e76e7b2511cfeefc5c22d91 | |
parent | 9b837b517d9829e774b69706736d593bdef66022 (diff) | |
download | rt.equinox.framework-bdc5c580de8d051932df04ed46a621d95ac06c2e.tar.gz rt.equinox.framework-bdc5c580de8d051932df04ed46a621d95ac06c2e.tar.xz rt.equinox.framework-bdc5c580de8d051932df04ed46a621d95ac06c2e.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>
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); + } } } |