Allow disabling version to extract in zip. am: ba150accda

am: a64b0a1fe0

Change-Id: Ibd024e731e80d7feb7c38da3b94f4e7013eb85dd

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.
+             */
+        }
+    }
 }