Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tomjakubowski opened this issue Feb 5, 2016 · 5 comments

Comments

@tomjakubowski
Copy link

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
Copy link
Author

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

@BinaryMuse
Copy link
Owner

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
Copy link
Owner

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.

@mojombo
Copy link

mojombo commented Feb 22, 2016

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.

@tomocrafter
Copy link

tomocrafter commented Jun 3, 2023

Hi, I also encountered this issue while parsing the config that contains snowflake id which is bigger than number maximum normally. I think we can wrap the number when above javascript number limitation with BigInt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants