BZ-19516 Use BufferedInputStream for reduced memory usage in Zip task, in certain cases

WHATSNEW

@@ -32,6 +32,11 @@ Fixed bugs:
    up creating a new symlink under the target directory.
    Bugzilla Report 58683
 
+ * Improvement to the Zip task for reduced memory usage in certain
+   cases. Thanks to Glen Lewis for reporting the issue and 
+   suggesting the fix.
+   Bugzilla Report 19516
+
 Other changes:
 --------------
 

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

@@ -17,8 +17,7 @@
  */
 package org.apache.tools.ant.taskdefs;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
+import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -1077,7 +1076,7 @@ public class Zip extends MatchingTask {
                 if (keepCompression) {
                     doCompress = (ze.getMethod() == ZipEntry.DEFLATED);
                 }
-                try (InputStream is = zf.getInputStream(ze)) {
+                try (final BufferedInputStream is = new BufferedInputStream(zf.getInputStream(ze))) {
                     zipFile(is, zOut, prefix + name, ze.getTime(),
                             fromArchive, mode, ze.getExtraFields(true));
                 } finally {
@@ -1085,7 +1084,7 @@ public class Zip extends MatchingTask {
                 }
             }
         } else {
-            try (InputStream is = r.getInputStream()) {
+            try (final BufferedInputStream is = new BufferedInputStream(r.getInputStream())) {
                 zipFile(is, zOut, prefix + name, r.getLastModified(),
                         fromArchive, mode, r instanceof ZipResource
                         ? ((ZipResource) r).getExtraFields() : null);
@@ -1776,7 +1775,7 @@ public class Zip extends MatchingTask {
      * @since Ant 1.5.2
      * @throws IOException on error
      */
-    protected void zipFile(InputStream in, final ZipOutputStream zOut, final String vPath,
+    protected void zipFile(final InputStream in, final ZipOutputStream zOut, final String vPath,
                            final long lastModified, final File fromArchive, final int mode)
         throws IOException {
         // fromArchive is used in subclasses overriding this method
@@ -1806,7 +1805,12 @@ public class Zip extends MatchingTask {
             final ZipEntry ze = new ZipEntry(vPath);
             ze.setTime(fixedModTime != null ? modTimeMillis : lastModified);
             ze.setMethod(doCompress ? ZipEntry.DEFLATED : ZipEntry.STORED);
-
+            // if the input stream doesn't support mark/reset ability, we wrap it in a
+            // stream that adds that support.
+            // Note: We do *not* close this newly created wrapping input stream, since
+            // we don't "own" the underlying input stream that's passed to us and closing
+            // that is the responsibility of the caller.
+            final InputStream markableInputStream = in.markSupported() ? in : new BufferedInputStream(in);
             /*
              * ZipOutputStream.putNextEntry expects the ZipEntry to
              * know its size and the CRC sum before you start writing
@@ -1817,31 +1821,15 @@ public class Zip extends MatchingTask {
             if (!zOut.isSeekable() && !doCompress) {
                 long size = 0;
                 final CRC32 cal = new CRC32();
-                if (!in.markSupported()) {
-                    // Store data into a byte[]
-                    final ByteArrayOutputStream bos = new ByteArrayOutputStream();
-
-                    final byte[] buffer = new byte[BUFFER_SIZE];
-                    int count = 0;
-                    do {
-                        size += count;
-                        cal.update(buffer, 0, count);
-                        bos.write(buffer, 0, count);
-                        count = in.read(buffer, 0, buffer.length);
-                    } while (count != -1);
-                    in = new ByteArrayInputStream(bos.toByteArray());
-
-                } else {
-                    in.mark(Integer.MAX_VALUE);
-                    final byte[] buffer = new byte[BUFFER_SIZE];
-                    int count = 0;
-                    do {
-                        size += count;
-                        cal.update(buffer, 0, count);
-                        count = in.read(buffer, 0, buffer.length);
-                    } while (count != -1);
-                    in.reset();
-                }
+                markableInputStream.mark(Integer.MAX_VALUE);
+                final byte[] buffer = new byte[BUFFER_SIZE];
+                int count = 0;
+                do {
+                    size += count;
+                    cal.update(buffer, 0, count);
+                    count = markableInputStream.read(buffer, 0, buffer.length);
+                } while (count != -1);
+                markableInputStream.reset();
                 ze.setSize(size);
                 ze.setCrc(cal.getValue());
             }
@@ -1860,7 +1848,7 @@ public class Zip extends MatchingTask {
                 if (count != 0) {
                     zOut.write(buffer, 0, count);
                 }
-                count = in.read(buffer, 0, buffer.length);
+                count = markableInputStream.read(buffer, 0, buffer.length);
             } while (count != -1);
         }
         addedFiles.add(vPath);
@@ -1916,9 +1904,9 @@ public class Zip extends MatchingTask {
                                      getLocation());
         }
 
-        try (InputStream fIn = Files.newInputStream(file.toPath())) {
+        try (final BufferedInputStream bIn = new BufferedInputStream(Files.newInputStream(file.toPath()))) {
             // ZIPs store time with a granularity of 2 seconds, round up
-            zipFile(fIn, zOut, vPath,
+            zipFile(bIn, zOut, vPath,
                     file.lastModified() + (roundUp ? ROUNDUP_MILLIS : 0),
                     null, mode);
         }