toml-node icon indicating copy to clipboard operation
toml-node copied to clipboard

TOML allows large-magnitude integers that can't be represented exactly as JS numbers

Open tomjakubowski opened this issue 8 years ago • 5 comments

The TOML spec says that numbers can lie in the range of 64-bit signed integers. This presents a problem for JavaScript implementations of TOML, since the entire range of 64-bit signed integers can't be represented exactly by JavaScript. In particular, only the closed range of integers [Number.MIN_SAFE_INTEGER, Number.MAX_SAFE_INTEGER] can be represented exactly by the JavaScript number primitive.

To demonstrate that this is a problem for toml-node, check this out:

> require('toml').parse('max_safe = 9007199254740991\n' +
    'max_safe_plus_two = 9007199254740993')
{ max_safe: 9007199254740991,
  max_safe_plus_two: 9007199254740992 }

There are a few ways to deal with this problem (parse large ints as strings, depend on a BigInteger library for large ints, throw RangeErrors, among others). I don't have a strong opinion on which option is best, but I do think the library should address this issue in some way!

tomjakubowski avatar Feb 05 '16 01:02 tomjakubowski

Er, my original report didn't actually demonstrate the problem! Edited the issue to give a better example.

tomjakubowski avatar Feb 05 '16 11:02 tomjakubowski

Thanks for the report, @tomjakubowski. I agree that the library should do something other than just giving the wrong data back. I'll think on this, and am open to suggestions.

BinaryMuse avatar Feb 06 '16 08:02 BinaryMuse

After thinking about this, I think my inclination is to throw a RangeError if numbers fall outside the safe range. Number.MIN/MAX_SAFE_INTEGER can't be used in all browser environments so I think it'd need to be defined in the lib.

However, I'm on the fence about whether or not it'd be better to output some sort of warning instead, or allow an option for ignoring errors caused by numbers outside the safe range. I could imagine a scenario where consumption of the number may not be important, but other parts of the TOML file are.

@mojombo, I'm curious if you have any thoughts on how a parser would ideally behave here.

BinaryMuse avatar Feb 20 '16 17:02 BinaryMuse

In my opinion, this should throw an error and be well documented in the project as a limitation and deviation from the TOML spec. I'm guessing it will be rare that a developer doesn't experience the error during development. In my mind, silently returning an incorrect value is the worst kind of behavior and could lead to extremely frustrating bug hunting for some unlucky soul.

mojombo avatar Feb 22 '16 21:02 mojombo