peterolson / BigInteger.js

An arbitrary length integer library for Javascript
The Unlicense
1.12k stars 187 forks source link

bigInt(long hexstring) throws an "Invalid integer error" #205

Closed wanseob closed 4 years ago

wanseob commented 4 years ago

Bug reproduction

$ node
> bigInt = require('big-integer')
> bigInt('0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')
Thrown:
Error: Invalid integer: 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
    at parseStringValue (~/node_modules/big-integer/BigInteger.js:1382:29)
    at parseValue (~/node_modules/big-integer/BigInteger.js:1413:20)
    at Integer (~/node_modules/big-integer/BigInteger.js:15:16)
wanseob commented 4 years ago

Current node version is v12.16.2

peterolson commented 4 years ago

bigInt doesn't use the 0x prefix for hex numbers. You can pass 16 as a second argument to specify that the input is hex:

bigInt('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 16)
wanseob commented 4 years ago

Thank you it helped. FYI, it also worked for a short length '0x' prefixed hex like bigInt('0xaaaa') thou I was confused.

wanseob commented 4 years ago

Tested again, but it still has a problem. Tested with node v10 & v12 Here's the log.

> bigInt('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')
Thrown:
Error: Invalid integer: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
    at parseStringValue (/home/wanseob/Projects/wanseob/zkopru/node_modules/big-integer/BigInteger.js:1382:29)
    at parseValue (/home/wanseob/Projects/wanseob/zkopru/node_modules/big-integer/BigInteger.js:1413:20)
    at Integer (/home/wanseob/Projects/wanseob/zkopru/node_modules/big-integer/BigInteger.js:15:16)
peterolson commented 4 years ago

Did you forget the second argument? You didn't pass in 16

wanseob commented 4 years ago

Oo yes you're right, apologize my silly mistake

gre commented 3 years ago

the problem is that it seems to work in the V8 implementation so you might want to reconsider supporting the '0x...' syntax. this lib can't act as polyfill because of this.

(left is node, right is this library)

Capture d’écran 2020-12-10 à 21 12 34

or maybe don't state in the Readme it is a polyfill.

peterolson commented 3 years ago

There was no intention to the exact API of native BigInt. This library was created before native BigInt was proposed, and contains features that native BigInt does not have, so doing so would introduce breaking changes.

What the readme means is that this library will work as expected whether or not the environment it runs on supports native BigInt. If there is no support, it will use an array-of-numbers implementation. If BigInt is supported, then this library will take advantage of it.