Allow disabling version to extract in zip.

Add an option to ZFile to ignore the "version to extract"
in the central directory header and in the local header.
This allows supporting some zip tools that write weird
values in the field.

Test: JUnit tests included
Change-Id: If8f14737ddb50f30508937e13f376cb30bde71e2

6 files changed, 173 insertions, 8 deletions

diff --git a/src/main/java/com/android/apkzlib/zip/CentralDirectory.java b/src/main/java/com/android/apkzlib/zip/CentralDirectory.java
index 083bc65..e088298 100644
--- a/src/main/java/com/android/apkzlib/zip/CentralDirectory.java
+++ b/src/main/java/com/android/apkzlib/zip/CentralDirectory.java
@@ -18,6 +18,7 @@ package com.android.apkzlib.zip;
 
 import com.android.apkzlib.utils.CachedSupplier;
 import com.android.apkzlib.zip.utils.MsDosDateTimeUtils;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
@@ -28,7 +29,6 @@ import com.google.common.util.concurrent.ListenableFuture;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -53,7 +53,8 @@ class CentralDirectory {
     /**
      * Field in the central directory with the minimum version required to extract the entry.
      */
-    private static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2(F_MADE_BY.endOffset(),
+    @VisibleForTesting
+    static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2(F_MADE_BY.endOffset(),
             "Version to extract", new ZipFieldInvariantNonNegative());
 
     /**
@@ -275,7 +276,8 @@ class CentralDirectory {
         long madeBy = F_MADE_BY.read(bytes);
 
         long versionNeededToExtract = F_VERSION_EXTRACT.read(bytes);
-        if (versionNeededToExtract > MAX_VERSION_TO_EXTRACT) {
+        if (versionNeededToExtract > MAX_VERSION_TO_EXTRACT
+                && !file.getSkipVersionToExtractValidation()) {
             throw new IOException("Unknown version needed to extract in zip directory entry: "
                     + versionNeededToExtract + ".");
         }
@@ -403,7 +405,7 @@ class CentralDirectory {
     private byte[] computeByteRepresentation() {
 
         List<StoredEntry> sorted = Lists.newArrayList(mEntries.values());
-        Collections.sort(sorted, StoredEntry.COMPARE_BY_NAME);
+        sorted.sort(StoredEntry.COMPARE_BY_NAME);
 
         CentralDirectoryHeader[] cdhs = new CentralDirectoryHeader[mEntries.size()];
         CentralDirectoryHeaderCompressInfo[] compressInfos =
diff --git a/src/main/java/com/android/apkzlib/zip/StoredEntry.java b/src/main/java/com/android/apkzlib/zip/StoredEntry.java
index a8ee56e..10289ca 100644
--- a/src/main/java/com/android/apkzlib/zip/StoredEntry.java
+++ b/src/main/java/com/android/apkzlib/zip/StoredEntry.java
@@ -18,6 +18,7 @@ package com.android.apkzlib.zip;
 
 import com.android.apkzlib.zip.utils.CloseableByteSource;
 import com.android.apkzlib.zip.utils.CloseableDelegateByteSource;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
 import com.google.common.io.ByteSource;
@@ -84,7 +85,8 @@ public class StoredEntry {
     /**
      * Local header field: version to extract, should match the CDH's.
      */
-    private static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2(
+    @VisibleForTesting
+    static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2(
             F_LOCAL_SIGNATURE.endOffset(), "Version to extract",
             new ZipFieldInvariantNonNegative());
 
@@ -399,7 +401,13 @@ public class StoredEntry {
 
         ByteBuffer bytes = ByteBuffer.wrap(localHeader);
         F_LOCAL_SIGNATURE.verify(bytes);
-        F_VERSION_EXTRACT.verify(bytes, compressInfo.getVersionExtract());
+
+        if (mFile.getSkipVersionToExtractValidation()) {
+            F_VERSION_EXTRACT.skip(bytes);
+        } else {
+            F_VERSION_EXTRACT.verify(bytes, compressInfo.getVersionExtract());
+        }
+
         F_GP_BIT.verify(bytes, mCdh.getGpBit().getValue());
         F_METHOD.verify(bytes, compressInfo.getMethod().methodCode);
 
diff --git a/src/main/java/com/android/apkzlib/zip/ZFile.java b/src/main/java/com/android/apkzlib/zip/ZFile.java
index 301b942..9fd1ef1 100644
--- a/src/main/java/com/android/apkzlib/zip/ZFile.java
+++ b/src/main/java/com/android/apkzlib/zip/ZFile.java
@@ -373,6 +373,12 @@ public class ZFile implements Closeable {
      */
     private boolean mSkipDataDescriptorVerification;
 
+    /**
+     * Should the "version to extract" field validation be skipped? See
+     * {@link ZFileOptions#getSkipZipVersionToExtractValidation()}.
+     */
+    private boolean mSkipVersionToExtractValidation;
+
 
     /**
      * Creates a new zip file. If the zip file does not exist, then no file is created at this
@@ -415,6 +421,7 @@ public class ZFile implements Closeable {
         mCoverEmptySpaceUsingExtraField = options.getCoverEmptySpaceUsingExtraField();
         mAutoSortFiles = options.getAutoSortFiles();
         mSkipDataDescriptorVerification = options.getSkipDataDescriptorValidation();
+        mSkipVersionToExtractValidation = options.getSkipZipVersionToExtractValidation();
 
         /*
          * These two values will be overwritten by openReadOnly() below if the file exists.
@@ -2416,6 +2423,15 @@ public class ZFile implements Closeable {
     }
 
     /**
+     * Checks whether the "version to extract" validation should be skipped.
+     *
+     * @return should it be skipped?
+     */
+    boolean getSkipVersionToExtractValidation() {
+        return mSkipVersionToExtractValidation;
+    }
+
+    /**
      * Hint to where files should be positioned.
      */
     enum PositionHint {
diff --git a/src/main/java/com/android/apkzlib/zip/ZFileOptions.java b/src/main/java/com/android/apkzlib/zip/ZFileOptions.java
index cc523c5..39da52a 100644
--- a/src/main/java/com/android/apkzlib/zip/ZFileOptions.java
+++ b/src/main/java/com/android/apkzlib/zip/ZFileOptions.java
@@ -66,6 +66,11 @@ public class ZFileOptions {
     private boolean mSkipDataDescriptionValidation;
 
     /**
+     * Should validation of minimum zip version to extract be performed?
+     */
+    private boolean mSkipZipVersionToExtractValidation;
+
+    /**
      * Creates a new options object. All options are set to their defaults.
      */
     public ZFileOptions() {
@@ -205,4 +210,24 @@ public class ZFileOptions {
     public void setSkipDataDescriptionValidation(boolean skip) {
         mSkipDataDescriptionValidation = skip;
     }
+
+    /**
+     * Should the "minimum zip version to extract" field of the local and central headers be
+     * validated or not? By default they are, but some zip tools put weird values in there.
+     *
+     * @return should the "zip version to extract" field be ignored?
+     */
+    public boolean getSkipZipVersionToExtractValidation() {
+        return mSkipZipVersionToExtractValidation;
+    }
+
+    /**
+     * Sets whether the "zip version to extract" field validation should be skipped. See
+     * {@link #getSkipZipVersionToExtractValidation()}.
+     *
+     * @param skip should the "zip version to extract" field be ignored?
+     */
+    public void setSkipZipVersionToExtractValidation(boolean skip) {
+        mSkipZipVersionToExtractValidation = skip;
+    }
 }
diff --git a/src/main/java/com/android/apkzlib/zip/ZipField.java b/src/main/java/com/android/apkzlib/zip/ZipField.java
index 3551e71..db6bb0b 100644
--- a/src/main/java/com/android/apkzlib/zip/ZipField.java
+++ b/src/main/java/com/android/apkzlib/zip/ZipField.java
@@ -247,6 +247,15 @@ abstract class ZipField {
     }
 
     /**
+     * Obtains the offset at which the field starts.
+     *
+     * @return the start offset
+     */
+    int offset() {
+        return mOffset;
+    }
+
+    /**
      * Obtains the offset at which the field ends. This is the exact offset at which the next
      * field starts.
      *
diff --git a/src/test/java/com/android/apkzlib/zip/ZFileTest.java b/src/test/java/com/android/apkzlib/zip/ZFileTest.java
index 742b9bd..265ef00 100644
--- a/src/test/java/com/android/apkzlib/zip/ZFileTest.java
+++ b/src/test/java/com/android/apkzlib/zip/ZFileTest.java
@@ -45,6 +45,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.RandomAccessFile;
+import java.util.Locale;
 import java.util.Random;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -1394,8 +1395,6 @@ public class ZFileTest {
             /*
              * We should be complaining about the CRC32 somewhere...
              */
-            boolean foundCrc32Complain = false;
-
             assertTrue(
                     Throwables.getCausalChain(e).stream()
                             .map(Throwable::getMessage)
@@ -1413,4 +1412,110 @@ public class ZFileTest {
              */
         }
     }
+
+    @Test
+    public void detectIncorrectVersionToExtractInCentralDirectory() throws Exception {
+        File zipFile = new File(mTemporaryFolder.getRoot(), "a.zip");
+
+        /*
+         * Create a valid zip file.
+         */
+        try (ZFile zf = new ZFile(zipFile)) {
+            zf.add("foo", new ByteArrayInputStream(new byte[0]));
+        }
+
+        /*
+         * Change the "version to extract" in the central directory to 0x7777.
+         */
+        int versionToExtractOffset =
+                ZFileTestConstants.LOCAL_HEADER_SIZE
+                        + 3
+                        + CentralDirectory.F_VERSION_EXTRACT.offset();
+        byte[] allZipBytes = Files.toByteArray(zipFile);
+        allZipBytes[versionToExtractOffset] = 0x77;
+        allZipBytes[versionToExtractOffset + 1] = 0x77;
+        Files.write(allZipBytes, zipFile);
+
+        /*
+         * Opening the file should fail.
+         */
+        try {
+            new ZFile(zipFile);
+            fail();
+        } catch (IOException e) {
+            /*
+             * We should complain about the version to extract somewhere...
+             */
+            assertTrue(
+                    Throwables.getCausalChain(e).stream()
+                            .map(Throwable::getMessage)
+                            .anyMatch(s -> s.toLowerCase(Locale.US).contains("version")));
+            assertTrue(
+                    Throwables.getCausalChain(e).stream()
+                            .map(Throwable::getMessage)
+                            .anyMatch(s -> s.toLowerCase(Locale.US).contains("extract")));
+        }
+
+        /*
+         * Setting the ignore version extract validation should allow the zip to be opened.
+         */
+        ZFileOptions options = new ZFileOptions();
+        options.setSkipZipVersionToExtractValidation(true);
+        try (ZFile zf = new ZFile(zipFile, options)) {
+            /*
+             * Nothing to do.
+             */
+        }
+    }
+    @Test
+    public void detectIncorrectVersionToExtractInLocalHeader() throws Exception {
+        File zipFile = new File(mTemporaryFolder.getRoot(), "a.zip");
+
+        /*
+         * Create a valid zip file.
+         */
+        try (ZFile zf = new ZFile(zipFile)) {
+            zf.add("foo", new ByteArrayInputStream(new byte[0]));
+        }
+
+        /*
+         * Change the "version to extract" in the local header to 0x7777.
+         */
+        int versionToExtractOffset = StoredEntry.F_VERSION_EXTRACT.offset();
+        byte[] allZipBytes = Files.toByteArray(zipFile);
+        allZipBytes[versionToExtractOffset] = 0x77;
+        allZipBytes[versionToExtractOffset + 1] = 0x77;
+        Files.write(allZipBytes, zipFile);
+
+        /*
+         * Opening the file should fail.
+         */
+        try {
+            new ZFile(zipFile);
+            fail();
+        } catch (IOException e) {
+            /*
+             * We should complain about the version to extract somewhere...
+             */
+            assertTrue(
+                    Throwables.getCausalChain(e).stream()
+                            .map(Throwable::getMessage)
+                            .anyMatch(s -> s.toLowerCase(Locale.US).contains("version")));
+            assertTrue(
+                    Throwables.getCausalChain(e).stream()
+                            .map(Throwable::getMessage)
+                            .anyMatch(s -> s.toLowerCase(Locale.US).contains("extract")));
+        }
+
+        /*
+         * Setting the ignore version extract validation should allow the zip to be opened.
+         */
+        ZFileOptions options = new ZFileOptions();
+        options.setSkipZipVersionToExtractValidation(true);
+        try (ZFile zf = new ZFile(zipFile, options)) {
+            /*
+             * Nothing to do.
+             */
+        }
+    }
 }