From bf50b7d915ee406298670ceebcaa1e22616a23e0 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sat, 17 Feb 2024 17:11:22 -0500 Subject: [PATCH] Internal refactoring - Set version of SPDX to avoid odd issue on Java 8 - Reuse BoundedInputStream - Add Archive tests - Add Segment tests --- pom.xml | 4 +- .../compress/harmony/pack200/Codec.java | 2 +- .../compress/harmony/unpack200/Archive.java | 26 ++--- .../unpack200/Pack200UnpackerAdapter.java | 107 +++++++++++++++++- .../compress/harmony/unpack200/Segment.java | 6 +- .../compress/java/util/jar/Pack200.java | 5 +- .../harmony/unpack200/ArchiveTest.java | 38 +++++-- .../harmony/unpack200/SegmentTest.java | 31 +++-- 8 files changed, 179 insertions(+), 40 deletions(-) diff --git a/pom.xml b/pom.xml index 2332273f294..cc6f1b89b59 100644 --- a/pom.xml +++ b/pom.xml @@ -63,6 +63,7 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. javax.crypto.*;resolution:=optional, org.apache.commons.commons-codec;resolution:=optional, org.apache.commons.commons-io;resolution:=optional, + org.apache.commons.lang3.reflect;resolution:=optional, * @@ -78,6 +79,8 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. 2.0.12 9.6 2024-01-01T00:00:00Z + + 0.5.5 @@ -211,7 +214,6 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. org.apache.commons commons-lang3 3.14.0 - test org.osgi diff --git a/src/main/java/org/apache/commons/compress/harmony/pack200/Codec.java b/src/main/java/org/apache/commons/compress/harmony/pack200/Codec.java index a575558bab1..06630b3230d 100644 --- a/src/main/java/org/apache/commons/compress/harmony/pack200/Codec.java +++ b/src/main/java/org/apache/commons/compress/harmony/pack200/Codec.java @@ -144,7 +144,7 @@ public int[] decodeInts(final int n, final InputStream in) throws IOException, P * @throws Pack200Exception if there is a problem decoding the value or that the value is invalid */ public int[] decodeInts(final int n, final InputStream in, final int firstValue) throws IOException, Pack200Exception { - final int[] result = new int[check(n + 1, in)]; + final int[] result = new int[check(n, in) + 1]; result[0] = firstValue; int last = firstValue; for (int i = 1; i < n + 1; i++) { diff --git a/src/main/java/org/apache/commons/compress/harmony/unpack200/Archive.java b/src/main/java/org/apache/commons/compress/harmony/unpack200/Archive.java index f70c56ca932..ff39c3142ac 100644 --- a/src/main/java/org/apache/commons/compress/harmony/unpack200/Archive.java +++ b/src/main/java/org/apache/commons/compress/harmony/unpack200/Archive.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -69,18 +68,15 @@ public class Archive { * Creates an Archive with streams for the input and output files. Note: If you use this method then calling {@link #setRemovePackFile(boolean)} will have * no effect. * - * @param inputStream TODO - * @param outputStream TODO + * @param inputStream the input stream, preferably a {@link BoundedInputStream}. The bound can the the file size. + * @param outputStream the JAR output stream. + * @throws IOException if an I/O error occurs */ - public Archive(final InputStream inputStream, final JarOutputStream outputStream) { - this.inputStream = inputStream instanceof BoundedInputStream ? (BoundedInputStream) inputStream : new BoundedInputStream(inputStream); + public Archive(final InputStream inputStream, final JarOutputStream outputStream) throws IOException { + this.inputStream = Pack200UnpackerAdapter.newBoundedInputStream(inputStream); this.outputStream = outputStream; if (inputStream instanceof FileInputStream) { - try { - inputPath = Paths.get(((FileInputStream) inputStream).getFD().toString()); - } catch (final IOException e) { - throw new UncheckedIOException(e); - } + inputPath = Paths.get(Pack200UnpackerAdapter.readPath((FileInputStream) inputStream)); } else { inputPath = null; } @@ -90,18 +86,18 @@ public Archive(final InputStream inputStream, final JarOutputStream outputStream /** * Creates an Archive with the given input and output file names. * - * @param inputFileName TODO - * @param outputFileName TODO + * @param inputFileName the input file name. + * @param outputFileName the output file name * @throws FileNotFoundException if the input file does not exist - * @throws FileNotFoundException TODO - * @throws IOException TODO + * @throws IOException if an I/O error occurs */ + @SuppressWarnings("resource") public Archive(final String inputFileName, final String outputFileName) throws FileNotFoundException, IOException { this.inputPath = Paths.get(inputFileName); - this.outputFileName = outputFileName; this.inputSize = Files.size(this.inputPath); this.inputStream = new BoundedInputStream(Files.newInputStream(inputPath), inputSize); this.outputStream = new JarOutputStream(new BufferedOutputStream(new FileOutputStream(outputFileName))); + this.outputFileName = outputFileName; } private boolean available(final InputStream inputStream) throws IOException { diff --git a/src/main/java/org/apache/commons/compress/harmony/unpack200/Pack200UnpackerAdapter.java b/src/main/java/org/apache/commons/compress/harmony/unpack200/Pack200UnpackerAdapter.java index f8038ba71b8..6817f6ca51a 100644 --- a/src/main/java/org/apache/commons/compress/harmony/unpack200/Pack200UnpackerAdapter.java +++ b/src/main/java/org/apache/commons/compress/harmony/unpack200/Pack200UnpackerAdapter.java @@ -18,8 +18,12 @@ import java.io.BufferedInputStream; import java.io.File; +import java.io.FileInputStream; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.URISyntaxException; +import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -29,25 +33,124 @@ import org.apache.commons.compress.harmony.pack200.Pack200Exception; import org.apache.commons.compress.java.util.jar.Pack200.Unpacker; import org.apache.commons.io.input.BoundedInputStream; +import org.apache.commons.lang3.reflect.FieldUtils; /** * This class provides the binding between the standard Pack200 interface and the internal interface for (un)packing. */ public class Pack200UnpackerAdapter extends Pack200Adapter implements Unpacker { + /** + * Creates a new BoundedInputStream bound by the size of the given file. + *

+ * The new BoundedInputStream wraps a new {@link BufferedInputStream}. + *

+ * + * @param file The file. + * @return a new BoundedInputStream + * @throws IOException if an I/O error occurs + */ static BoundedInputStream newBoundedInputStream(final File file) throws IOException { return newBoundedInputStream(file.toPath()); } + private static BoundedInputStream newBoundedInputStream(final FileInputStream fileInputStream) throws IOException { + return newBoundedInputStream(readPath(fileInputStream)); + } + + static BoundedInputStream newBoundedInputStream(final InputStream inputStream) throws IOException { + if (inputStream instanceof BoundedInputStream) { + return (BoundedInputStream) inputStream; + } + if (inputStream instanceof FilterInputStream) { + return newBoundedInputStream(unwrap((FilterInputStream) inputStream)); + } + if (inputStream instanceof FileInputStream) { + return newBoundedInputStream((FileInputStream) inputStream); + } + // No limit + return new BoundedInputStream(inputStream); + } + + /** + * Creates a new BoundedInputStream bound by the size of the given path. + *

+ * The new BoundedInputStream wraps a new {@link BufferedInputStream}. + *

+ * + * @param path The path. + * @return a new BoundedInputStream + * @throws IOException if an I/O error occurs + */ @SuppressWarnings("resource") - static BoundedInputStream newBoundedInputStream(final Path inputPath) throws IOException { - return new BoundedInputStream(new BufferedInputStream(Files.newInputStream(inputPath)), Files.size(inputPath)); + static BoundedInputStream newBoundedInputStream(final Path path) throws IOException { + return new BoundedInputStream(new BufferedInputStream(Files.newInputStream(path)), Files.size(path)); } + /** + * Creates a new BoundedInputStream bound by the size of the given file. + *

+ * The new BoundedInputStream wraps a new {@link BufferedInputStream}. + *

+ * + * @param first the path string or initial part of the path string. + * @param more additional strings to be joined to form the path string. + * @return a new BoundedInputStream + * @throws IOException if an I/O error occurs + */ static BoundedInputStream newBoundedInputStream(final String first, final String... more) throws IOException { return newBoundedInputStream(Paths.get(first, more)); } + /** + * Creates a new BoundedInputStream bound by the size of the given URL to a file. + *

+ * The new BoundedInputStream wraps a new {@link BufferedInputStream}. + *

+ * + * @param path The URL. + * @return a new BoundedInputStream + * @throws IOException if an I/O error occurs + * @throws URISyntaxException + */ + static BoundedInputStream newBoundedInputStream(final URL url) throws IOException, URISyntaxException { + return newBoundedInputStream(Paths.get(url.toURI())); + } + + @SuppressWarnings("unchecked") + private static T readField(final Object object, final String fieldName) { + try { + return (T) FieldUtils.readField(object, fieldName, true); + } catch (final IllegalAccessException e) { + return null; + } + } + + static String readPath(final FileInputStream fis) { + return readField(fis, "path"); + } + + /** + * Unwraps the given FilterInputStream to return its wrapped InputStream. + * + * @param filterInputStream The FilterInputStream to unwrap. + * @return The wrapped InputStream + */ + static InputStream unwrap(final FilterInputStream filterInputStream) { + return readField(filterInputStream, "in"); + } + + /** + * Unwraps the given InputStream if it is an FilterInputStream to return its wrapped InputStream. + * + * @param filterInputStream The FilterInputStream to unwrap. + * @return The wrapped InputStream + */ + @SuppressWarnings("resource") + static InputStream unwrap(final InputStream inputStream) { + return inputStream instanceof FilterInputStream ? unwrap((FilterInputStream) inputStream) : inputStream; + } + @Override public void unpack(final File file, final JarOutputStream out) throws IOException { if (file == null || out == null) { diff --git a/src/main/java/org/apache/commons/compress/harmony/unpack200/Segment.java b/src/main/java/org/apache/commons/compress/harmony/unpack200/Segment.java index 576fead7513..262cebccdaf 100644 --- a/src/main/java/org/apache/commons/compress/harmony/unpack200/Segment.java +++ b/src/main/java/org/apache/commons/compress/harmony/unpack200/Segment.java @@ -49,6 +49,7 @@ import org.apache.commons.compress.harmony.unpack200.bytecode.ClassFileEntry; import org.apache.commons.compress.harmony.unpack200.bytecode.InnerClassesAttribute; import org.apache.commons.compress.harmony.unpack200.bytecode.SourceFileAttribute; +import org.apache.commons.io.input.BoundedInputStream; /** * A Pack200 archive consists of one or more segments. Each segment is stand-alone, in the sense that every segment has the magic number header; thus, every @@ -462,7 +463,7 @@ public void setPreRead(final boolean value) { /** * Unpacks a packed stream (either .pack. or .pack.gz) into a corresponding JarOuputStream. * - * @param inputStream a packed input stream. + * @param inputStream a packed input stream, preferably a {@link BoundedInputStream}. * @param out output stream. * @throws Pack200Exception if there is a problem unpacking * @throws IOException if there is a problem with I/O during unpacking @@ -484,7 +485,8 @@ void unpackProcess() throws IOException, Pack200Exception { * Package-private accessors for unpacking stages */ void unpackRead(final InputStream inputStream) throws IOException, Pack200Exception { - final InputStream in = inputStream.markSupported() ? inputStream : new BufferedInputStream(inputStream); + @SuppressWarnings("resource") + final InputStream in = Pack200UnpackerAdapter.newBoundedInputStream(inputStream); header = new SegmentHeader(this); header.read(in); diff --git a/src/main/java/org/apache/commons/compress/java/util/jar/Pack200.java b/src/main/java/org/apache/commons/compress/java/util/jar/Pack200.java index c836b25e694..1055503bc1c 100644 --- a/src/main/java/org/apache/commons/compress/java/util/jar/Pack200.java +++ b/src/main/java/org/apache/commons/compress/java/util/jar/Pack200.java @@ -30,6 +30,7 @@ import java.util.jar.JarOutputStream; import org.apache.commons.compress.harmony.archive.internal.nls.Messages; +import org.apache.commons.io.input.BoundedInputStream; /** * Class factory for {@link Pack200.Packer} and {@link Pack200.Unpacker}. @@ -230,7 +231,7 @@ public interface Unpacker { /** * Unpacks the contents of the specified {@code File} to the specified JAR output stream. * - * @param in file to be uncompressed. + * @param in file to uncompress. * @param out JAR output stream of uncompressed data. * @throws IOException if I/O exception occurs. */ @@ -239,7 +240,7 @@ public interface Unpacker { /** * Unpacks the specified stream to the specified JAR output stream. * - * @param in stream to uncompressed. + * @param in stream to uncompress, preferably a {@link BoundedInputStream}. * @param out JAR output stream of uncompressed data. * @throws IOException if I/O exception occurs. */ diff --git a/src/test/java/org/apache/commons/compress/harmony/unpack200/ArchiveTest.java b/src/test/java/org/apache/commons/compress/harmony/unpack200/ArchiveTest.java index 9b0d0cc10b0..fc8372b98c3 100644 --- a/src/test/java/org/apache/commons/compress/harmony/unpack200/ArchiveTest.java +++ b/src/test/java/org/apache/commons/compress/harmony/unpack200/ArchiveTest.java @@ -33,9 +33,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.net.URL; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Enumeration; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -196,18 +193,39 @@ public void testLoggingOptions() throws Exception { "references_oom.pack", "segment_header_oom.pack", "signatures_oom.pack" - // @formatter:on + // @formatter:on }) // Tests of various files that can cause out of memory errors - public void testParsingOOM(final String testFileName) throws Exception { + public void testParsingOOMBounded(final String testFileName) throws Exception { final URL url = Segment.class.getResource("/org/apache/commons/compress/pack/" + testFileName); - final Path path = Paths.get(url.toURI()); - try (InputStream in = new BoundedInputStream(new BufferedInputStream(Files.newInputStream(path)), Files.size(path)); + try (BoundedInputStream in = Pack200UnpackerAdapter.newBoundedInputStream(url); JarOutputStream out = new JarOutputStream(NullOutputStream.INSTANCE)) { assertThrows(IOException.class, () -> new Archive(in, out).unpack()); } } + @ParameterizedTest + @ValueSource(strings = { + // @formatter:off + "bandint_oom.pack", + "cpfloat_oom.pack", + "cputf8_oom.pack", + "favoured_oom.pack", + "filebits_oom.pack", + "flags_oom.pack", + "references_oom.pack", + "segment_header_oom.pack", + "signatures_oom.pack" + // @formatter:on + }) + // Tests of various files that can cause out of memory errors + public void testParsingOOMUnbounded(final String testFileName) throws Exception { + try (InputStream is = Segment.class.getResourceAsStream("/org/apache/commons/compress/pack/" + testFileName); + JarOutputStream out = new JarOutputStream(NullOutputStream.INSTANCE)) { + assertThrows(IOException.class, () -> new Archive(is, out).unpack()); + } + } + @Test public void testRemovePackFile() throws Exception { final File original = new File(Archive.class.getResource("/pack200/sql.pack.gz").toURI()); @@ -221,10 +239,10 @@ public void testRemovePackFile() throws Exception { read = inputStream.read(bytes); } } - final String inputFile = copy.getPath(); + final String inputFileName = copy.getPath(); final File file = createTempFile("sqlout", ".jar"); - final String outputFile = file.getPath(); - final Archive archive = new Archive(inputFile, outputFile); + final String outputFileName = file.getPath(); + final Archive archive = new Archive(inputFileName, outputFileName); archive.setRemovePackFile(true); archive.unpack(); assertFalse(copy.exists()); diff --git a/src/test/java/org/apache/commons/compress/harmony/unpack200/SegmentTest.java b/src/test/java/org/apache/commons/compress/harmony/unpack200/SegmentTest.java index 5a3b8937a94..f4ba3d67103 100644 --- a/src/test/java/org/apache/commons/compress/harmony/unpack200/SegmentTest.java +++ b/src/test/java/org/apache/commons/compress/harmony/unpack200/SegmentTest.java @@ -20,7 +20,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import java.io.BufferedInputStream; import java.io.BufferedReader; import java.io.File; import java.io.FileOutputStream; @@ -28,9 +27,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.net.URL; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.JarOutputStream; @@ -113,10 +109,31 @@ public void testJustResources() throws Exception { // @formatter:on }) // Tests of various files that can cause out of memory errors - public void testParsingOOM(final String testFileName) throws Exception { + public void testParsingOOMBounded(final String testFileName) throws Exception { final URL url = Segment.class.getResource("/org/apache/commons/compress/pack/" + testFileName); - final Path path = Paths.get(url.toURI()); - try (InputStream in = new BoundedInputStream(new BufferedInputStream(Files.newInputStream(path)), Files.size(path)); + try (BoundedInputStream in = Pack200UnpackerAdapter.newBoundedInputStream(url); + JarOutputStream out = new JarOutputStream(NullOutputStream.INSTANCE)) { + assertThrows(IOException.class, () -> new Segment().unpack(in, out)); + } + } + + @ParameterizedTest + @ValueSource(strings = { + // @formatter:off + "bandint_oom.pack", + "cpfloat_oom.pack", + "cputf8_oom.pack", + "favoured_oom.pack", + "filebits_oom.pack", + "flags_oom.pack", + "references_oom.pack", + "segment_header_oom.pack", + "signatures_oom.pack" + // @formatter:on + }) + // Tests of various files that can cause out of memory errors + public void testParsingOOMUnounded(final String testFileName) throws Exception { + try (InputStream in = Segment.class.getResourceAsStream("/org/apache/commons/compress/pack/" + testFileName); JarOutputStream out = new JarOutputStream(NullOutputStream.INSTANCE)) { assertThrows(IOException.class, () -> new Segment().unpack(in, out)); }