Internal refactoring

- Set version of SPDX to avoid odd issue on Java 8
- Reuse BoundedInputStream
- Add Archive tests
- Add Segment tests

Author: garydgregory

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,
       *
     </commons.osgi.import>
 
@@ -78,6 +79,8 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj.
     <slf4j.version>2.0.12</slf4j.version>
     <asm.version>9.6</asm.version>
     <project.build.outputTimestamp>2024-01-01T00:00:00Z</project.build.outputTimestamp>
+    <!-- spdx 0.6.0 can require Java 11 depending on undocumented behavior which kicks in for us here. -->
+    <commons.spdx.version>0.5.5</commons.spdx.version>
   </properties>
 
   <issueManagement>
@@ -211,7 +214,6 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj.
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>
       <version>3.14.0</version>
-      <scope>test</scope>
     </dependency>    
     <dependency>
       <groupId>org.osgi</groupId>

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++) {

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 {

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.
+     * <p>
+     * The new BoundedInputStream wraps a new {@link BufferedInputStream}.
+     * </p>
+     *
+     * @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.
+     * <p>
+     * The new BoundedInputStream wraps a new {@link BufferedInputStream}.
+     * </p>
+     *
+     * @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.
+     * <p>
+     * The new BoundedInputStream wraps a new {@link BufferedInputStream}.
+     * </p>
+     *
+     * @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.
+     * <p>
+     * The new BoundedInputStream wraps a new {@link BufferedInputStream}.
+     * </p>
+     *
+     * @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> 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) {

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);

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.
          */

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());