You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2019/08/09 15:18:50 UTC
[commons-compress] 01/02: COMPRESS-486 deal with resouce leaks in
example code
This is an automated email from the ASF dual-hosted git repository.
bodewig pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit 91c2f0df50e86c65131cf9b83f1cba2e10c9786f
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Sun May 12 14:29:29 2019 +0200
COMPRESS-486 deal with resouce leaks in example code
---
src/changes/changes.xml | 7 ++
.../compress/archivers/examples/Archiver.java | 82 +++++++++++++--
.../archivers/examples/CloseableConsumer.java | 56 ++++++++++
.../examples/CloseableConsumerAdapter.java | 46 +++++++++
.../compress/archivers/examples/Expander.java | 113 +++++++++++++++++++--
5 files changed, 291 insertions(+), 13 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index b41a411..aacc9a1 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -96,6 +96,13 @@ The <action> type attribute can be add,update,fix,remove.
Throw IOException rather than RuntimeExceptions for certain
malformed LZ4 or Snappy inputs.
</action>
+ <action type="update" date="2019-08-09" issue="COMPRESS-486">
+ The Expander and Archive example classes can leak resources
+ they have wrapped around passed in streams or channels. The
+ methods consuming streams and channels have been adapted to
+ give the calling code a chance to deal with those wrapper
+ resources.
+ </action>
</release>
<release version="1.18" date="2018-08-16"
description="Release 1.18">
diff --git a/src/main/java/org/apache/commons/compress/archivers/examples/Archiver.java b/src/main/java/org/apache/commons/compress/archivers/examples/Archiver.java
index b02c79c..8623862 100644
--- a/src/main/java/org/apache/commons/compress/archivers/examples/Archiver.java
+++ b/src/main/java/org/apache/commons/compress/archivers/examples/Archiver.java
@@ -71,12 +71,12 @@ public class Archiver {
if (prefersSeekableByteChannel(format)) {
try (SeekableByteChannel c = FileChannel.open(target.toPath(), StandardOpenOption.WRITE,
StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {
- create(format, c, directory);
+ create(format, c, directory, CloseableConsumer.CLOSING_CONSUMER);
}
return;
}
try (OutputStream o = Files.newOutputStream(target.toPath())) {
- create(format, o, directory);
+ create(format, o, directory, CloseableConsumer.CLOSING_CONSUMER);
}
}
@@ -85,15 +85,50 @@ public class Archiver {
* format} by recursively including all files and directories in
* {@code directory}.
*
+ * <p>This method creates a wrapper around the target stream
+ * which is never closed and thus leaks resources, please use
+ * {@link #create(String,OutputStream,File,CloseableConsumer)}
+ * instead.</p>
+ *
* @param format the archive format. This uses the same format as
* accepted by {@link ArchiveStreamFactory}.
* @param target the stream to write the new archive to.
* @param directory the directory that contains the files to archive.
* @throws IOException if an I/O error occurs
* @throws ArchiveException if the archive cannot be created for other reasons
+ * @deprecated this method leaks resources
*/
+ @Deprecated
public void create(String format, OutputStream target, File directory) throws IOException, ArchiveException {
- create(new ArchiveStreamFactory().createArchiveOutputStream(format, target), directory);
+ create(format, target, directory, CloseableConsumer.NULL_CONSUMER);
+ }
+
+ /**
+ * Creates an archive {@code target} using the format {@code
+ * format} by recursively including all files and directories in
+ * {@code directory}.
+ *
+ * <p>This method creates a wrapper around the archive stream and
+ * the caller of this method is responsible for closing it -
+ * probably at the same time as closing the stream itself. The
+ * caller is informed about the wrapper object via the {@code
+ * closeableConsumer} callback</p>
+ *
+ * @param format the archive format. This uses the same format as
+ * accepted by {@link ArchiveStreamFactory}.
+ * @param target the stream to write the new archive to.
+ * @param directory the directory that contains the files to archive.
+ * @param closeableConsumer is informed about the stream wrapped around the passed in stream
+ * @throws IOException if an I/O error occurs
+ * @throws ArchiveException if the archive cannot be created for other reasons
+ * @since 1.19
+ */
+ public void create(String format, OutputStream target, File directory,
+ CloseableConsumer closeableConsumer) throws IOException, ArchiveException {
+ try (CloseableConsumerAdapter c = new CloseableConsumerAdapter(closeableConsumer)) {
+ create(c.track(new ArchiveStreamFactory().createArchiveOutputStream(format, target)),
+ directory);
+ }
}
/**
@@ -101,25 +136,60 @@ public class Archiver {
* format} by recursively including all files and directories in
* {@code directory}.
*
+ * <p>This method creates a wrapper around the target channel
+ * which is never closed and thus leaks resources, please use
+ * {@link #create(String,SeekableByteChannel,File,CloseableConsumer)}
+ * instead.</p>
+ *
* @param format the archive format. This uses the same format as
* accepted by {@link ArchiveStreamFactory}.
* @param target the channel to write the new archive to.
* @param directory the directory that contains the files to archive.
* @throws IOException if an I/O error occurs
* @throws ArchiveException if the archive cannot be created for other reasons
+ * @deprecated this method leaks resources
*/
+ @Deprecated
public void create(String format, SeekableByteChannel target, File directory)
throws IOException, ArchiveException {
+ create(format, target, directory, CloseableConsumer.NULL_CONSUMER);
+ }
+
+ /**
+ * Creates an archive {@code target} using the format {@code
+ * format} by recursively including all files and directories in
+ * {@code directory}.
+ *
+ * <p>This method creates a wrapper around the archive channel and
+ * the caller of this method is responsible for closing it -
+ * probably at the same time as closing the channel itself. The
+ * caller is informed about the wrapper object via the {@code
+ * closeableConsumer} callback</p>
+ *
+ * @param format the archive format. This uses the same format as
+ * accepted by {@link ArchiveStreamFactory}.
+ * @param target the channel to write the new archive to.
+ * @param directory the directory that contains the files to archive.
+ * @param closeableConsumer is informed about the stream wrapped around the passed in stream
+ * @throws IOException if an I/O error occurs
+ * @throws ArchiveException if the archive cannot be created for other reasons
+ * @since 1.19
+ */
+ public void create(String format, SeekableByteChannel target, File directory,
+ CloseableConsumer closeableConsumer)
+ throws IOException, ArchiveException {
+ try (CloseableConsumerAdapter c = new CloseableConsumerAdapter(closeableConsumer)) {
if (!prefersSeekableByteChannel(format)) {
- create(format, Channels.newOutputStream(target), directory);
+ create(format, c.track(Channels.newOutputStream(target)), directory);
} else if (ArchiveStreamFactory.ZIP.equalsIgnoreCase(format)) {
- create(new ZipArchiveOutputStream(target), directory);
+ create(c.track(new ZipArchiveOutputStream(target)), directory);
} else if (ArchiveStreamFactory.SEVEN_Z.equalsIgnoreCase(format)) {
- create(new SevenZOutputFile(target), directory);
+ create(c.track(new SevenZOutputFile(target)), directory);
} else {
// never reached as prefersSeekableByteChannel only returns true for ZIP and 7z
throw new ArchiveException("Don't know how to handle format " + format);
}
+ }
}
/**
diff --git a/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumer.java b/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumer.java
new file mode 100644
index 0000000..1c84bbf
--- /dev/null
+++ b/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumer.java
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.commons.compress.archivers.examples;
+
+import java.io.Closeable;
+import java.io.IOException;
+
+/**
+ * Is informed about a closable resource that has been wrapped around
+ * a passed in stream or channel by Expander or Archiver.
+ *
+ * <p>This provides a way to close said resources in the calling code.</p>
+ *
+ * @since 1.19
+ */
+public interface CloseableConsumer {
+ /**
+ * Closes the passed in Closeable immediately.
+ */
+ public static CloseableConsumer CLOSING_CONSUMER = new CloseableConsumer() {
+ @Override
+ public void accept(Closeable c) throws IOException {
+ c.close();
+ }
+ };
+
+ /**
+ * Completely ignores the passed in Closeable.
+ */
+ public static CloseableConsumer NULL_CONSUMER = new CloseableConsumer() {
+ @Override
+ public void accept(Closeable c) { }
+ };
+
+ /**
+ * Is informed about a closable resource that has been wrapped around
+ * a passed in stream or channel by Expander or Archiver.
+ */
+ void accept(Closeable c) throws IOException;
+}
diff --git a/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumerAdapter.java b/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumerAdapter.java
new file mode 100644
index 0000000..2f2f7ab
--- /dev/null
+++ b/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumerAdapter.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.commons.compress.archivers.examples;
+
+import java.io.Closeable;
+import java.io.IOException;
+
+final class CloseableConsumerAdapter implements Closeable {
+ private final CloseableConsumer consumer;
+ private Closeable closeable;
+
+ CloseableConsumerAdapter(CloseableConsumer consumer) {
+ if (consumer == null) {
+ throw new NullPointerException("consumer must not be null");
+ }
+ this.consumer = consumer;
+ }
+
+ <C extends Closeable> C track(C closeable) {
+ this.closeable = closeable;
+ return closeable;
+ }
+
+ @Override
+ public void close() throws IOException {
+ if (closeable != null) {
+ consumer.accept(closeable);
+ }
+ }
+}
diff --git a/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java b/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java
index e2a3c1c..44998d9 100644
--- a/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java
+++ b/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java
@@ -84,12 +84,12 @@ public class Expander {
public void expand(String format, File archive, File targetDirectory) throws IOException, ArchiveException {
if (prefersSeekableByteChannel(format)) {
try (SeekableByteChannel c = FileChannel.open(archive.toPath(), StandardOpenOption.READ)) {
- expand(format, c, targetDirectory);
+ expand(format, c, targetDirectory, CloseableConsumer.CLOSING_CONSUMER);
}
return;
}
try (InputStream i = new BufferedInputStream(Files.newInputStream(archive.toPath()))) {
- expand(format, i, targetDirectory);
+ expand(format, i, targetDirectory, CloseableConsumer.CLOSING_CONSUMER);
}
}
@@ -98,52 +98,151 @@ public class Expander {
*
* <p>Tries to auto-detect the archive's format.</p>
*
+ * <p>This method creates a wrapper around the archive stream
+ * which is never closed and thus leaks resources, please use
+ * {@link #expand(InputStream,File,CloseableConsumer)}
+ * instead.</p>
+ *
* @param archive the file to expand
* @param targetDirectory the directory to write to
* @throws IOException if an I/O error occurs
* @throws ArchiveException if the archive cannot be read for other reasons
+ * @deprecated this method leaks resources
*/
+ @Deprecated
public void expand(InputStream archive, File targetDirectory) throws IOException, ArchiveException {
- expand(new ArchiveStreamFactory().createArchiveInputStream(archive), targetDirectory);
+ expand(archive, targetDirectory, CloseableConsumer.NULL_CONSUMER);
+ }
+
+ /**
+ * Expands {@code archive} into {@code targetDirectory}.
+ *
+ * <p>Tries to auto-detect the archive's format.</p>
+ *
+ * <p>This method creates a wrapper around the archive stream and
+ * the caller of this method is responsible for closing it -
+ * probably at the same time as closing the stream itself. The
+ * caller is informed about the wrapper object via the {@code
+ * closeableConsumer} callback</p>
+ *
+ * @param archive the file to expand
+ * @param targetDirectory the directory to write to
+ * @param closeableConsumer is informed about the stream wrapped around the passed in stream
+ * @throws IOException if an I/O error occurs
+ * @throws ArchiveException if the archive cannot be read for other reasons
+ * @since 1.19
+ */
+ public void expand(InputStream archive, File targetDirectory, CloseableConsumer closeableConsumer)
+ throws IOException, ArchiveException {
+ try (CloseableConsumerAdapter c = new CloseableConsumerAdapter(closeableConsumer)) {
+ expand(c.track(new ArchiveStreamFactory().createArchiveInputStream(archive)),
+ targetDirectory);
+ }
}
/**
* Expands {@code archive} into {@code targetDirectory}.
*
+ * <p>This method creates a wrapper around the archive stream
+ * which is never closed and thus leaks resources, please use
+ * {@link #expand(String,InputStream,File,CloseableConsumer)}
+ * instead.</p>
+ *
* @param archive the file to expand
* @param targetDirectory the directory to write to
* @param format the archive format. This uses the same format as
* accepted by {@link ArchiveStreamFactory}.
* @throws IOException if an I/O error occurs
* @throws ArchiveException if the archive cannot be read for other reasons
+ * @deprecated this method leaks resources
*/
+ @Deprecated
public void expand(String format, InputStream archive, File targetDirectory)
throws IOException, ArchiveException {
- expand(new ArchiveStreamFactory().createArchiveInputStream(format, archive), targetDirectory);
+ expand(format, archive, targetDirectory, CloseableConsumer.NULL_CONSUMER);
+ }
+
+ /**
+ * Expands {@code archive} into {@code targetDirectory}.
+ *
+ * <p>This method creates a wrapper around the archive stream and
+ * the caller of this method is responsible for closing it -
+ * probably at the same time as closing the stream itself. The
+ * caller is informed about the wrapper object via the {@code
+ * closeableConsumer} callback</p>
+ *
+ * @param archive the file to expand
+ * @param targetDirectory the directory to write to
+ * @param format the archive format. This uses the same format as
+ * accepted by {@link ArchiveStreamFactory}.
+ * @param closeableConsumer is informed about the stream wrapped around the passed in stream
+ * @throws IOException if an I/O error occurs
+ * @throws ArchiveException if the archive cannot be read for other reasons
+ * @since 1.19
+ */
+ public void expand(String format, InputStream archive, File targetDirectory, CloseableConsumer closeableConsumer)
+ throws IOException, ArchiveException {
+ try (CloseableConsumerAdapter c = new CloseableConsumerAdapter(closeableConsumer)) {
+ expand(c.track(new ArchiveStreamFactory().createArchiveInputStream(format, archive)),
+ targetDirectory);
+ }
}
/**
* Expands {@code archive} into {@code targetDirectory}.
*
+ * <p>This method creates a wrapper around the archive channel
+ * which is never closed and thus leaks resources, please use
+ * {@link #expand(String,SeekableByteChannel,File,CloseableConsumer)}
+ * instead.</p>
+ *
* @param archive the file to expand
* @param targetDirectory the directory to write to
* @param format the archive format. This uses the same format as
* accepted by {@link ArchiveStreamFactory}.
* @throws IOException if an I/O error occurs
* @throws ArchiveException if the archive cannot be read for other reasons
+ * @deprecated this method leaks resources
*/
+ @Deprecated
public void expand(String format, SeekableByteChannel archive, File targetDirectory)
throws IOException, ArchiveException {
+ expand(format, archive, targetDirectory, CloseableConsumer.NULL_CONSUMER);
+ }
+
+ /**
+ * Expands {@code archive} into {@code targetDirectory}.
+ *
+ * <p>This method creates a wrapper around the archive channel and
+ * the caller of this method is responsible for closing it -
+ * probably at the same time as closing the channel itself. The
+ * caller is informed about the wrapper object via the {@code
+ * closeableConsumer} callback</p>
+ *
+ * @param archive the file to expand
+ * @param targetDirectory the directory to write to
+ * @param format the archive format. This uses the same format as
+ * accepted by {@link ArchiveStreamFactory}.
+ * @param closeableConsumer is informed about the stream wrapped around the passed in channel
+ * @throws IOException if an I/O error occurs
+ * @throws ArchiveException if the archive cannot be read for other reasons
+ * @since 1.19
+ */
+ public void expand(String format, SeekableByteChannel archive, File targetDirectory,
+ CloseableConsumer closeableConsumer)
+ throws IOException, ArchiveException {
+ try (CloseableConsumerAdapter c = new CloseableConsumerAdapter(closeableConsumer)) {
if (!prefersSeekableByteChannel(format)) {
- expand(format, Channels.newInputStream(archive), targetDirectory);
+ expand(format, c.track(Channels.newInputStream(archive)), targetDirectory);
} else if (ArchiveStreamFactory.ZIP.equalsIgnoreCase(format)) {
- expand(new ZipFile(archive), targetDirectory);
+ expand(c.track(new ZipFile(archive)), targetDirectory);
} else if (ArchiveStreamFactory.SEVEN_Z.equalsIgnoreCase(format)) {
- expand(new SevenZFile(archive), targetDirectory);
+ expand(c.track(new SevenZFile(archive)), targetDirectory);
} else {
// never reached as prefersSeekableByteChannel only returns true for ZIP and 7z
throw new ArchiveException("Don't know how to handle format " + format);
}
+ }
}
/**