Made up my mind on the fix for PR 35000. Empty != broken, so make it two separate use cases.

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@677870 13f79535-47bb-0310-9956-ffa450edef68

WHATSNEW

@@ -58,9 +58,8 @@ Changes that could break older environments:
    passed in a null or empty InputStream to read from.
    Bugzilla Report 32200
 
- * <unzip> and <untar> will now fail on empty archives (or ZIP
-   archives with an empty central directory).
-   set failOnEmptyArchive to false to restore the old behavior.
+ * <unzip> will now fail when trying to extract certain broken
+   archives that would have been silently ignored in earlier version.
    Bugzilla report 35000.
 
  * Ant's <zip> family of tasks tries to preserve the existing Unix
@@ -226,6 +225,10 @@ Other changes:
    authentication.
    Bugzilla report 33718.
 
+ * a new failOnEmptyArchive attribute on <unzip> and <untar> can now
+   make the task fail the build if it tries to extract an empty
+   archive.
+
 Changes from Ant 1.7.0 TO Ant 1.7.1
 =============================================
 

docs/manual/CoreTasks/unzip.html

@@ -114,7 +114,7 @@ archive.</p>
     <td valign="top">failOnEmptyArchive</td>
     <td valign="top">whether trying to extract an empty archive is an
       error. <em>since Ant 1.8.0</em></td>
-    <td valign="top" align="center">No, defaults to true</td>
+    <td valign="top" align="center">No, defaults to false</td>
   </tr>
 </table>
 <h3>Examples</h3>

src/main/org/apache/tools/ant/taskdefs/Expand.java

@@ -66,7 +66,7 @@ public class Expand extends Task {
     private Vector patternsets = new Vector();
     private Union resources = new Union();
     private boolean resourcesSpecified = false;
-    private boolean failOnEmptyArchive = true;
+    private boolean failOnEmptyArchive = false;
 
     private static final String NATIVE_ENCODING = "native-encoding";
 
@@ -164,15 +164,19 @@ public class Expand extends Task {
                     getLocation());
         }
         try {
-            zf = new ZipFile(srcF, encoding, failOnEmptyArchive);
+            zf = new ZipFile(srcF, encoding);
+            boolean empty = true;
             Enumeration e = zf.getEntries();
             while (e.hasMoreElements()) {
+                empty = false;
                 ZipEntry ze = (ZipEntry) e.nextElement();
                 extractFile(fileUtils, srcF, dir, zf.getInputStream(ze),
                             ze.getName(), new Date(ze.getTime()),
                             ze.isDirectory(), mapper);
             }
-
+            if (empty && getFailOnEmptyArchive()) {
+                throw new BuildException("archive '" + srcF + "' is empty");
+            }
             log("expand complete", Project.MSG_VERBOSE);
         } catch (IOException ioe) {
             throw new BuildException(

src/main/org/apache/tools/ant/taskdefs/Untar.java

@@ -158,7 +158,7 @@ public class Untar extends Expand {
                             te.isDirectory(), mapper);
             }
             if (empty && getFailOnEmptyArchive()) {
-                throw new BuildException("archive is empty");
+                throw new BuildException("archive '" + name + "' is empty");
             }
             log("expand complete", Project.MSG_VERBOSE);
         } finally {

src/main/org/apache/tools/zip/ZipFile.java

@@ -146,78 +146,10 @@ public class ZipFile {
      * @throws IOException if an error occurs while reading the file.
      */
     public ZipFile(File f, String encoding) throws IOException {
-        this(f, encoding, false);
-    }
-
-    /**
-     * Opens the given file for reading, assuming the platform's
-     * native encoding for file names.
-     *
-     * @param f the archive.
-     * @param mustNotBeEmpty whether an empty central directory should
-     * case an error
-     *
-     * @throws IOException if an error occurs while reading the file.
-     *
-     * @since Ant 1.8.0
-     */
-    public ZipFile(File f, boolean mustNotBeEmpty) throws IOException {
-        this(f, null, mustNotBeEmpty);
-    }
-
-    /**
-     * Opens the given file for reading, assuming the platform's
-     * native encoding for file names.
-     *
-     * @param name name of the archive.
-     * @param mustNotBeEmpty whether an empty central directory should
-     * case an error
-     *
-     * @throws IOException if an error occurs while reading the file.
-     *
-     * @since Ant 1.8.0
-     */
-    public ZipFile(String name, boolean mustNotBeEmpty) throws IOException {
-        this(new File(name), null, mustNotBeEmpty);
-    }
-
-    /**
-     * Opens the given file for reading, assuming the specified
-     * encoding for file names.
-     *
-     * @param name name of the archive.
-     * @param encoding the encoding to use for file names
-     * @param mustNotBeEmpty whether an empty central directory should
-     * case an error
-     *
-     * @throws IOException if an error occurs while reading the file.
-     *
-     * @since Ant 1.8.0
-     */
-    public ZipFile(String name, String encoding,
-                   boolean mustNotBeEmpty) throws IOException {
-        this(new File(name), encoding, mustNotBeEmpty);
-    }
-
-    /**
-     * Opens the given file for reading, assuming the specified
-     * encoding for file names.
-     *
-     * @param f the archive.
-     * @param encoding the encoding to use for file names
-     * @param mustNotBeEmpty whether an empty central directory should
-     * case an error
-     *
-     * @throws IOException if an error occurs while reading the file.
-     *
-     * @since Ant 1.8.0
-     */
-    public ZipFile(File f, String encoding,
-                   boolean mustNotBeEmpty) throws IOException {
         this.encoding = encoding;
         archive = new RandomAccessFile(f, "r");
         try {
-            populateFromCentralDirectory(mustNotBeEmpty);
+            populateFromCentralDirectory();
             resolveLocalFileHeaderData();
         } catch (IOException e) {
             try {
@@ -334,7 +266,7 @@ public class ZipFile {
      * the central directory alone, but not the data that requires the
      * local file header or additional data to be read.</p>
      */
-    private void populateFromCentralDirectory(boolean mustNotBeEmpty)
+    private void populateFromCentralDirectory()
         throws IOException {
         positionAtCentralDirectory();
 
@@ -344,9 +276,9 @@ public class ZipFile {
         archive.readFully(signatureBytes);
         long sig = ZipLong.getValue(signatureBytes);
         final long cfhSig = ZipLong.getValue(ZipOutputStream.CFH_SIG);
-        if (mustNotBeEmpty && sig != cfhSig) {
+        if (sig != cfhSig && startsWithLocalFileHeader()) {
             throw new IOException("central directory is empty, can't expand"
-                                  + " archive.");
+                                  + " corrupt archive.");
         }
         while (sig == cfhSig) {
             archive.readFully(cfh);
@@ -581,6 +513,22 @@ public class ZipFile {
         }
     }
 
+    /**
+     * Checks whether the archive starts with a LFH.  If it doesn't,
+     * it may be an empty archive.
+     */
+    private boolean startsWithLocalFileHeader() throws IOException {
+        archive.seek(0);
+        final byte[] start = new byte[WORD];
+        archive.readFully(start);
+        for (int i = 0; i < start.length; i++) {
+            if (start[i] != ZipOutputStream.LFH_SIG[i]) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * InputStream that delegates requests to the underlying
      * RandomAccessFile, making sure that only bytes from a certain

src/tests/antunit/taskdefs/unzip-test.xml

@@ -32,7 +32,7 @@
 
   <target name="testFailureOnBrokenCentralDirectoryStructure">
     <au:expectfailure
-       expectedmessage="central directory is empty, can't expand archive.">
+       expectedmessage="central directory is empty, can't expand corrupt archive.">
       <unzip src="broken_cd.zip" dest="${dest.dir}"/>
     </au:expectfailure>
   </target>