Add AbstractArchiveBuilder for unified archiver support

src/changes/changes.xml

@@ -125,6 +125,7 @@ The <action> type attribute can be add,update,fix,remove.
       <action type="add" dev="ggregory" due-to="Gary Gregory, Piotr P. Karwasz">Introduce builders for all ArchiveInputStream implementations and deprecate some constructors.</action>
       <action type="add" dev="ggregory" due-to="Gary Gregory">TarFile now implements IOIterable&lt;TarArchiveEntry&gt;.</action>
       <action type="add" dev="ggregory" due-to="Gary Gregory">Add a builder for the TarFile class and deprecate some constructors.</action>
+      <action type="add" dev="pkarwasz" due-to="Piotr Karwasz">SevenZFile, TarFile, and ZipFile now always close underlying resources when builder or constructor fails.</action>
       <!-- UPDATE -->
       <action type="update" dev="ggregory" due-to="Gary Gregory">Bump org.apache.commons:commons-parent from 85 to 88 #707.</action>
       <action type="update" dev="ggregory" due-to="Gary Gregory">Bump org.apache.commons:commons-lang3 from 3.18.0 to 3.19.0.</action>

src/main/java/org/apache/commons/compress/archivers/AbstractArchiveBuilder.java

@@ -0,0 +1,42 @@
+/*
+ * 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
+ *
+ *   https://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;
+
+import org.apache.commons.io.build.AbstractStreamBuilder;
+
+/**
+ * Base class for builder pattern implementations of all archive readers.
+ *
+ * <p>Ensures that all {@code ArchiveInputStream} implementations and other
+ * archive handlers expose a consistent set of configuration options.</p>
+ *
+ * @param <T> The type of archive stream or file to build.
+ * @param <B> The type of the concrete builder subclass.
+ * @since 1.29.0
+ */
+public abstract class AbstractArchiveBuilder<T, B extends AbstractArchiveBuilder<T, B>>
+        extends AbstractStreamBuilder<T, B> {
+
+    /**
+     * Constructs a new instance.
+     */
+    protected AbstractArchiveBuilder() {
+        // empty
+    }
+}

src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java

@@ -26,7 +26,6 @@
 import java.util.Objects;
 
 import org.apache.commons.io.Charsets;
-import org.apache.commons.io.build.AbstractStreamBuilder;
 import org.apache.commons.io.function.IOConsumer;
 import org.apache.commons.io.function.IOIterator;
 import org.apache.commons.io.input.NullInputStream;
@@ -52,24 +51,6 @@
  */
 public abstract class ArchiveInputStream<E extends ArchiveEntry> extends FilterInputStream {
 
-    /**
-     * Generic builder for ArchiveInputStream instances.
-     *
-     * @param <T> The type of {@link ArchiveInputStream} to build.
-     * @param <B> The type of the concrete AbstractBuilder.
-     * @since 1.29.0
-     */
-    public abstract static class AbstractBuilder<T extends ArchiveInputStream<?>, B extends AbstractBuilder<T, B>>
-            extends AbstractStreamBuilder<T, B> {
-
-        /**
-         * Constructs a new instance.
-         */
-        protected AbstractBuilder() {
-            // empty
-        }
-    }
-
     /**
      * An iterator over a collection of a specific {@link ArchiveEntry} type.
      */
@@ -121,30 +102,45 @@ public Iterator<E> unwrap() {
      */
     @SuppressWarnings("resource")
     public ArchiveInputStream() {
-        this(new NullInputStream(), Charset.defaultCharset());
+        this(new NullInputStream(), Charset.defaultCharset().name());
     }
 
     /**
      * Constructs a new instance.
      *
      * @param inputStream the underlying input stream, or {@code null} if this instance is to be created without an underlying stream.
-     * @param charset charset.
+     * @param charsetName charset name.
+     * @since 1.26.0
+     */
+    protected ArchiveInputStream(final InputStream inputStream, final String charsetName) {
+        super(inputStream == null ? new NullInputStream() : inputStream);
+        this.charset = Charsets.toCharset(charsetName);
+    }
+
+    /**
+     * Constructs a new instance from a builder.
+     *
+     * @param builder The builder.
      * @since 1.29.0
      */
-    protected ArchiveInputStream(final InputStream inputStream, final Charset charset) {
-        super(inputStream);
-        this.charset = Charsets.toCharset(charset);
+    protected ArchiveInputStream(AbstractArchiveBuilder<?, ?> builder) throws IOException {
+        this(builder.getInputStream(), builder);
     }
 
     /**
-     * Constructs a new instance.
+     * Constructs a new instance from a builder and an input stream.
      *
-     * @param inputStream the underlying input stream, or {@code null} if this instance is to be created without an underlying stream.
-     * @param charsetName charset name.
-     * @since 1.26.0
+     * <p><strong>Note:</strong> This overload exists to support legacy constructors that did not declare
+     * {@link IOException}. For new constructors, prefer
+     * {@link #ArchiveInputStream(AbstractArchiveBuilder)} and propagate I/O errors to callers.</p>
+     *
+     * @param inputStream The underlying input stream, or {@code null} if this instance is to be created without an underlying stream.
+     * @param builder The builder.
+     * @since 1.29.0
      */
-    protected ArchiveInputStream(final InputStream inputStream, final String charsetName) {
-        this(inputStream, Charsets.toCharset(charsetName));
+    protected ArchiveInputStream(final InputStream inputStream, final AbstractArchiveBuilder<?, ?> builder) {
+        super(inputStream);
+        this.charset = builder.getCharset();
     }
 
     /**

src/main/java/org/apache/commons/compress/archivers/Lister.java

@@ -155,7 +155,7 @@ private  void listStream(final Path file, final String[] args) throws ArchiveExc
     }
 
     private  void listZipUsingTarFile(final Path file) throws IOException {
-        try (TarFile tarFile = new TarFile(file)) {
+        try (TarFile tarFile = TarFile.builder().setPath(file).get()) {
             println("Created " + tarFile);
             tarFile.getEntries().forEach(this::println);
         }

src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java

@@ -25,6 +25,7 @@
 import java.util.Arrays;
 import java.util.regex.Pattern;
 
+import org.apache.commons.compress.archivers.AbstractArchiveBuilder;
 import org.apache.commons.compress.archivers.ArchiveException;
 import org.apache.commons.compress.archivers.ArchiveInputStream;
 import org.apache.commons.compress.utils.ArchiveUtils;
@@ -52,7 +53,7 @@ public class ArArchiveInputStream extends ArchiveInputStream<ArArchiveEntry> {
      *
      * @since 1.29.0
      */
-    public static final class Builder extends AbstractBuilder<ArArchiveInputStream, Builder> {
+    public static final class Builder extends AbstractArchiveBuilder<ArArchiveInputStream, Builder> {
 
         private Builder() {
             setCharset(StandardCharsets.US_ASCII);
@@ -167,7 +168,7 @@ public static boolean matches(final byte[] buffer, final int ignored) {
     private final byte[] metaData = new byte[NAME_LEN + LAST_MODIFIED_LEN + USER_ID_LEN + GROUP_ID_LEN + FILE_MODE_LEN + LENGTH_LEN];
 
     private ArArchiveInputStream(final Builder builder) throws IOException {
-        this(builder.getInputStream(), builder);
+        super(builder);
     }
 
     /**
@@ -180,7 +181,7 @@ public ArArchiveInputStream(final InputStream inputStream) {
     }
 
     private ArArchiveInputStream(final InputStream inputStream, final Builder builder) {
-        super(inputStream, builder.getCharset());
+        super(inputStream, builder);
     }
 
     private int asInt(final byte[] byteArray, final int offset, final int len, final boolean treatBlankAsZero) throws IOException {

src/main/java/org/apache/commons/compress/archivers/arj/ArjArchiveInputStream.java

@@ -27,6 +27,7 @@
 import java.util.ArrayList;
 import java.util.zip.CRC32;
 
+import org.apache.commons.compress.archivers.AbstractArchiveBuilder;
 import org.apache.commons.compress.archivers.ArchiveEntry;
 import org.apache.commons.compress.archivers.ArchiveException;
 import org.apache.commons.compress.archivers.ArchiveInputStream;
@@ -60,7 +61,7 @@ public class ArjArchiveInputStream extends ArchiveInputStream<ArjArchiveEntry> {
      *
      * @since 1.29.0
      */
-    public static final class Builder extends AbstractBuilder<ArjArchiveInputStream, Builder> {
+    public static final class Builder extends AbstractArchiveBuilder<ArjArchiveInputStream, Builder> {
 
         private Builder() {
             setCharset(ENCODING_NAME);
@@ -117,7 +118,7 @@ public ArjArchiveInputStream(final InputStream inputStream) throws ArchiveExcept
     }
 
     private ArjArchiveInputStream(final InputStream inputStream, final Builder builder) throws ArchiveException {
-        super(new DataInputStream(inputStream), builder.getCharset());
+        super(new DataInputStream(inputStream), builder);
         dis = (DataInputStream) in;
         try {
             mainHeader = readMainHeader();

src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStream.java

@@ -22,6 +22,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 
+import org.apache.commons.compress.archivers.AbstractArchiveBuilder;
 import org.apache.commons.compress.archivers.ArchiveException;
 import org.apache.commons.compress.archivers.ArchiveInputStream;
 import org.apache.commons.compress.archivers.zip.ZipEncoding;
@@ -79,7 +80,7 @@ public class CpioArchiveInputStream extends ArchiveInputStream<CpioArchiveEntry>
      *
      * @since 1.29.0
      */
-    public static final class Builder extends AbstractBuilder<CpioArchiveInputStream, Builder> {
+    public static final class Builder extends AbstractArchiveBuilder<CpioArchiveInputStream, Builder> {
 
         private int blockSize = BLOCK_SIZE;
 
@@ -211,7 +212,7 @@ public CpioArchiveInputStream(final InputStream in) {
     }
 
     private CpioArchiveInputStream(final InputStream in, final Builder builder) {
-        super(in, builder.getCharset());
+        super(in, builder);
         if (builder.blockSize <= 0) {
             throw new IllegalArgumentException("blockSize must be bigger than 0");
         }

src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStream.java

@@ -29,6 +29,7 @@
 import java.util.Queue;
 import java.util.Stack;
 
+import org.apache.commons.compress.archivers.AbstractArchiveBuilder;
 import org.apache.commons.compress.archivers.ArchiveException;
 import org.apache.commons.compress.archivers.ArchiveInputStream;
 import org.apache.commons.compress.archivers.zip.ZipEncoding;
@@ -60,7 +61,7 @@ public class DumpArchiveInputStream extends ArchiveInputStream<DumpArchiveEntry>
      *
      * @since 1.29.0
      */
-    public static final class Builder extends AbstractBuilder<DumpArchiveInputStream, Builder> {
+    public static final class Builder extends AbstractArchiveBuilder<DumpArchiveInputStream, Builder> {
 
         private Builder() {
         }
@@ -153,7 +154,7 @@ public DumpArchiveInputStream(final InputStream is) throws ArchiveException {
     }
 
     private DumpArchiveInputStream(final InputStream is, final Builder builder) throws ArchiveException {
-        super(is, builder.getCharset());
+        super(is, builder);
         this.raw = new TapeInputStream(is);
         this.hasHitEOF = false;
         this.zipEncoding = ZipEncodingHelper.getZipEncoding(builder.getCharset());

src/main/java/org/apache/commons/compress/archivers/examples/Expander.java

@@ -395,11 +395,11 @@ public void expand(final String format, final SeekableByteChannel archive, final
             if (!prefersSeekableByteChannel(format)) {
                 expand(format, c.track(Channels.newInputStream(archive)), targetDirectory, CloseableConsumer.NULL_CONSUMER);
             } else if (ArchiveStreamFactory.TAR.equalsIgnoreCase(format)) {
-                expand(c.track(new TarFile(archive)), targetDirectory);
+                expand(c.track(TarFile.builder().setChannel(archive).get()), targetDirectory);
             } else if (ArchiveStreamFactory.ZIP.equalsIgnoreCase(format)) {
-                expand(c.track(ZipFile.builder().setSeekableByteChannel(archive).get()), targetDirectory);
+                expand(c.track(ZipFile.builder().setChannel(archive).get()), targetDirectory);
             } else if (ArchiveStreamFactory.SEVEN_Z.equalsIgnoreCase(format)) {
-                expand(c.track(SevenZFile.builder().setSeekableByteChannel(archive).get()), targetDirectory);
+                expand(c.track(SevenZFile.builder().setChannel(archive).get()), targetDirectory);
             } else {
                 // never reached as prefersSeekableByteChannel only returns true for TAR, ZIP and 7z
                 throw new ArchiveException("Don't know how to handle format '%s'", format);