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.

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

long encoding/decoding is not reversible for some large but safe js ints #455

Closed
NickKelly1 opened this issue Mar 8, 2024 · 1 comment · Fixed by #457
Closed

long encoding/decoding is not reversible for some large but safe js ints #455

NickKelly1 opened this issue Mar 8, 2024 · 1 comment · Fixed by #457

Comments

@NickKelly1
Copy link

NickKelly1 commented Mar 8, 2024

Large integers within the safe JS integer range can get pushed above the safe JS integer range during zigzag encoding/decoding causing a loss of precision (the zigzag encoded number has a float64 collision so can't be decoded back into the original number).

unsafe zigzag encoding calculation in Tap.ptototype.writeLong

Since these source numbers are in the safe JS integer range they pass the bounds check for long types

isSafeLong checks range [Number.MIN_SAFE_INTEGER + 1, Number.MAX_SAFE_INTEGER - 1]

LongType.prototype._check bounds check using isSafeLong

LongType.prototype._write bounds check using isSafeLong

Here are some examples:

import avsc from 'avsc'

const long = avsc.Type.forSchema({ type: 'long' })
console.log(`-4503599627370497: ${long.fromBuffer(long.toBuffer(-4503599627370497))}`)
console.log(`-4503599627370499: ${long.fromBuffer(long.toBuffer(-4503599627370499))}`)
console.log(`-4503599627370501: ${long.fromBuffer(long.toBuffer(-4503599627370501))}`)
console.log(`-4503599627370503: ${long.fromBuffer(long.toBuffer(-4503599627370503))}`)
console.log(`-4503599627370505: ${long.fromBuffer(long.toBuffer(-4503599627370505))}`)
console.log(`-4503599627370507: ${long.fromBuffer(long.toBuffer(-4503599627370507))}`)
console.log(`-4503599627370509: ${long.fromBuffer(long.toBuffer(-4503599627370509))}`)
console.log(`-4503599627370511: ${long.fromBuffer(long.toBuffer(-4503599627370511))}`)
// -4503599627370497: 4503599627370496
// -4503599627370499: 4503599627370498
// -4503599627370501: 4503599627370500
// -4503599627370503: 4503599627370502
// -4503599627370505: 4503599627370504
// -4503599627370507: 4503599627370506
// -4503599627370509: 4503599627370508
// -4503599627370511: 4503599627370510
@mtth
Copy link
Owner

mtth commented Mar 25, 2024

Thanks for reporting @NickKelly1 - great find. The bounds should be tighter.

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 a pull request may close this issue.

2 participants