search

ashi009 / node-fast-crc32c

node.js CRC-32C algorithm with hardware acceleration and software fallback.
MIT License
29 stars 20 forks source link

Breaking Change #19

Closed sammaxwellxyz closed 5 years ago

sammaxwellxyz commented 5 years ago

Hi,

We've received the following error

TypeError: Initial CRC-32C is not an integer!

We believe it to be due to your package as it wasn't an issue before your last release. We're not using node 12 but are using the LTS v10.15.2. Sorry I can't be more constructive or resolve the issue

f5io commented 5 years ago

This is specifically related to this dependency tree:

@google-cloud/storage@2.5.0
|- hash-stream-validation@0.2.1
  |- fast-crc32c [optional dependency]

hash-stream-validation - You can see in the README here it's an implicit optional dependency that will attempt to be required.

We've had this issue because elsewhere in our system we have:

snappystream@1.4.0
|- fast-crc32c@^1.0.4

FYI, we've rectified our issue by explicitly hardening on fast-crc32c@1.0.4 in our package.json

ashi009 commented 5 years ago

For now, I'll unpublish v1.0.6 until this is resolved.

ashi009 commented 5 years ago

@f5io and @sammaxwellxyz fast-crc32c@1.0.6 is unpublished, please reinstall and the issue should go away.

The error message is from the sse4_crc32, which suggests that hash-stream-validation passed an invalid initial value to the hasher. It seems that a bug in the consumer end being revealed due to the last change, which enables sse4_crc32 on node 12. sse4_crc32 is written in C++ and it checks the type before casting it to uint32, which is much stricter compared to the JS fallback.

ashi009 commented 5 years ago

Yep, https://github.com/stephenplusplus/hash-stream-validation/blob/master/index.js#L23 -- an undefined is passed at the initial round.

stephenplusplus commented 5 years ago

Thanks for chasing that down! I believe I have released the fix here, which is backwards compatible with the last release of fast-crc32c: https://github.com/stephenplusplus/hash-stream-validation/pull/8

f5io commented 5 years ago

Thanks for this @stephenplusplus and @ashi009, the quick response is much appreciated :)

ashi009 commented 5 years ago

The next release will be troublesome. As anyone who's using hash-stream-validation < 0.2.0 will see breakage as it will install v1.0.6.

@stephenplusplus would you mind marking hash-stream-validation@<0.2.0 as deprecated with a link pointing to here or your PR?

stephenplusplus commented 5 years ago

Nice catch 👍. I published a new version in the 0.1 line that includes the fix above.