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/16 14:30:00 UTC

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

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 f43c608  COMPRESS-479 use strategy pattern to handle unparseable extra fields
     new 1f5fed3  this class looks wierd nowadays
     new 60a389c  COMPRESS-479 allow strategy to control all parts of extra field parsing

The 2 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/zip/ExtraFieldParsingBehavior.java   |  62 +++++++++++
 .../compress/archivers/zip/ExtraFieldUtils.java    | 113 +++++++++++----------
 .../compress/archivers/zip/ZipArchiveEntry.java    |  98 ++++++++++--------
 3 files changed, 180 insertions(+), 93 deletions(-)
 create mode 100644 src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldParsingBehavior.java


[commons-compress] 02/02: COMPRESS-479 allow strategy to control all parts of extra field parsing

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 60a389c2ffa88f5a5de9e902ac2cfdf17fb57993
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Fri Aug 16 16:29:05 2019 +0200

    COMPRESS-479 allow strategy to control all parts of extra field parsing
---
 .../archivers/zip/ExtraFieldParsingBehavior.java   |  62 ++++++++++++
 .../compress/archivers/zip/ExtraFieldUtils.java    | 110 +++++++++++----------
 .../compress/archivers/zip/ZipArchiveEntry.java    |  98 ++++++++++--------
 3 files changed, 177 insertions(+), 93 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
new file mode 100644
index 0000000..2e29450
--- /dev/null
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ExtraFieldParsingBehavior.java
@@ -0,0 +1,62 @@
+/*
+ *  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.zip;
+
+import java.util.zip.ZipException;
+
+/**
+ * Controls details of parsing zip extra fields.
+ *
+ * @since 1.19
+ */
+public interface ExtraFieldParsingBehavior extends UnparseableExtraFieldBehavior {
+    /**
+     * Creates an instance of ZipExtraField for the given id.
+     *
+     * <p>A good default implementation would be {@link
+     * ExtraFieldUtils#createExtraField}.</p>
+     *
+     * @param headerId the id for the extra field
+     * @return an instance of ZipExtraField, must not be null
+     * @throws ZipException if an error occurs
+     * @throws InstantiationException if unable to instantiate the class
+     * @throws IllegalAccessException if not allowed to instantiate the class
+     */
+    ZipExtraField createExtraField(final ZipShort headerId)
+        throws ZipException, InstantiationException, IllegalAccessException;
+
+    /**
+     * Fills in the extra field data for a single extra field.
+     *
+     * <p>A good default implementation would be {@link
+     * ExtraFieldUtils#fillExtraField}.</p>
+     *
+     * @param field 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. Usually this is the same as {@code
+     * field} but it oculd be a replacement extra field as well
+     * @throws ZipException if an error occurs
+     */
+    ZipExtraField fill(ZipExtraField field, byte[] data, int off, int len, boolean local)
+        throws ZipException;
+}
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 3985972..3fb60c3 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
@@ -85,9 +85,9 @@ public class ExtraFieldUtils {
      */
     public static ZipExtraField createExtraField(final ZipShort headerId)
         throws InstantiationException, IllegalAccessException {
-        final Class<?> c = implementations.get(headerId);
-        if (c != null) {
-            return (ZipExtraField) c.newInstance();
+        ZipExtraField field = createExtraFieldNoDefault(headerId);
+        if (field != null) {
+            return field;
         }
         final UnrecognizedExtraField u = new UnrecognizedExtraField();
         u.setHeaderId(headerId);
@@ -95,6 +95,24 @@ public class ExtraFieldUtils {
     }
 
     /**
+     * Create an instance of the appropriate ExtraField.
+     * @param headerId the header identifier
+     * @return an instance of the appropriate ExtraField or null if
+     * the id is not supported
+     * @throws InstantiationException if unable to instantiate the class
+     * @throws IllegalAccessException if not allowed to instantiate the class
+     * @since 1.19
+     */
+    public static ZipExtraField createExtraFieldNoDefault(final ZipShort headerId)
+        throws InstantiationException, IllegalAccessException {
+        final Class<?> c = implementations.get(headerId);
+        if (c != null) {
+            return (ZipExtraField) c.newInstance();
+        }
+        return null;
+    }
+
+    /**
      * Split the array into ExtraFields and populate them with the
      * given data as local file data, throwing an exception if the
      * data cannot be parsed.
@@ -136,7 +154,25 @@ public class ExtraFieldUtils {
     public static ZipExtraField[] parse(final byte[] data, final boolean local,
                                         final UnparseableExtraFieldBehavior onUnparseableData)
         throws ZipException {
-        return parse(data, local, onUnparseableData, ParseErrorBehavior.THROW);
+        return parse(data, local, new ExtraFieldParsingBehavior() {
+            @Override
+            public ZipExtraField onUnparseableExtraField(byte[] data, int off, int len, boolean local,
+                int claimedLength) throws ZipException {
+                return onUnparseableData.onUnparseableExtraField(data, off, len, local, claimedLength);
+            }
+
+            @Override
+            public ZipExtraField createExtraField(final ZipShort headerId)
+                throws ZipException, InstantiationException, IllegalAccessException {
+                return ExtraFieldUtils.createExtraField(headerId);
+            }
+
+            @Override
+            public ZipExtraField fill(ZipExtraField field, byte[] data, int off, int len, boolean local)
+                throws ZipException {
+                return fillExtraField(field, data, off, len, local);
+            }
+        });
     }
 
     /**
@@ -157,8 +193,7 @@ public class ExtraFieldUtils {
      * @since 1.19
      */
     public static ZipExtraField[] parse(final byte[] data, final boolean local,
-                                        final UnparseableExtraFieldBehavior onUnparseableData,
-                                        final ParseErrorBehavior onParseError)
+                                        final ExtraFieldParsingBehavior parsingbehavior)
         throws ZipException {
         final List<ZipExtraField> v = new ArrayList<>();
         int start = 0;
@@ -167,7 +202,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 = onUnparseableData.onUnparseableExtraField(data, start, data.length - start,
+                ZipExtraField field = parsingbehavior.onUnparseableExtraField(data, start, data.length - start,
                     local, length);
                 if (field != null) {
                     v.add(field);
@@ -177,23 +212,13 @@ public class ExtraFieldUtils {
                 // available data
                 break LOOP;
             }
-            switch (onParseError) {
-            case MAKE_UNRECOGNIZED:
-                try {
-                    v.add(parseField(headerId, local, data, start + WORD, length));
-                } catch (ZipException ex) {
-                    final UnrecognizedExtraField u = new UnrecognizedExtraField();
-                    u.setHeaderId(headerId);
-                    fillField(u, local, data, start + WORD, length);
-                    v.add(u);
-                }
-                break;
-            case THROW: // FALLTHROUGH
-            default:
-                v.add(parseField(headerId, local, data, start + WORD, length));
-                break;
+            try {
+                ZipExtraField ze = parsingbehavior.createExtraField(headerId);
+                v.add(parsingbehavior.fill(ze, data, start + WORD, length, local));
+                start += length + WORD;
+            } catch (final InstantiationException | IllegalAccessException ie) {
+                throw (ZipException) new ZipException(ie.getMessage()).initCause(ie);
             }
-            start += length + WORD;
         }
 
         final ZipExtraField[] result = new ZipExtraField[v.size()];
@@ -277,25 +302,22 @@ public class ExtraFieldUtils {
         return result;
     }
 
-    private static ZipExtraField parseField(final ZipShort headerId, final boolean local, final byte[] data,
-        final int off, final int len) throws ZipException {
-        try {
-            final ZipExtraField ze = createExtraField(headerId);
-            fillField(ze, local, data, off, len);
-            return ze;
-        } catch (final InstantiationException | IllegalAccessException ie) {
-            throw (ZipException) new ZipException(ie.getMessage()).initCause(ie);
-        }
-    }
-
-    private static void fillField(final ZipExtraField ze, final boolean local, final byte[] data, final int off,
-        final int len) throws ZipException {
+    /**
+     * Fills in the extra field data into the given instance.
+     *
+     * <p>Calls {@link ZipExtraField#parseFromCentralDirectoryData} or {@link ZipExtraField#parseFromLocalFileData} internally and wraps any {@link ArrayIndexOutOfBoundsException} thrown into a {@link ZipException}.</p>
+     *
+     * @since 1.19
+     */
+    public static ZipExtraField fillExtraField(final ZipExtraField ze, final byte[] data, final int off,
+        final int len, final boolean local) throws ZipException {
         try {
             if (local) {
                 ze.parseFromLocalFileData(data, off, len);
             } else {
                 ze.parseFromCentralDirectoryData(data, off, len);
             }
+            return ze;
         } catch (ArrayIndexOutOfBoundsException aiobe) {
             throw (ZipException) new ZipException("Failed to parse corrupt ZIP extra field of type "
                 + Integer.toHexString(ze.getHeaderId().getValue())).initCause(aiobe);
@@ -384,20 +406,4 @@ public class ExtraFieldUtils {
         }
 
     }
-
-    /**
-     * What shall {@link #parse} do if parsing the extra field fails.
-     *
-     * @since 1.19
-     */
-    public enum ParseErrorBehavior {
-        /**
-         * Throw an exception if parsing the extra field fails.
-         */
-        THROW,
-        /**
-         * Replace the extra field with an instance of an {@link UnrecognizedExtraField}.
-         */
-        MAKE_UNRECOGNIZED;
-    }
 }
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 585dfb2..68b34b4 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
@@ -173,9 +173,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
         setName(entry.getName());
         final byte[] extra = entry.getExtra();
         if (extra != null) {
-            setExtraFields(ExtraFieldUtils.parse(extra, true,
-                ExtraFieldUtils.UnparseableExtraField.READ, ExtraFieldUtils.ParseErrorBehavior.MAKE_UNRECOGNIZED));
-        } else {
+            setExtraFields(ExtraFieldUtils.parse(extra, true, ExtraFieldParsingMode.BEST_EFFORT));        } else {
             // initializes extra data to an empty byte array
             setExtra();
         }
@@ -445,27 +443,25 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
 
     /**
      * Retrieves extra fields.
-     * @param includeUnparseable whether to also return unparseable
-     * extra fields as {@link UnparseableExtraFieldData} if such data
-     * exists.
+     * @param parsingBehavior controls parsing of extra fields.
      * @return an array of the extra fields
      *
      * @throws ZipException if parsing fails, can not happen if {@code
-     * mode} is {@link ExtraFieldParsingMode.BEST_EFFORT}.
+     * parsingBehavior} is {@link ExtraFieldParsingMode.BEST_EFFORT}.
      *
      * @since 1.19
      */
-    public ZipExtraField[] getExtraFields(final ExtraFieldParsingMode mode)
+    public ZipExtraField[] getExtraFields(final ExtraFieldParsingBehavior parsingBehavior)
         throws ZipException {
-        if (mode == ExtraFieldParsingMode.BEST_EFFORT) {
+        if (parsingBehavior == ExtraFieldParsingMode.BEST_EFFORT) {
             return getExtraFields(true);
         }
         byte[] local = getExtra();
         List<ZipExtraField> localFields = new ArrayList<>(Arrays.asList(ExtraFieldUtils.parse(local, true,
-            mode.getOnUnparseableData(), mode.getOnParseError())));
+            parsingBehavior)));
         byte[] central = ExtraFieldUtils.mergeCentralDirectoryData(getAllExtraFieldsNoCopy());
         List<ZipExtraField> centralFields = new ArrayList<>(Arrays.asList(ExtraFieldUtils.parse(central, false,
-            mode.getOnUnparseableData(), mode.getOnParseError())));
+            parsingBehavior)));
         List<ZipExtraField> merged = new ArrayList<>();
         for (ZipExtraField l : localFields) {
             ZipExtraField c = null;
@@ -676,10 +672,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
     @Override
     public void setExtra(final byte[] extra) throws RuntimeException {
         try {
-            final ZipExtraField[] local =
-                ExtraFieldUtils.parse(extra, true,
-                    ExtraFieldUtils.UnparseableExtraField.READ,
-                    ExtraFieldUtils.ParseErrorBehavior.MAKE_UNRECOGNIZED);
+            final ZipExtraField[] local = ExtraFieldUtils.parse(extra, true, ExtraFieldParsingMode.BEST_EFFORT);
             mergeExtraFields(local, true);
         } catch (final ZipException e) {
             // actually this is not possible as of Commons Compress 1.1
@@ -704,10 +697,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
      */
     public void setCentralDirectoryExtra(final byte[] b) {
         try {
-            final ZipExtraField[] central =
-                ExtraFieldUtils.parse(b, false,
-                    ExtraFieldUtils.UnparseableExtraField.READ,
-                    ExtraFieldUtils.ParseErrorBehavior.MAKE_UNRECOGNIZED);
+            final ZipExtraField[] central = ExtraFieldUtils.parse(b, false, ExtraFieldParsingMode.BEST_EFFORT);
             mergeExtraFields(central, false);
         } catch (final ZipException e) {
             // actually this is not possible as of Commons Compress 1.19
@@ -1114,7 +1104,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
      *
      * @since 1.19
      */
-    public enum ExtraFieldParsingMode {
+    public enum ExtraFieldParsingMode implements ExtraFieldParsingBehavior {
         /**
          * Try to parse as many extra fields as possible and wrap
          * unknown extra fields as well as supported extra fields that
@@ -1126,8 +1116,12 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
          *
          * <p>This is the default behavior starting with Commons Compress 1.19.</p>
          */
-        BEST_EFFORT(ExtraFieldUtils.UnparseableExtraField.READ,
-            ExtraFieldUtils.ParseErrorBehavior.MAKE_UNRECOGNIZED),
+        BEST_EFFORT(ExtraFieldUtils.UnparseableExtraField.READ) {
+            @Override
+            public ZipExtraField fill(ZipExtraField field, byte[] data, int off, int len, boolean local) {
+                return fillAndMakeUnrecognizedOnError(field, data, off, len, local);
+            }
+        },
         /**
          * Try to parse as many extra fields as possible and wrap
          * unknown extra fields in {@link UnrecognizedExtraField}.
@@ -1142,8 +1136,7 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
          * <p>This used to be the default behavior prior to Commons
          * Compress 1.19.</p>
          */
-        STRICT_FOR_KNOW_EXTRA_FIELDS(ExtraFieldUtils.UnparseableExtraField.READ,
-            ExtraFieldUtils.ParseErrorBehavior.THROW),
+        STRICT_FOR_KNOW_EXTRA_FIELDS(ExtraFieldUtils.UnparseableExtraField.READ),
         /**
          * Try to parse as many extra fields as possible and wrap
          * unknown extra fields as well as supported extra fields that
@@ -1151,12 +1144,13 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
          *
          * <p>Ignore extra data that doesn't follow the recommended
          * pattern.</p>
-         *
-         * <p>This used to be the default behavior prior to Commons
-         * Compress 1.19.</p>
          */
-        ONLY_PARSEABLE_LENIENT(ExtraFieldUtils.UnparseableExtraField.SKIP,
-            ExtraFieldUtils.ParseErrorBehavior.THROW),
+        ONLY_PARSEABLE_LENIENT(ExtraFieldUtils.UnparseableExtraField.SKIP) {
+            @Override
+            public ZipExtraField fill(ZipExtraField field, byte[] data, int off, int len, boolean local) {
+                return fillAndMakeUnrecognizedOnError(field, data, off, len, local);
+            }
+        },
         /**
          * Try to parse as many extra fields as possible and wrap
          * unknown extra fields in {@link UnrecognizedExtraField}.
@@ -1167,30 +1161,52 @@ public class ZipArchiveEntry extends java.util.zip.ZipEntry
          * <p>Throw an exception if an extra field that is generally
          * supported cannot be parsed.</p>
          */
-        ONLY_PARSEABLE_STRICT(ExtraFieldUtils.UnparseableExtraField.SKIP,
-            ExtraFieldUtils.ParseErrorBehavior.THROW),
+        ONLY_PARSEABLE_STRICT(ExtraFieldUtils.UnparseableExtraField.SKIP),
         /**
          * Throw an exception if any of the recognized extra fields
          * cannot be parsed or any extra field violates the
          * recommended pattern.
          */
-        DRACONIC(ExtraFieldUtils.UnparseableExtraField.THROW,
-            ExtraFieldUtils.ParseErrorBehavior.THROW);
+        DRACONIC(ExtraFieldUtils.UnparseableExtraField.THROW);
 
         private final ExtraFieldUtils.UnparseableExtraField onUnparseableData;
-        private final ExtraFieldUtils.ParseErrorBehavior onParseError;
 
-        private ExtraFieldParsingMode(ExtraFieldUtils.UnparseableExtraField onUnparseableData,
-                                      ExtraFieldUtils.ParseErrorBehavior onParseError) {
+        private ExtraFieldParsingMode(ExtraFieldUtils.UnparseableExtraField onUnparseableData) {
             this.onUnparseableData = onUnparseableData;
-            this.onParseError = onParseError;
         }
 
-        public ExtraFieldUtils.UnparseableExtraField getOnUnparseableData() {
-            return onUnparseableData;
+        @Override
+        public ZipExtraField onUnparseableExtraField(byte[] data, int off, int len, boolean local,
+            int claimedLength) throws ZipException {
+            return onUnparseableData.onUnparseableExtraField(data, off, len, local, claimedLength);
         }
-        public ExtraFieldUtils.ParseErrorBehavior getOnParseError() {
-            return onParseError;
+
+        @Override
+        public ZipExtraField createExtraField(final ZipShort headerId)
+            throws ZipException, InstantiationException, IllegalAccessException {
+            return ExtraFieldUtils.createExtraField(headerId);
+        }
+
+        @Override
+        public ZipExtraField fill(ZipExtraField field, byte[] data, int off, int len, boolean local)
+            throws ZipException {
+            return ExtraFieldUtils.fillExtraField(field, data, off, len, local);
+        }
+
+        private static ZipExtraField fillAndMakeUnrecognizedOnError(ZipExtraField field, byte[] data, int off,
+            int len, boolean local) {
+            try {
+                return ExtraFieldUtils.fillExtraField(field, data, off, len, local);
+            } catch (ZipException ex) {
+                final UnrecognizedExtraField u = new UnrecognizedExtraField();
+                u.setHeaderId(field.getHeaderId());
+                if (local) {
+                    u.setLocalFileDataData(Arrays.copyOfRange(data, off, off + len));
+                } else {
+                    u.setCentralDirectoryData(Arrays.copyOfRange(data, off, off + len));
+                }
+                return u;
+            }
         }
     }
 }


[commons-compress] 01/02: this class looks wierd nowadays

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 1f5fed36ffbe5c58186239d428589b8437011931
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Fri Aug 16 15:22:08 2019 +0200

    this class looks wierd nowadays
---
 .../org/apache/commons/compress/archivers/zip/ExtraFieldUtils.java     | 3 +++
 1 file changed, 3 insertions(+)

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 aa54bac..3985972 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
@@ -306,6 +306,9 @@ public class ExtraFieldUtils {
      * "enum" for the possible actions to take if the extra field
      * cannot be parsed.
      *
+     * <p>This class has been created long before Java 5 and would
+     * have been a real enum ever since.</p>
+     *
      * @since 1.1
      */
     public static final class UnparseableExtraField implements UnparseableExtraFieldBehavior {