Skip to content
This repository was archived by the owner on Dec 3, 2022. It is now read-only.

Conversation

@jridgewell
Copy link
Contributor

There were overflow cases hidden in the encode and decode:

  1. Encoding
    • Any number with the 31st bit set would be incorrect, because it used >> instead of >>>. So the 31st bit would be set to the 32nd (sign bit, making it negative). Then when we try to encode the number, we would that it's num < 0 after the first iteration.
  2. Decoding
    • The encoded representation of -2147483648 (smallest 32bit signed int) would return -0
    • Any number with the 31st bit set would return the opposite sign, because it used >> instead of >>>.
There were overflow cases hidden in the encode and decode: 1. Encoding - Any number with the 31st bit set would be incorrect, because it used `>>` instead of `>>>`. So the 31st bit would be set to the 32nd (sign bit, making it negative). Then when we try to encode the number, we would that it's `num < 0` after the first iteration. 2. Decoding - The encoded representation of -2147483648 (smallest 32bit signed int) would return -0 - Any number with the 31st bit set would return the opposite sign, because it used `>>` instead of `>>>`.
@Rich-Harris Rich-Harris merged commit 688753e into Rich-Harris:master Jul 3, 2019
@jridgewell jridgewell deleted the i32 branch July 3, 2019 20:36
@jridgewell
Copy link
Contributor Author

The test failure here is due to assert.deepEqual changing behavior in node 12, not because of the PR.

@Rich-Harris
Copy link
Owner

Yeah, looking at that right now

@Rich-Harris
Copy link
Owner

Released 1.4.5. Thank you

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants