This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit bf50b7d915ee406298670ceebcaa1e22616a23e0
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Sat Feb 17 17:11:22 2024 -0500
Internal refactoring
- Set version of SPDX to avoid odd issue on Java 8
- Reuse BoundedInputStream
- Add Archive tests
- Add Segment tests
---
pom.xml | 4 +-
.../commons/compress/harmony/pack200/Codec.java | 2 +-
.../compress/harmony/unpack200/Archive.java | 26 +++--
.../harmony/unpack200/Pack200UnpackerAdapter.java | 107 ++++++++++++++++++++-
.../compress/harmony/unpack200/Segment.java | 6 +-
.../commons/compress/java/util/jar/Pack200.java | 5 +-
.../compress/harmony/unpack200/ArchiveTest.java | 38 ++++++--
.../compress/harmony/unpack200/SegmentTest.java | 31 ++++--
8 files changed, 179 insertions(+), 40 deletions(-)
diff --git a/pom.xml b/pom.xml
index 2332273f2..cc6f1b89b 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,
*
</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>
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 a575558ba..06630b323 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 abstract class Codec {
* @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 f70c56ca9..ff39c3142 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.FileOutputStream;
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 class Archive {
/**
* 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 f8038ba71..6817f6ca5 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 @@ package org.apache.commons.compress.harmony.unpack200;
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.Pack200Adapter;
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) {
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 576fead75..262cebccd 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.ClassFile;
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 class Segment {
/**
* 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 @@ public class Segment {
* 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 c836b25e6..1055503bc 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.JarInputStream;
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 abstract class Pack200 {
/**
* 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 abstract class Pack200 {
/**
* 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 9b0d0cc10..fc8372b98 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.IOException;
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 class ArchiveTest extends AbstractTempDirTest {
"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 class ArchiveTest extends AbstractTempDirTest {
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 5a3b8937a..f4ba3d671 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.Assert.assertThrows;
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.IOException;
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 class SegmentTest extends AbstractTempDirTest {
// @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));
}
