Bug 684347: nsZipWriter::addEntryChannel(queue=true) creates corrupt zip file. r=mossop
authorKris Maglione <maglione.k@gmail.com>
Tue, 25 Oct 2011 17:30:59 -0700
changeset 79215 926699a40a9b17767447d481291037fe91330da3
parent 79212 cc66accc81813cd35c5da8f7d50b559c4185ecc2
child 79216 9bbb30ec51f5ab0e155c092ef8e1d03893331881
push id21380
push userbmo@edmorley.co.uk
push dateWed, 26 Oct 2011 23:31:27 +0000
treeherderautoland@16a8d2ab5240 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmossop
bugs684347
milestone10.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 684347: nsZipWriter::addEntryChannel(queue=true) creates corrupt zip file. r=mossop
modules/libjar/zipwriter/src/nsZipWriter.cpp
modules/libjar/zipwriter/test/unit/head_zipwriter.js
modules/libjar/zipwriter/test/unit/test_asyncadd.js
--- a/modules/libjar/zipwriter/src/nsZipWriter.cpp
+++ b/modules/libjar/zipwriter/src/nsZipWriter.cpp
@@ -907,53 +907,43 @@ inline nsresult nsZipWriter::BeginProces
                                             aItem->mFile);
             NS_ENSURE_SUCCESS(rv, rv);
         }
         // If a dir then this will fall through to the plain dir addition
     }
 
     PRUint32 zipAttributes = ZIP_ATTRS(aItem->mPermissions, ZIP_ATTRS_FILE);
 
-    if (aItem->mStream) {
+    if (aItem->mStream || aItem->mChannel) {
         nsRefPtr<nsZipHeader> header = new nsZipHeader();
         NS_ENSURE_TRUE(header, NS_ERROR_OUT_OF_MEMORY);
 
         header->Init(aItem->mZipEntry, aItem->mModTime, zipAttributes,
                      mCDSOffset);
         nsresult rv = header->WriteFileHeader(mStream);
         NS_ENSURE_SUCCESS(rv, rv);
 
         nsRefPtr<nsZipDataStream> stream = new nsZipDataStream();
+        NS_ENSURE_TRUE(stream, NS_ERROR_OUT_OF_MEMORY);
         rv = stream->Init(this, mStream, header, aItem->mCompression);
         NS_ENSURE_SUCCESS(rv, rv);
 
-        nsCOMPtr<nsIInputStreamPump> pump;
-        rv = NS_NewInputStreamPump(getter_AddRefs(pump), aItem->mStream, -1,
-                                   -1, 0, 0, true);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        rv = pump->AsyncRead(stream, nsnull);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        return NS_OK;
-    }
+        if (aItem->mStream) {
+            nsCOMPtr<nsIInputStreamPump> pump;
+            rv = NS_NewInputStreamPump(getter_AddRefs(pump), aItem->mStream,
+                                       -1, -1, 0, 0, true);
+            NS_ENSURE_SUCCESS(rv, rv);
 
-    if (aItem->mChannel) {
-        nsRefPtr<nsZipHeader> header = new nsZipHeader();
-        NS_ENSURE_TRUE(header, NS_ERROR_OUT_OF_MEMORY);
-
-        header->Init(aItem->mZipEntry, aItem->mModTime, zipAttributes,
-                     mCDSOffset);
-
-        nsRefPtr<nsZipDataStream> stream = new nsZipDataStream();
-        NS_ENSURE_TRUE(stream, NS_ERROR_OUT_OF_MEMORY);
-        nsresult rv = stream->Init(this, mStream, header, aItem->mCompression);
-        NS_ENSURE_SUCCESS(rv, rv);
-        rv = aItem->mChannel->AsyncOpen(stream, nsnull);
-        NS_ENSURE_SUCCESS(rv, rv);
+            rv = pump->AsyncRead(stream, nsnull);
+            NS_ENSURE_SUCCESS(rv, rv);
+        }
+        else {
+            rv = aItem->mChannel->AsyncOpen(stream, nsnull);
+            NS_ENSURE_SUCCESS(rv, rv);
+        }
 
         return NS_OK;
     }
 
     // Must be plain directory addition
     *complete = true;
     return InternalAddEntryDirectory(aItem->mZipEntry, aItem->mModTime,
                                      aItem->mPermissions);
--- a/modules/libjar/zipwriter/test/unit/head_zipwriter.js
+++ b/modules/libjar/zipwriter/test/unit/head_zipwriter.js
@@ -58,16 +58,19 @@ const ZIP_METHOD_DEFLATE = 8
 const ZIP_EXTENDED_TIMESTAMP_SIZE = 9;
 
 const PR_USEC_PER_MSEC = 1000;
 const PR_USEC_PER_SEC  = 1000000;
 const PR_MSEC_PER_SEC  = 1000;
 
 const DATA_DIR = "data/";
 
+var ioSvc = Cc["@mozilla.org/network/io-service;1"]
+             .getService(Ci.nsIIOService);
+
 var ZipWriter = Components.Constructor("@mozilla.org/zipwriter;1",
                                        "nsIZipWriter");
 var ZipReader = Components.Constructor("@mozilla.org/libjar/zip-reader;1",
                                        "nsIZipReader", "open");
 
 var dirSvc = Cc["@mozilla.org/file/directory_service;1"]
               .getService(Ci.nsIProperties);
 var tmpDir = dirSvc.get(NS_OS_TEMP_DIR, Ci.nsIFile);
--- a/modules/libjar/zipwriter/test/unit/test_asyncadd.js
+++ b/modules/libjar/zipwriter/test/unit/test_asyncadd.js
@@ -15,16 +15,17 @@
  *
  * The Initial Developer of the Original Code is
  * Dave Townsend <dtownsend@oxymoronical.com>.
  *
  * Portions created by the Initial Developer are Copyright (C) 2007
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
+ *     Kris Maglione <maglione.k@gmail.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -65,42 +66,67 @@ var observer = {
     size += ZIP_EOCDR_HEADER_SIZE;
 
     do_check_eq(size, tmpFile.fileSize);
 
     // Test the stored data with the zipreader
     var zipR = new ZipReader(tmpFile);
 
     for (var i = 0; i < TESTS.length; i++) {
-      do_check_true(zipR.hasEntry(TESTS[i].name));
+      var source = do_get_file(DATA_DIR + TESTS[i].name);
+      for (let method in methods) {
+        var entryName = method + "/" + TESTS[i].name;
+        do_check_true(zipR.hasEntry(entryName));
 
-      var source = do_get_file(DATA_DIR + TESTS[i].name);
-      var entry = zipR.getEntry(TESTS[i].name);
-      do_check_eq(entry.realSize, TESTS[i].size);
-      do_check_eq(entry.size, TESTS[i].size);
-      do_check_eq(entry.CRC32, TESTS[i].crc);
-      do_check_eq(Math.floor(entry.lastModifiedTime / PR_USEC_PER_SEC),
-                  Math.floor(source.lastModifiedTime / PR_MSEC_PER_SEC));
+        var entry = zipR.getEntry(entryName);
+        do_check_eq(entry.realSize, TESTS[i].size);
+        do_check_eq(entry.size, TESTS[i].size);
+        do_check_eq(entry.CRC32, TESTS[i].crc);
+        do_check_eq(Math.floor(entry.lastModifiedTime / PR_USEC_PER_SEC),
+                    Math.floor(source.lastModifiedTime / PR_MSEC_PER_SEC));
 
-      zipR.test(TESTS[i].name);
+        zipR.test(entryName);
+      }
     }
 
     zipR.close();
     do_test_finished();
   }
 };
 
+var methods = {
+  file: function method_file(entry, source)
+  {
+    zipW.addEntryFile(entry, Ci.nsIZipWriter.COMPRESSION_NONE, source,
+                      true);
+  },
+  channel: function method_channel(entry, source)
+  {
+    zipW.addEntryChannel(entry, source.lastModifiedTime * PR_MSEC_PER_SEC,
+                         Ci.nsIZipWriter.COMPRESSION_NONE,
+                         ioSvc.newChannelFromURI(ioSvc.newFileURI(source)), true);
+  },
+  stream: function method_stream(entry, source)
+  {
+    zipW.addEntryStream(entry, source.lastModifiedTime * PR_MSEC_PER_SEC,
+                        Ci.nsIZipWriter.COMPRESSION_NONE,
+                        ioSvc.newChannelFromURI(ioSvc.newFileURI(source)).open(), true);
+  }
+}
+
 function run_test()
 {
   zipW.open(tmpFile, PR_RDWR | PR_CREATE_FILE | PR_TRUNCATE);
 
   for (var i = 0; i < TESTS.length; i++) {
     var source = do_get_file(DATA_DIR+TESTS[i].name);
-    zipW.addEntryFile(TESTS[i].name, Ci.nsIZipWriter.COMPRESSION_NONE, source,
-                      true);
-    size += ZIP_FILE_HEADER_SIZE + ZIP_CDS_HEADER_SIZE +
-            (ZIP_EXTENDED_TIMESTAMP_SIZE * 2) +
-            (TESTS[i].name.length*2) + TESTS[i].size;
+    for (let method in methods) {
+      var entry = method + "/" + TESTS[i].name;
+      methods[method](entry, source);
+      size += ZIP_FILE_HEADER_SIZE + ZIP_CDS_HEADER_SIZE +
+              (ZIP_EXTENDED_TIMESTAMP_SIZE * 2) +
+              (entry.length*2) + TESTS[i].size;
+    }
   }
   do_test_pending();
   zipW.processQueue(observer, null);
   do_check_true(zipW.inQueue);
 }