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/17 10:28:10 UTC

[commons-compress] branch master updated (60a389c -> dd0063a)

This is an automated email from the ASF dual-hosted git repository.

bodewig pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git.


    from 60a389c  COMPRESS-479 allow strategy to control all parts of extra field parsing
     new 923aa6e  COMPRESS-479 verify non-null return requirements
     new 6b8ae85  javadoc warnings
     new dd0063a  COMPRESS-479 simplify some implementation parts, add tests

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../archivers/examples/CloseableConsumer.java      |  5 ++
 .../archivers/zip/ExtraFieldParsingBehavior.java   |  5 +-
 .../compress/archivers/zip/ExtraFieldUtils.java    | 28 ++++---
 .../zip/X0017_StrongEncryptionHeader.java          |  2 +
 .../compress/archivers/zip/ZipArchiveEntry.java    | 87 +++++++++++-----------
 .../archivers/zip/ZipArchiveEntryTest.java         | 64 +++++++++++++++-
 6 files changed, 132 insertions(+), 59 deletions(-)


[commons-compress] 03/03: COMPRESS-479 simplify some implementation parts, add tests

Posted by bo...@apache.org.
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 dd0063a0ee0349dcbd71c2e0ac7341df7dec6742
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Sat Aug 17 12:27:36 2019 +0200

    COMPRESS-479 simplify some implementation parts, add tests
---
 .../compress/archivers/zip/ZipArchiveEntry.java    | 85 +++++++++++-----------
 .../archivers/zip/ZipArchiveEntryTest.java         | 64 +++++++++++++++-
 2 files changed, 103 insertions(+), 46 deletions(-)

diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
index 0412d21..767f615 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
@@ -27,6 +27,8 @@ import java.util.Date;
 import java.util.List;
 import java.util.zip.ZipException;
 
+import static java.util.Arrays.copyOf;
+
 /**
  * Extension that adds better handling of extra fields and provides
  * access to the internal and external file attributes.
@@ -400,15 +402,18 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
      * @param fields an array of extra fields
      */
     public void setExtraFields(final ZipExtraField[] fields) {
+        unparseableExtra = null;
         final List<ZipExtraField> newFields = new ArrayList<>();
-        for (final ZipExtraField field : fields) {
-            if (field instanceof UnparseableExtraFieldData) {
-                unparseableExtra = (UnparseableExtraFieldData) field;
-            } else {
-                newFields.add( field);
+        if (fields != null) {
+            for (final ZipExtraField field : fields) {
+                if (field instanceof UnparseableExtraFieldData) {
+                    unparseableExtra = (UnparseableExtraFieldData) field;
+                } else {
+                    newFields.add(field);
+                }
             }
         }
-        extraFields = newFields.toArray(new ZipExtraField[0]);
+        extraFields = newFields.toArray(noExtraFields);
         setExtra();
     }
 
@@ -456,10 +461,13 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
         if (parsingBehavior == ExtraFieldParsingMode.BEST_EFFORT) {
             return getExtraFields(true);
         }
+        if (parsingBehavior == ExtraFieldParsingMode.ONLY_PARSEABLE_LENIENT) {
+            return getExtraFields(false);
+        }
         byte[] local = getExtra();
         List<ZipExtraField> localFields = new ArrayList<>(Arrays.asList(ExtraFieldUtils.parse(local, true,
             parsingBehavior)));
-        byte[] central = ExtraFieldUtils.mergeCentralDirectoryData(getAllExtraFieldsNoCopy());
+        byte[] central = getCentralDirectoryExtra();
         List<ZipExtraField> centralFields = new ArrayList<>(Arrays.asList(ExtraFieldUtils.parse(central, false,
             parsingBehavior)));
         List<ZipExtraField> merged = new ArrayList<>();
@@ -480,7 +488,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
             merged.add(l);
         }
         merged.addAll(centralFields);
-        return merged.toArray(new ZipExtraField[0]);
+        return merged.toArray(noExtraFields);
     }
 
     private ZipExtraField[] getParseableExtraFieldsNoCopy() {
@@ -492,7 +500,8 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
 
     private ZipExtraField[] getParseableExtraFields() {
         final ZipExtraField[] parseableExtraFields = getParseableExtraFieldsNoCopy();
-        return (parseableExtraFields == extraFields) ? copyOf(parseableExtraFields) : parseableExtraFields;
+        return (parseableExtraFields == extraFields) ? copyOf(parseableExtraFields, parseableExtraFields.length)
+            : parseableExtraFields;
     }
 
     /**
@@ -506,14 +515,6 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
         return unparseableExtra != null ? getMergedFields() : extraFields;
     }
 
-    private ZipExtraField[] copyOf(final ZipExtraField[] src) {
-        return copyOf(src, src.length);
-    }
-
-    private ZipExtraField[] copyOf(final ZipExtraField[] src, final int length) {
-        return Arrays.copyOf(src, length);
-    }
-
     private ZipExtraField[] getMergedFields() {
         final ZipExtraField[] zipExtraFields = copyOf(extraFields, extraFields.length + 1);
         zipExtraFields[extraFields.length] = unparseableExtra;
@@ -526,7 +527,8 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
 
     private ZipExtraField[] getAllExtraFields() {
         final ZipExtraField[] allExtraFieldsNoCopy = getAllExtraFieldsNoCopy();
-        return (allExtraFieldsNoCopy == extraFields) ? copyOf( allExtraFieldsNoCopy) : allExtraFieldsNoCopy;
+        return (allExtraFieldsNoCopy == extraFields) ? copyOf(allExtraFieldsNoCopy, allExtraFieldsNoCopy.length)
+            : allExtraFieldsNoCopy;
     }
 
     private ZipExtraField findUnparseable(List<ZipExtraField> fs) {
@@ -560,13 +562,13 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
             unparseableExtra = (UnparseableExtraFieldData) ze;
         } else {
             if (extraFields == null) {
-                extraFields = new ZipExtraField[]{ ze};
+                extraFields = new ZipExtraField[]{ ze };
             } else {
-                if (getExtraField(ze.getHeaderId())!= null){
+                if (getExtraField(ze.getHeaderId()) != null) {
                     removeExtraField(ze.getHeaderId());
                 }
                 final ZipExtraField[] zipExtraFields = copyOf(extraFields, extraFields.length + 1);
-                zipExtraFields[zipExtraFields.length -1] = ze;
+                zipExtraFields[zipExtraFields.length - 1] = ze;
                 extraFields = zipExtraFields;
             }
         }
@@ -584,11 +586,11 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
         if (ze instanceof UnparseableExtraFieldData) {
             unparseableExtra = (UnparseableExtraFieldData) ze;
         } else {
-            if (getExtraField(ze.getHeaderId()) != null){
+            if (getExtraField(ze.getHeaderId()) != null) {
                 removeExtraField(ze.getHeaderId());
             }
             final ZipExtraField[] copy = extraFields;
-            final int newLen = extraFields != null ? extraFields.length + 1: 1;
+            final int newLen = extraFields != null ? extraFields.length + 1 : 1;
             extraFields = new ZipExtraField[newLen];
             extraFields[0] = ze;
             if (copy != null){
@@ -609,14 +611,14 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
 
         final List<ZipExtraField> newResult = new ArrayList<>();
         for (final ZipExtraField extraField : extraFields) {
-            if (!type.equals(extraField.getHeaderId())){
-                newResult.add( extraField);
+            if (!type.equals(extraField.getHeaderId())) {
+                newResult.add(extraField);
             }
         }
         if (extraFields.length == newResult.size()) {
             throw new java.util.NoSuchElementException();
         }
-        extraFields = newResult.toArray(new ZipExtraField[0]);
+        extraFields = newResult.toArray(noExtraFields);
         setExtra();
     }
 
@@ -740,7 +742,8 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
      */
     @Override
     public boolean isDirectory() {
-        return getName().endsWith("/");
+        final String n = getName();
+        return n != null && n.endsWith("/");
     }
 
     /**
@@ -809,7 +812,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
      */
     public byte[] getRawName() {
         if (rawName != null) {
-            return Arrays.copyOf(rawName, rawName.length);
+            return copyOf(rawName, rawName.length);
         }
         return null;
     }
@@ -857,7 +860,8 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
         // on the super.hashCode() method since super.getName() always return
         // the empty string in the current implemention (there's no setter)
         // so it is basically draining the performance of a hashmap lookup
-        return getName().hashCode();
+        final String n = getName();
+        return (n == null ? "" : n).hashCode();
     }
 
     /**
@@ -886,8 +890,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
      * @param f the extra fields to merge
      * @param local whether the new fields originate from local data
      */
-    private void mergeExtraFields(final ZipExtraField[] f, final boolean local)
-        throws ZipException {
+    private void mergeExtraFields(final ZipExtraField[] f, final boolean local) {
         if (extraFields == null) {
             setExtraFields(f);
         } else {
@@ -901,30 +904,24 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
                 if (existing == null) {
                     addExtraField(element);
                 } else {
-                    byte[] b = null;
+                    final byte[] b = local ? element.getLocalFileDataData()
+                        : element.getCentralDirectoryData();
                     try {
                         if (local) {
-                            b = element.getLocalFileDataData();
                             existing.parseFromLocalFileData(b, 0, b.length);
                         } else {
-                            b = element.getCentralDirectoryData();
                             existing.parseFromCentralDirectoryData(b, 0, b.length);
                         }
                     } catch (ZipException ex) {
-                        if (b == null) {
-                            throw ex;
-                        }
-                        // emulate ExtraFieldUtils.ParseErrorBehavior.MAKE_UNRECOGNIZED
+                        // emulate ExtraFieldParsingMode.fillAndMakeUnrecognizedOnError
                         final UnrecognizedExtraField u = new UnrecognizedExtraField();
                         u.setHeaderId(existing.getHeaderId());
                         if (local) {
-                            final byte[] raw = existing.getCentralDirectoryData();
-                            u.parseFromLocalFileData(b, 0, b.length);
-                            u.parseFromCentralDirectoryData(raw, 0, raw.length);
+                            u.setLocalFileDataData(b);
+                            u.setCentralDirectoryData(existing.getCentralDirectoryData());
                         } else {
-                            final byte[] raw = existing.getLocalFileDataData();
-                            u.parseFromLocalFileData(raw, 0, raw.length);
-                            u.parseFromCentralDirectoryData(b, 0, b.length);
+                            u.setLocalFileDataData(existing.getLocalFileDataData());
+                            u.setCentralDirectoryData(b);
                         }
                         removeExtraField(existing.getHeaderId());
                         addExtraField(u);
diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntryTest.java b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntryTest.java
index 584235f..4ae85e0 100644
--- a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntryTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntryTest.java
@@ -299,12 +299,72 @@ public class ZipArchiveEntryTest {
     @Test
     public void reparsingUnicodeExtraWithUnsupportedversionThrowsInStrictMode()
         throws Exception {
-        thrown.expect(ZipException.class);
-        thrown.expectMessage("Unsupported version [116] for UniCode path extra data.");
         try (ZipFile zf = new ZipFile(getFile("COMPRESS-479.zip"))) {
             ZipArchiveEntry ze = zf.getEntry("%U20AC_for_Dollar.txt");
+            thrown.expect(ZipException.class);
+            thrown.expectMessage("Unsupported version [116] for UniCode path extra data.");
             ze.getExtraFields(ZipArchiveEntry.ExtraFieldParsingMode.STRICT_FOR_KNOW_EXTRA_FIELDS);
         }
     }
 
+    @Test
+    public void bestEffortIncludesUnparseableExtraData() throws Exception {
+        ZipExtraField[] extraFields = parsingModeBehaviorTestData();
+        ZipArchiveEntry ze = new ZipArchiveEntry("foo");
+        ze.setExtraFields(extraFields);
+        ZipExtraField[] read = ze.getExtraFields(ZipArchiveEntry.ExtraFieldParsingMode.BEST_EFFORT);
+        assertEquals(extraFields.length, read.length);
+    }
+
+    @Test
+    public void onlyParseableLenientExcludesUnparseableExtraData() throws Exception {
+        ZipExtraField[] extraFields = parsingModeBehaviorTestData();
+        ZipArchiveEntry ze = new ZipArchiveEntry("foo");
+        ze.setExtraFields(extraFields);
+        ZipExtraField[] read = ze.getExtraFields(ZipArchiveEntry.ExtraFieldParsingMode.ONLY_PARSEABLE_LENIENT);
+        assertEquals(extraFields.length, read.length + 1);
+    }
+
+    @Test
+    public void strictForKnowExtraFieldsIncludesUnparseableExtraData() throws Exception {
+        ZipExtraField[] extraFields = parsingModeBehaviorTestData();
+        ZipArchiveEntry ze = new ZipArchiveEntry("foo");
+        ze.setExtraFields(extraFields);
+        ZipExtraField[] read = ze.getExtraFields(ZipArchiveEntry.ExtraFieldParsingMode.STRICT_FOR_KNOW_EXTRA_FIELDS);
+        assertEquals(extraFields.length, read.length);
+    }
+
+    @Test
+    public void onlyParseableStrictExcludesUnparseableExtraData() throws Exception {
+        ZipExtraField[] extraFields = parsingModeBehaviorTestData();
+        ZipArchiveEntry ze = new ZipArchiveEntry("foo");
+        ze.setExtraFields(extraFields);
+        ZipExtraField[] read = ze.getExtraFields(ZipArchiveEntry.ExtraFieldParsingMode.ONLY_PARSEABLE_STRICT);
+        assertEquals(extraFields.length, read.length + 1);
+    }
+
+    @Test
+    public void draconicThrowsOnUnparseableExtraData() throws Exception {
+        ZipExtraField[] extraFields = parsingModeBehaviorTestData();
+        ZipArchiveEntry ze = new ZipArchiveEntry("foo");
+        ze.setExtraFields(extraFields);
+        thrown.expect(ZipException.class);
+        thrown.expectMessage("Bad extra field starting at");
+        ze.getExtraFields(ZipArchiveEntry.ExtraFieldParsingMode.DRACONIC);
+    }
+
+    private ZipExtraField[] parsingModeBehaviorTestData() {
+        final AsiExtraField a = new AsiExtraField();
+        a.setDirectory(true);
+        a.setMode(0755);
+        final UnrecognizedExtraField u = new UnrecognizedExtraField();
+        u.setHeaderId(ExtraFieldUtilsTest.UNRECOGNIZED_HEADER);
+        u.setLocalFileDataData(new byte[0]);
+        final UnparseableExtraFieldData x = new UnparseableExtraFieldData();
+        byte[] unparseable = new byte[] {
+            0, 0, (byte) 0xff, (byte) 0xff, 0, 0, 0
+        };
+        x.parseFromLocalFileData(unparseable, 0, unparseable.length);
+        return new ZipExtraField[] { a, u, x };
+    }
 }


[commons-compress] 02/03: javadoc warnings

Posted by bo...@apache.org.
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 6b8ae855e41aa1c0e7e07c72d384c5ce88f67999
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Fri Aug 16 18:14:33 2019 +0200

    javadoc warnings
---
 .../archivers/examples/CloseableConsumer.java      |  5 +++++
 .../compress/archivers/zip/ExtraFieldUtils.java    | 25 +++++++++++++---------
 .../zip/X0017_StrongEncryptionHeader.java          |  2 ++
 .../compress/archivers/zip/ZipArchiveEntry.java    |  2 +-
 4 files changed, 23 insertions(+), 11 deletions(-)

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
index 670bf40..dead47f 100644
--- a/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumer.java
+++ b/src/main/java/org/apache/commons/compress/archivers/examples/CloseableConsumer.java
@@ -53,6 +53,11 @@ public interface CloseableConsumer {
      * Callback that is informed about a closable resource that has
      * been wrapped around a passed in stream or channel by Expander
      * or Archiver when Expander or Archiver no longer need them.
+     *
+     * @param c Closeable created by Expander or Archiver that is now
+     * no longer used
+     *
+     * @throws IOException on error
      */
     void accept(Closeable c) throws IOException;
 }
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java b/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java
index 78f16ba..d0309e7 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java
@@ -180,21 +180,16 @@ public class ExtraFieldUtils {
      * Split the array into ExtraFields and populate them with the
      * given data.
      * @param data an array of bytes
+     * @param parsingBehavior controls parsing of extra fields.
      * @param local whether data originates from the local file data
      * or the central directory
-     * @param onUnparseableData what to do if the extra field data
-     * cannot be parsed.
-     * @param onParseError what to do if the field's key is recognized
-     * but our implementation class fails to handle it. If the key and
-     * length cannot be parsed at all {@code onUnparseableData} will
-     * determine the behavior.
      * @return an array of ExtraFields
      * @throws ZipException on error
      *
      * @since 1.19
      */
     public static ZipExtraField[] parse(final byte[] data, final boolean local,
-                                        final ExtraFieldParsingBehavior parsingbehavior)
+                                        final ExtraFieldParsingBehavior parsingBehavior)
         throws ZipException {
         final List<ZipExtraField> v = new ArrayList<>();
         int start = 0;
@@ -203,7 +198,7 @@ public class ExtraFieldUtils {
             final ZipShort headerId = new ZipShort(data, start);
             final int length = new ZipShort(data, start + 2).getValue();
             if (start + WORD + length > data.length) {
-                ZipExtraField field = parsingbehavior.onUnparseableExtraField(data, start, data.length - start,
+                ZipExtraField field = parsingBehavior.onUnparseableExtraField(data, start, data.length - start,
                     local, length);
                 if (field != null) {
                     v.add(field);
@@ -214,9 +209,9 @@ public class ExtraFieldUtils {
                 break LOOP;
             }
             try {
-                ZipExtraField ze = Objects.requireNonNull(parsingbehavior.createExtraField(headerId),
+                ZipExtraField ze = Objects.requireNonNull(parsingBehavior.createExtraField(headerId),
                     "createExtraField must not return null");
-                v.add(Objects.requireNonNull(parsingbehavior.fill(ze, data, start + WORD, length, local),
+                v.add(Objects.requireNonNull(parsingBehavior.fill(ze, data, start + WORD, length, local),
                     "fill must not return null"));
                 start += length + WORD;
             } catch (final InstantiationException | IllegalAccessException ie) {
@@ -310,6 +305,16 @@ public class ExtraFieldUtils {
      *
      * <p>Calls {@link ZipExtraField#parseFromCentralDirectoryData} or {@link ZipExtraField#parseFromLocalFileData} internally and wraps any {@link ArrayIndexOutOfBoundsException} thrown into a {@link ZipException}.</p>
      *
+     * @param ze the extra field instance to fill
+     * @param data the array of extra field data
+     * @param off offset into data where this field's data starts
+     * @param len the length of this field's data
+     * @param local whether the extra field data stems from the local
+     * file header. If this is false then the data is part if the
+     * central directory header extra data.
+     * @return the filled field, will never be {@code null}
+     * @throws ZipException if an error occurs
+     *
      * @since 1.19
      */
     public static ZipExtraField fillExtraField(final ZipExtraField ze, final byte[] data, final int off,
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java b/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java
index d3669e9..a9600a7 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java
@@ -301,6 +301,7 @@ public class X0017_StrongEncryptionHeader extends PKWareExtraHeader {
      * @param data the buffer to read data from
      * @param offset offset into buffer to read data
      * @param length the length of data
+     * @throws ZipException if an error occurs
      */
     public void parseCentralDirectoryFormat(final byte[] data, final int offset, final int length)
         throws ZipException {
@@ -333,6 +334,7 @@ public class X0017_StrongEncryptionHeader extends PKWareExtraHeader {
      * @param data the buffer to read data from
      * @param offset offset into buffer to read data
      * @param length the length of data
+     * @throws ZipException if an error occurs
      */
     public void parseFileFormat(final byte[] data, final int offset, final int length)
         throws ZipException {
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
index 68b34b4..0412d21 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
@@ -447,7 +447,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
      * @return an array of the extra fields
      *
      * @throws ZipException if parsing fails, can not happen if {@code
-     * parsingBehavior} is {@link ExtraFieldParsingMode.BEST_EFFORT}.
+     * parsingBehavior} is {@link ExtraFieldParsingMode#BEST_EFFORT}.
      *
      * @since 1.19
      */


[commons-compress] 01/03: COMPRESS-479 verify non-null return requirements

Posted by bo...@apache.org.
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 923aa6ee0a46706d35a7d12c47e72bd1c3efc8f2
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Fri Aug 16 18:02:37 2019 +0200

    COMPRESS-479 verify non-null return requirements
---
 .../commons/compress/archivers/zip/ExtraFieldParsingBehavior.java  | 5 +++--
 .../org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java | 7 +++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldParsingBehavior.java b/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldParsingBehavior.java
index 2e29450..943a41a 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldParsingBehavior.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldParsingBehavior.java
@@ -32,7 +32,7 @@ public interface ExtraFieldParsingBehavior extends UnparseableExtraFieldBehavior
      * ExtraFieldUtils#createExtraField}.</p>
      *
      * @param headerId the id for the extra field
-     * @return an instance of ZipExtraField, must not be null
+     * @return an instance of ZipExtraField, must not be {@code null}
      * @throws ZipException if an error occurs
      * @throws InstantiationException if unable to instantiate the class
      * @throws IllegalAccessException if not allowed to instantiate the class
@@ -54,7 +54,8 @@ public interface ExtraFieldParsingBehavior extends UnparseableExtraFieldBehavior
      * file header. If this is false then the data is part if the
      * central directory header extra data.
      * @return the filled field. Usually this is the same as {@code
-     * field} but it oculd be a replacement extra field as well
+     * field} but it oculd be a replacement extra field as well. Must
+     * not be {@code null}.
      * @throws ZipException if an error occurs
      */
     ZipExtraField fill(ZipExtraField field, byte[] data, int off, int len, boolean local)
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java b/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java
index 3fb60c3..78f16ba 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java
@@ -20,6 +20,7 @@ package org.apache.commons.compress.archivers.zip;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.zip.ZipException;
 
@@ -213,8 +214,10 @@ public class ExtraFieldUtils {
                 break LOOP;
             }
             try {
-                ZipExtraField ze = parsingbehavior.createExtraField(headerId);
-                v.add(parsingbehavior.fill(ze, data, start + WORD, length, local));
+                ZipExtraField ze = Objects.requireNonNull(parsingbehavior.createExtraField(headerId),
+                    "createExtraField must not return null");
+                v.add(Objects.requireNonNull(parsingbehavior.fill(ze, data, start + WORD, length, local),
+                    "fill must not return null"));
                 start += length + WORD;
             } catch (final InstantiationException | IllegalAccessException ie) {
                 throw (ZipException) new ZipException(ie.getMessage()).initCause(ie);