Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DeflateCRC32Stream is broken with NodeJS 15.6.0 #31

Closed
xfournet opened this issue Jan 22, 2021 · 6 comments
Closed

DeflateCRC32Stream is broken with NodeJS 15.6.0 #31

xfournet opened this issue Jan 22, 2021 · 6 comments

Comments

@xfournet
Copy link
Contributor

Starting with NodeJS 15.6.0, calling end(chunk) on a DeflateCRC32Stream instance, the checksum is no more updated. This cause this bug archiverjs/node-archiver#491

const { DeflateCRC32Stream } = require('crc32-stream');

console.log('Node version:', process.version);
console.log();

const s = new DeflateCRC32Stream();
s.end(Buffer.from('hello world'));
console.log('crc32: ', s.hex());
console.log('rawSize: ', s.size(false));
Node version: v15.5.1

crc32:  0D4A1185
rawSize:  11
Node version: v15.6.0

crc32:  00000000
rawSize:  0
@Aaron1011
Copy link

Aaron1011 commented Jan 22, 2021

Looking at the changelog for Node v15.6.0, I noticed nodejs/node#36618, which seems potentially related.

@xfournet
Copy link
Contributor Author

Probably the end function was calling the write (overloaded in DeflateCRC32Stream) but now end is not calling this write function anymore causing the problem.

@Aaron1011
Copy link

Aaron1011 commented Jan 22, 2021

EDIT: @xfournet has the same idea in #31 (comment)

Minimized even further:

const {DeflateRaw} = require('zlib');

class MyStream extends DeflateRaw {
    constructor(options) {
        super(options);
    }

    write(chunk, enc, cb) {
        console.log("Write: " + chunk + " " + enc + " " + cb);
    }
}

console.log("Running");
const s = new MyStream();
s.end(Buffer.from('hello world'));

On Node v15.5.0, this prints:

Running
Write: hello world undefined undefined

On Node v15.6.0, this prints:

Running

@Aaron1011
Copy link

Opened nodejs/node#37027

@xfournet
Copy link
Contributor Author

In meantime I wrote this patch (a bit ugly) to use with patch-package. It works with NodeJS 15.5.1 & 15.6.0

File patches/crc32-stream+4.0.1.patch

diff --git a/node_modules/crc32-stream/lib/deflate-crc32-stream.js b/node_modules/crc32-stream/lib/deflate-crc32-stream.js
index ffe7661..65bf095 100644
--- a/node_modules/crc32-stream/lib/deflate-crc32-stream.js
+++ b/node_modules/crc32-stream/lib/deflate-crc32-stream.js
@@ -12,6 +12,15 @@ const {DeflateRaw} = require('zlib');
 
 const crc32 = require('crc-32');
 
+class TestDeflateRaw extends DeflateRaw {
+  write(chunk, enc, cb) {
+    this.writeCalled = true;
+  }
+}
+const testStream = new TestDeflateRaw();
+testStream.end(Buffer.from('testStream'));
+const processEndChunk = !testStream.writeCalled;
+
 class DeflateCRC32Stream extends DeflateRaw {
   constructor(options) {
     super(options);
@@ -40,6 +49,15 @@ class DeflateCRC32Stream extends DeflateRaw {
     return super.write(chunk, enc, cb);
   }
 
+  end(chunk, enc, cb) {
+    if (processEndChunk && chunk) {
+      this.checksum = crc32.buf(chunk, this.checksum) >>> 0;
+      this.rawSize += chunk.length;
+    }
+
+    return super.end(chunk, enc, cb);
+  }
+
   digest(encoding) {
     const checksum = Buffer.allocUnsafe(4);
     checksum.writeUInt32BE(this.checksum >>> 0, 0);

xfournet added a commit to xfournet/node-crc32-stream that referenced this issue Jan 28, 2021
@xfournet
Copy link
Contributor Author

Fix in PR #32

artem-karpenko pushed a commit to artem-karpenko/archiver-zip-encrypted that referenced this issue Jun 8, 2021
* Update `compress-commons` for Node 15 support

Right now we're producing ZIP files with incorrect CRC32s on Node 15+.
This has a few of effects:

- macOS Finder gets confused and doesn't detect the ZIP file as password
  protected. Instead, it unzips everything as a 0 byte file

- Running `unzip` from the command line succeeds but warns about
  incorrect CRC32

- Our unit tests (correctly) fail

This is ultimately due to archiverjs/node-crc32-stream#31

To pull in the fix for that we need `crc32-stream@4.0.2`. That requires
updating both `compress-commons` and `zip-stream` to their latest
versions.

The version bumps look scary here but they're mostly due to dropping
support for old Node versions. They now require Node 10+; this should be
fine considering our Travis config only tests 12+.

* Update `package-lock.json`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants