webtorrent / node-bencode

bencode de/encoder for nodejs
MIT License
166 stars 36 forks source link

Fix encoding of large numbers #92

Closed jhermsmeier closed 1 year ago

jhermsmeier commented 3 years ago

So, this is a fun one I noticed yesterday: Number.toString() can not be relied upon to give you all digits of an integer:

// Number.toString()
> 340282366920938463463374607431768211456..toString()
'3.402823669209385e+38'

// Number.toPrecision() doesn't really help either
> 340282366920938463463374607431768211456..toPrecision(100)
'340282366920938463463374607431768211456.0000000000000000000000000000000000000000000000000000000000000'

// Neither does Number.toFixed()
> 340282366920938463463374607431768211456..toFixed(0)
'3.402823669209385e+38'

// Oh, look
> BigInt(340282366920938463463374607431768211456).toString()
'340282366920938463463374607431768211456'

While this is reasonably unlikely to happen in real-world usage as Number.toString() starts returning exponential notation at 270, it still could occur, and considering this might be run on other engines than V8 which might have different limits, this could potentially occur sooner.

Unfortunately the only fix I found for this is going through BigInt() – I guess we could hand-roll something, but BigInt() seems to be the reliable thing to go for here – what do you think?

themasch commented 3 years ago

I think this needs a new merge from master (with conflict solving in encode.js) and we need to remove the float test since this breaks with bigint.

ThaUnknown commented 1 year ago

superseeded by https://github.com/webtorrent/node-bencode/pull/150