Skip to content

use crc-32 rather than crc module#28

Merged
ctalkington merged 1 commit into
archiverjs:masterfrom
DonBrinn:Issue27/use-crc-32
Nov 19, 2020
Merged

use crc-32 rather than crc module#28
ctalkington merged 1 commit into
archiverjs:masterfrom
DonBrinn:Issue27/use-crc-32

Conversation

@DonBrinn

Copy link
Copy Markdown
Contributor
  • Fixes Issue 27.
  • The crc module has a defect where it computes an incorrect checksum if the seed checksum value is 0. 0 is a legitimate checksum value, but that module handles it incorrectly.
  • This change switches to using crc-32 instead.
  • New regression unit tests are added.

Comment thread test/checksum.js
const checksum = new CRC32Stream();
const deadend = new DeadEndStream();

const expectedChecksumValue = crc32.buf([157, 10, 217, 109, 100, 200, 300]) >>> 0;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value here is the same whether using the crc module or crc-32 module.

Comment thread test/checksum.js
const expectedChecksumValue = crc32.buf([157, 10, 217, 109, 100, 200, 300]) >>> 0;

checksum.on('end', function() {
assert.equal(checksum.digest().readUInt32BE(0), expectedChecksumValue);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails if the product code is using the crc module.

@ctalkington ctalkington merged commit c9bd85a into archiverjs:master Nov 19, 2020
@ctalkington ctalkington changed the title Issue 27: use crc-32 rather than crc because it works when seed=0 use crc-32 rather than crc module ( Nov 19, 2020
@ctalkington ctalkington changed the title use crc-32 rather than crc module ( use crc-32 rather than crc module Nov 19, 2020
@alexgorbatchev

Copy link
Copy Markdown

Hi @DonBrinn, I believe this is the expected behavior. The default initial value for CRC32 is 0xFFFFFFFF, which is -1. This could be verified over at http://www.sunshine2k.de/coding/javascript/crc/crc_js.html for example. CRC32 of helloworld is 0xF9EB20AD with default CRC32 Initial Value of 0xFFFFFFFF. Changing initial value to 0 produces 0xE59EB724.

I've also verified this with Ruby:

require 'zlib'

puts("no initial value   : #{(Zlib::crc32('helloworld')).to_s(16)}")
puts("with initial value : #{Zlib::crc32('world', Zlib::crc32('hello')).to_s(16)}")
>
no initial value   : f9eb20ad
with initial value : f9eb20ad

The same output is produced by node-crc CRC32 function:

const together = crc32(Buffer.from('helloworld'));
let separately = crc32(Buffer.from('hello'));
separately = crc32(Buffer.from('world'), separately);
expect(separately).to.equal(together);

console.log({
  crc: crc32.model,
  together: together.toString(16),
  separately: separately.toString(16),
});
>
{ crc: 'crc-32', together: 'f9eb20ad', separately: 'f9eb20ad' }

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

Successfully merging this pull request may close these issues.

invalid checksum is possible if bytes are written separately

3 participants