pierrec / node-lz4

LZ4 fast compression algorithm for NodeJS
MIT License
438 stars 98 forks source link

Tests no longer pass under Node 10 #62

Closed leafi closed 6 years ago

leafi commented 6 years ago

The Buffer readUInt32LE, readInt32LE, writeUInt32LE, writeInt32LE calls have changed (there's no more noAssert parameter) and how things are interpreted also seems to have changed.

The code that deals with xxhash now seems to compare signed & unsigned types, and write calls sometimes reject values (seemingly) for being outside the range of a 32-bit integer.

See: https://gist.github.com/leafi/be4fe7807577d170ef558dd0081a3091 for full log

11 passing (30ms) 3 failing

1) LZ4 checksum should encode/decode data: RangeError [ERR_OUT_OF_RANGE]: The value of "value" is out of range. It must be >= -2147483648 and <= 2147483647. Received 3675999523 at checkInt (internal/buffer.js:35:11) at writeU_Int32LE (internal/buffer.js:515:3) at Buffer.writeInt32LE (internal/buffer.js:684:10) at Encoder._flush (lib/encoder_stream.js:215:7) at Encoder.prefinish (_stream_transform.js:141:10) at prefinish (_stream_writable.js:643:14) at finishMaybe (_stream_writable.js:651:5) at endWritable (_stream_writable.js:662:3) at Encoder.Writable.end (_stream_writable.js:602:5) at Object.LZ4_compress [as encode] (lib/encoder.js:14:10) at Context. (test/checksum-test.js:12:20)

2) LZ4 checksum should encode/decode data: RangeError [ERR_OUT_OF_RANGE]: The value of "value" is out of range. It must be >= -2147483648 and <= 2147483647. Received 2319569705 at checkInt (internal/buffer.js:35:11) at writeU_Int32LE (internal/buffer.js:515:3) at Buffer.writeInt32LE (internal/buffer.js:684:10) at Encoder._flush (lib/encoder_stream.js:215:7) at Encoder.prefinish (_stream_transform.js:141:10) at prefinish (_stream_writable.js:643:14) at finishMaybe (_stream_writable.js:651:5) at endWritable (_stream_writable.js:662:3) at Encoder.Writable.end (_stream_writable.js:602:5) at Object.LZ4_compress [as encode] (lib/encoder.js:14:10) at Context. (test/checksum-test.js:21:20)

3) LZ4 checksum should encode/decode data: RangeError [ERR_OUT_OF_RANGE]: The value of "value" is out of range. It must be >= -2147483648 and <= 2147483647. Received 3729892867 at checkInt (internal/buffer.js:35:11) at writeU_Int32LE (internal/buffer.js:515:3) at Buffer.writeInt32LE (internal/buffer.js:684:10) at Encoder._flush (lib/encoder_stream.js:215:7) at Encoder.prefinish (_stream_transform.js:141:10) at prefinish (_stream_writable.js:643:14) at finishMaybe (_stream_writable.js:651:5) at endWritable (_stream_writable.js:662:3) at Encoder.Writable.end (_stream_writable.js:602:5) at Object.LZ4_compress [as encode] (lib/encoder.js:14:10) at Context. (test/checksum-test.js:31:20)

ChALkeR commented 6 years ago

@leafi @pierrec Shouldn't that be writeUInt32LE? I.e. unsigned, not signed int.

ChALkeR commented 6 years ago

Changing that one line from writeInt32LE to writeUInt32LE makes tests pass.

https://github.com/pierrec/node-lz4/blob/4c8cc28c2482c86c29222df34ef8206acd79f6c4/lib/encoder_stream.js#L215

ChALkeR commented 6 years ago

Fix PR: https://github.com/pierrec/node-lz4/pull/65