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.

Sign up for GitHub

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

Jump to bottom

uint64 range limited by int64 #781

Closed
jmank88 opened this issue May 22, 2022 · 2 comments
Closed

uint64 range limited by int64 #781

jmank88 opened this issue May 22, 2022 · 2 comments
Labels
bug Issues describing a bug in go-toml.

Comments

@jmank88
Copy link
Contributor

jmank88 commented May 22, 2022

Describe the bug
Attempting to unmarshal a value a larger than math.MaxInt64 results in a value out of range error from strconv.ParseInt, even when the target field is a uint64.

To Reproduce
https://go.dev/play/p/L_zgsE-vlAl

Expected behavior
I expected the full range of uint64 values to be supported.

Versions

  • go-toml: v2.0.1
  • go: 1.18
  • operating system: Linux
@jmank88 jmank88 mentioned this issue May 22, 2022
Closed
@pelletier pelletier added the bug Issues describing a bug in go-toml. label May 23, 2022
@pelletier
Copy link
Owner

Thank you for reporting this!

jmank88 added a commit to smartcontractkit/go-toml that referenced this issue May 23, 2022
@jmank88
support decoding full uint64 range - fixes pelletier#781
e098af8
pelletier added a commit that referenced this issue Jun 1, 2022
@pelletier
Encode: add bound check for uint64 > math.Int64
0da4333 
As brought up on #782, there is an asymetry between numbers go-toml can encode
and decode. Specifically, unsigned numbers larger than `math.Int64` could be
encoded but not decoded.

We considered two options: allow decoding of those numbers, or prevent those
numbers to be decoded.

Ultimately we decided to narrow the range of numbers to be encoded. The TOML
specification only requires parsers to support at least the 64 bits integer
range. Allowing larger numbers would create non-standard TOML documents, which
may not be readable (at best) by other implementations. It is a safer default to
generate documents valid by default. People who wish to deal with larger numbers
can provide a custom type implementing `encoding.TextMarshaler`.

Refs #781
@pelletier pelletier mentioned this issue Jun 1, 2022
Merged
pelletier added a commit that referenced this issue Jun 1, 2022
@pelletier
Encode: add bound check for uint64 > math.Int64 (#785)
85bfc0e 
As brought up on #782, there is an asymetry between numbers go-toml can encode
and decode. Specifically, unsigned numbers larger than `math.Int64` could be
encoded but not decoded.

We considered two options: allow decoding of those numbers, or prevent those
numbers to be decoded.

Ultimately we decided to narrow the range of numbers to be encoded. The TOML
specification only requires parsers to support at least the 64 bits integer
range. Allowing larger numbers would create non-standard TOML documents, which
may not be readable (at best) by other implementations. It is a safer default to
generate documents valid by default. People who wish to deal with larger numbers
can provide a custom type implementing `encoding.TextMarshaler`.

Refs #781
@pelletier
Copy link
Owner

Merged 85bfc0e. It prevents the asymmetry problem you pointed out by preventing uint64 > maxInt64 to be encoded.

Thank you again for reporting since and driving the discussion to improve the library!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Issues describing a bug in go-toml.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pelletier @jmank88