diff --git a/src/main/java/org/apache/commons/compress/compressors/gzip/ExtraField.java b/src/main/java/org/apache/commons/compress/compressors/gzip/ExtraField.java index 3bf5a8a7203..2de74e9fb6b 100644 --- a/src/main/java/org/apache/commons/compress/compressors/gzip/ExtraField.java +++ b/src/main/java/org/apache/commons/compress/compressors/gzip/ExtraField.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Objects; @@ -28,8 +29,17 @@ import org.apache.commons.compress.compressors.gzip.ExtraField.SubField; /** - * If the {@code FLG.FEXTRA} bit is set, an "extra field" is present in the header, with total length XLEN bytes. It consists of a series of subfields, each of - * the form: + * If the {@code FLG.FEXTRA} bit is set, an "extra field" is present in the + * header, with total length XLEN bytes. + * + *
+ * +---+---+=================================+
+ * | XLEN  |...XLEN bytes of "extra field"...| (more-->)
+ * +---+---+=================================+
+ * 
+ * + * This class represents the extra field payload (excluding the XLEN 2 bytes). + * The ExtraField payload consists of a series of subfields, each of the form: * *
  * +---+---+---+---+==================================+
@@ -37,8 +47,9 @@
  * +---+---+---+---+==================================+
  * 
* - * This class does not expose the internal subfields list to prevent adding subfields without total extra length validation. However a copy of the list is - * available. + * This class does not expose the internal subfields list to prevent adding + * subfields without total extra length validation. The class is iterable, but + * this iterator is immutable. * * @see RFC 1952 GZIP File Format Specification * @since 1.28.0 @@ -46,8 +57,8 @@ public class ExtraField implements Iterable { /** - * If the {@code FLG.FEXTRA} bit is set, an "extra field" is present in the header, with total length XLEN bytes. It consists of a series of subfields, each - * of the form: + * If the {@code FLG.FEXTRA} bit is set, an "extra field" is present in the header, with total length XLEN bytes. + * It consists of a series of subfields, each of the form: * *
      * +---+---+---+---+==================================+
@@ -185,13 +196,14 @@ public SubField getSubFieldAt(final int index) {
     }
 
     /**
-     * Returns an iterator over the SubField elements in this extra field in proper sequence.
+     * Returns an immutable iterator over the SubField elements in this extra field
+     * in the order they were added.
      *
-     * @return an iterator over the SubField elements in this extra field in proper sequence.
+     * @return an immutable naturally ordered iterator over the SubField elements.
      */
     @Override
     public Iterator iterator() {
-        return subFields.iterator();
+        return Collections.unmodifiableList(subFields).iterator();
     }
 
     byte[] toByteArray() {
@@ -211,4 +223,50 @@ byte[] toByteArray() {
         return ba;
     }
 
+    /**
+     * Test is this extra field has no subfields.
+     *
+     * @return true if there are no subfields, false otherwise.
+     */
+    public boolean isEmpty() {
+        return subFields.isEmpty();
+    }
+
+    /**
+     * Removes all subfields from this instance.
+     */
+    public void clear() {
+        subFields.clear();
+        totalSize = 0;
+    }
+
+    /**
+     * Calculate the size in bytes of the encoded extra field. This does not include
+     * its own 16 bits size when embeded in the gzip header. For N sub fields, the
+     * total is all subfields payloads bytes + 4N.
+     *
+     * @return the bytes count of this extra payload when encoded.
+     */
+    public int getEncodedSize() {
+        return totalSize;
+    }
+
+    /**
+     * Return the count of subfields currently in in this extra field.
+     *
+     * @return the count of subfields contained in this instance.
+     */
+    public int getSize() {
+        return subFields.size();
+    }
+
+    /**
+     * Finds the first subfield that matched the id if found, null otherwise
+     *
+     * @return the 1st SubField that matched or null.
+     */
+    public SubField findFirstSubField(String subfieldId) {
+        return subFields.stream().filter(f -> f.getId().equals(subfieldId)).findFirst().orElse(null);
+    }
+
 }
diff --git a/src/test/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStreamTest.java b/src/test/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStreamTest.java
index 3f7b930fb0e..370148b98eb 100644
--- a/src/test/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStreamTest.java
@@ -22,6 +22,8 @@
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
@@ -32,8 +34,8 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.commons.compress.compressors.gzip.ExtraField.SubField;
 import org.junit.jupiter.api.Test;
@@ -94,11 +96,12 @@ public void testChineseFileNameUTF8() throws IOException {
     /**
      * Tests the gzip extra header containing subfields.
      *
-     * @throws IOException When the test fails.
+     * @throws IOException When the test has issues with the underlying file system or unexpected gzip operations.
      */
     @ParameterizedTest
     // @formatter:off
     @CsvSource({
+        "0,    42, false",
         "1,      , true",
         "1,     0, false",
         "1, 65531, false",
@@ -108,7 +111,8 @@ public void testChineseFileNameUTF8() throws IOException {
         "2, 32763, false"
     })
     // @formatter:on
-    public void testExtraSubfields(final int subFieldCount, final Integer payloadSize, final boolean shouldFail) throws IOException {
+    public void testExtraSubfields(final int subFieldCount, final Integer payloadSize, final boolean shouldFail)
+            throws IOException {
         final Path tempSourceFile = Files.createTempFile("test_gzip_extra_", ".txt");
         final Path targetFile = Files.createTempFile("test_gzip_extra_", ".txt.gz");
         Files.write(tempSourceFile, "Hello World!".getBytes(StandardCharsets.ISO_8859_1));
@@ -128,10 +132,13 @@ public void testExtraSubfields(final int subFieldCount, final Integer payloadSiz
                 break;
             }
         }
-        assertEquals(shouldFail, failed, "appending subfield " + (shouldFail ? "succes" : "failure") + " was not expected.");
+        assertEquals(shouldFail, failed, "add subfield " + (shouldFail ? "succes" : "failure") + " was not expected.");
         if (shouldFail) {
             return;
         }
+        if (subFieldCount > 0) {
+            assertThrows(UnsupportedOperationException.class, () -> extra.iterator().remove());
+        }
         parameters.setExtraField(extra);
         try (OutputStream fos = Files.newOutputStream(targetFile);
                 GzipCompressorOutputStream gos = new GzipCompressorOutputStream(fos, parameters)) {
@@ -139,17 +146,21 @@ public void testExtraSubfields(final int subFieldCount, final Integer payloadSiz
         }
         try (GzipCompressorInputStream gis = new GzipCompressorInputStream(Files.newInputStream(targetFile))) {
             final ExtraField extra2 = gis.getMetaData().getExtraField();
+            assertEquals(subFieldCount == 0, extra2.isEmpty());
+            assertEquals(subFieldCount, extra2.getSize());
+            assertEquals(4 * subFieldCount + subFieldCount * payloadSize, extra2.getEncodedSize());
+            ArrayList listCopy = new ArrayList<>();
+            extra2.forEach(listCopy::add);
+            assertEquals(subFieldCount, listCopy.size());
             for (int i = 0; i < subFieldCount; i++) {
                 final SubField sf = extra2.getSubFieldAt(i);
+                assertSame(sf, listCopy.get(i));
+                assertSame(sf, extra2.findFirstSubField("z" + i));
                 assertEquals("z" + i, sf.getId()); // id was saved/loaded correctly
                 assertArrayEquals(payloads[i], sf.getPayload(), "field " + i + " has wrong payload");
             }
-            final AtomicInteger i = new AtomicInteger();
-            gis.getMetaData().getExtraField().forEach(sf -> {
-                assertEquals("z" + i, sf.getId()); // id was saved/loaded correctly
-                assertArrayEquals(payloads[i.intValue()], sf.getPayload(), "field " + i + " has wrong payload");
-                i.incrementAndGet();
-            });
+            extra2.clear();
+            assertTrue(extra2.isEmpty());
         }
     }