Skip to content

Commit

Permalink
Internal refactoring
Browse files Browse the repository at this point in the history
- Set version of SPDX to avoid odd issue on Java 8
- Reuse BoundedInputStream
- Add Archive tests
- Add Segment tests
  • Loading branch information
garydgregory committed Feb 17, 2024
1 parent d497645 commit bf50b7d
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 40 deletions.
4 changes: 3 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand Down
Loading

0 comments on commit bf50b7d

Please sign in to comment.