MAX_INT64 issue

[pldubouilh]

According to the bep_0044, it should be possible to set an integer to MAX_INT64, (i.e. 0x7fffffffffffffff). libtorrent successfully bencode this number, although node-bencode doesn't allow this and I receive this error :

WARNING: Possible data corruption detected with value 9223372036854776000: Bencoding only defines support for integers, value was converted to 0

Which is fairly normal as my number is too big.

At the moment I hacked the number encoding case to push a hand crafted buffer when I detect an overflow, but it's not pretty at all... I don't see any easy solution, apart from using a library dedicated to handling large numbers, like node-int64.

Any thoughts ?
Cheers !

[themasch]

From what I see is that we support the number range thats supported by JavaScript (from Number.MIN_SAFE_INTEGER to Number.MAX_SAFE_INTEGER). Number.MAX_VALUE can be bigger (like 1.8e308) but when you compare try "Number.MAX_SAFE_INTEGER + 1 === Number.MAX_SAFE_INTEGER + 2" it acutally evaluates to true because of how javascript stores numbers (see MDN).

While we could implement some magic to support Int64 objects and convert them to bencode integers, we would need to do the same while decoding. This would involve some checks on the string representation of the number to figure out if we need to return a Number or an Int64. Does Int64 behave exactly like a Number (except the supporting-bigger-ints side)? Because if it does not, our API would sometime return a Number and sometimes (when interger is to big) return something that behaves like a Number but not always which is a terrible library to have.

Also to consider:

• impact on performance (this is going to be a big slow-down!)
• first real dependency for node-bencode would be node-int64 then.

I'm not really sure if we want to do this.
We maybe should, because its a spec. But maybe we shouldn't because JS doesn't safely support numbers > 2^53-1.

@jhermsmeier, opinions?

[jhermsmeier]

I guess we could get away with numbers up to 2^53-1 with some bit-mangeling (I think we currently only support encoding numbers up to 2^32-1 and decoding safely up to 2^53-1 via parseInt(n,10)), but I don't think it would be beneficial to use node-int64 or the like as those do not return primitive values that can be worked with easily, thus significantly reducing the usability of this module (in most applications).

I don't see any easy solution, apart from using a library dedicated to handling large numbers, like node-int64.

As an intermediate solution, you could patch bencode.encode.number and bencode.decode.integer during runtime (after you loaded bencode) to support objects from node-int64.

I'm not really sure if we want to do this.
We maybe should, because its a spec.

I think we should bake it in some way or another – what do you think about being able to do something like require('bencode/int64') to manually opt into 64bit integer support – which would then have a different API regarding numbers for the "power-users"?

[pldubouilh]

Does Int64 behave exactly like a Number (except the supporting-bigger-ints side)?

Nope, node-int64 is not for arithmetic, and will not behave like Numbers. Other libraries like node-bignums can add support for usual arithmetic functions, but it's only partial (through .add , .div)...

I think we should bake it in some way or another – what do you think about being able to do something like require('bencode/int64') to manually opt into 64bit integer support

Absolutely, I'll have a look at doing that tomorrow - although I'm a bit concerned about the performance !

[pldubouilh]

Right, so I did a quick implementation using BigNumbers . It's available here. It's not the most elegant solution, but eh, it's doing its job : allowing large numbers to be encoded/decoded.

The performance are, well, debatable ; I have a slight improvement in decoding (not at each run), but a massive (half as fast) regression in encoding. Which is very annoying as 99% of the time it would be useless to have support for large numbers !

                  Integers
       1,433,768 op/s » Numbers decode
       1,456,189 op/s » BigNumber decode

                  Integers
         787,552 op/s » Numbers Encode
         360,592 op/s » BigNumber encode

[themasch]

So what we need is a file that contains this bignum implementation and monkey-patches bencode.encode, bencode.encode.number, and bencode.decode.integer when required, right?

So one could just

let bencode = require('bencode'), 
    idontcare = require('bencode/bignum')

or would it be better do directly import 'bencode/bignum' and that one returns a complete bencode implementation with fixes for bignums.
This would look more nicely in code but wouldn't support multiple extensions.

With the first way we could support future extensions by just including more of them (if its assured that they do not introduce collisions)

[pldubouilh]

Right, so here's a patch. It's simply patching the encode/decode handler to treat bignumbers.js objects properly. Also FYI, @solderzzc had a related issue on a software using node-bencode yesterday.

You can test it using the following code

var BigNumber = require( 'bignumber.js' )
var bencode = require('./bigencode.js')

var data = {
  string: 'Hello World',
  integer: new BigNumber('9223372036854775807'),
  dict: {
    key: 'This is a string within a dictionary'
  },
  list: [ 'string', 'tesst', new BigNumber('22'), {} ]
}

var encoded = bencode.encode(data)
var decoded = bencode.decode(encoded)

console.log(encoded.toString())
console.log('\n ++++++++++++ \n')
console.log(decoded)
debugger