hildjj / node-cbor

Encode and decode CBOR documents, with both easy mode, streaming mode, and SAX-style evented mode.
MIT License
356 stars 73 forks source link

MAXINT64: BigInt('0xffffffffffffffff') crashes on React-Native #156

Closed vitorpamplona closed 2 years ago

vitorpamplona commented 2 years ago

The only way to use cbor in react-native is to comment this line:

https://github.com/hildjj/node-cbor/blob/86696c16192b4823c5e90253a9989747ce4a3ae5/packages/cbor/lib/constants.js#L76

Issue happens on the current react-native demo as well: https://github.com/hildjj/node-cbor-expo-example

hildjj commented 2 years ago

Are BigInt's actually signed 64-bit integers in react native? What happens if you change that to 0x7fffffffffffffff?

hildjj commented 2 years ago

Also, what error are you getting?

vitorpamplona commented 2 years ago

Apologies, I should have explained better.

I am using big-integer ("big-integer": "^1.6.49") to bring BigInt to react-native since it's not fully supported. With this lib, the error is on the parseStringValue

Error: Invalid integer: 0xffffffffffffffff

If I comment this specific line, all BigInt operations work fine.

vitorpamplona commented 2 years ago

I am not sure why MAXINT32 passes but MAXINT64 doesn't

https://github.com/hildjj/node-cbor/blob/86696c16192b4823c5e90253a9989747ce4a3ae5/packages/cbor/lib/constants.js#L75

vitorpamplona commented 2 years ago

If I just run your demo (https://github.com/hildjj/node-cbor/issues/136#issuecomment-799790773), i get this:

ReferenceError: Can't find variable: BigInt

hildjj commented 2 years ago

That error doesn't make much sense. Either BigInt is defined, or it's not. Try console.log(globalThis.BigInt). You should get something like [Function: BigInt]

hildjj commented 2 years ago

Oh, I get it. bit-integer looks to see if the number fits in 32 bits using (effectively) Math.floor(n) === n. If not, it can't parse hex numbers. That's a bug in their library, since their regex doesn't match https://tc39.es/ecma262/multipage/abstract-operations.html#prod-StringNumericLiteral

hildjj commented 2 years ago

https://github.com/peterolson/BigInteger.js/issues/205 was closed wontfix. We may be at an impasse, because BigInt("ffffffffffffffff", 16) doesn't work.

hildjj commented 2 years ago

The quickest fix, assuming that big-integer refuses to fix this, is to write a wrapper function, and register that as BigInt in your bundler:

const bi = require('big-integer')

function myBigInt(value) {
  if (typeof value === 'string') {
    const match = value.match(/^0([xo])([0-9a-f]+)$/i)
    if (match) {
      return bi(match[2], match[1].toLowerCase() === 'x' ? 16 : 8)
    }
  }
  return bi(value)
}

console.log(myBigInt('0xffffffffffffffff'))
console.log(myBigInt('0o7777777777777777'))
console.log(myBigInt('10'))
vitorpamplona commented 2 years ago

The trick works! You can see it here: https://github.com/Path-Check/universal-verifier-app

hildjj commented 2 years ago

Glad to hear. Let me know if you run into any more issues.

hildjj commented 2 years ago

I've added a cbor-rn-prereqs package that should a) make this easier for others, and b) ensure the code we came up with is tested. I updated https://github.com/hildjj/node-cbor-expo-example to use it. See https://github.com/hildjj/node-cbor/tree/main/packages/cbor-rn-prereqs for details.