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.

Sign up for GitHub

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

Jump to bottom

Encode() produces wrong value when low half of UUID is zero #21

Closed
cpick opened this issue May 14, 2021 · 0 comments · Fixed by #22
Closed

Encode() produces wrong value when low half of UUID is zero #21

cpick opened this issue May 14, 2021 · 0 comments · Fixed by #22

Comments

@cpick
Copy link

cpick commented May 14, 2021

base57.numToString() uses number.Uint64() which is undefined if number
"cannot be represented in a uint64" which is the case since it starts off containing a 128 bit integer.

This causes incorrect encoding of any UUID whose low half bits are all zero (along with any other cases where those bits were zero after some number of divisions by b.alphabet.Length()).

00000000-0000-0000-8000-000000000000 encodes to "z7C8BNkRBVT22", but 00000000-0000-0001-0000-000000000000 erroneously encodes to "2222222222222" which clashes with the encoding of 00000000-0000-0000-0000-000000000000 as opposed to the expected "yDNELiVqLxt22".
cpick pushed a commit to cpick/shortuuid that referenced this issue May 14, 2021
Fix Encode() when low half of UUID is zero
2cfdf8c 
Previously, `base57.numToString()` would use `number.Uint64()` which
[is undefined](https://pkg.go.dev/math/big#Int.Uint64) if `number`
"cannot be represented in a uint64" which is the case since it starts
off containing a 128 bit integer.

This would cause incorrect encoding of any UUID whose low half bits were
all zero (along with any other cases where those bits were zero after
some number of divisions by `b.alphabet.Length()`).

The first added test shows an edge case that previously passed, the
second an edge case that previously failed.

Switch to using `big.Int.Cmp()` which works with any value.

Fixes lithammer#21
cpick pushed a commit to cpick/shortuuid that referenced this issue May 14, 2021
Fix Encode() when low half of UUID is zero
36add62 
Previously, `base57.numToString()` would use `number.Uint64()` which
[is undefined](https://pkg.go.dev/math/big#Int.Uint64) if `number`
"cannot be represented in a uint64" which is the case since it starts
off containing a 128 bit integer.

This would cause incorrect encoding of any UUID whose low half bits were
all zero (along with any other cases where those bits were zero after
some number of divisions by `b.alphabet.Length()`).

The first added test shows an edge case that previously passed, the
second an edge case that previously failed.

Switch to using `big.Int.Cmp()` which works with any value.

Fixes lithammer#21
@cpick cpick mentioned this issue May 14, 2021
Merged
cpick pushed a commit to cpick/shortuuid that referenced this issue May 14, 2021
Fix Encode() when low half of UUID is zero
d7ed10b 
Previously, `base57.numToString()` would use `number.Uint64()` which
[is undefined](https://pkg.go.dev/math/big#Int.Uint64) if `number`
"cannot be represented in a uint64" which is the case since it starts
off containing a 128 bit integer.

This would cause incorrect encoding of any UUID whose low half bits were
all zero (along with any other cases where those bits were zero after
some number of divisions by `b.alphabet.Length()`).

The first added test shows an edge case that previously passed, the
second an edge case that previously failed.

Switch to using `big.Int.Cmp()` which works with any value.

Fixes lithammer#21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Encode() when low half of UUID is zero cpick/shortuuid
1 participant
@cpick