From 85bfc0ed5117cb316bbbcd9e65ecfcb95419be88 Mon Sep 17 00:00:00 2001 From: Thomas Pelletier Date: Tue, 31 May 2022 22:10:41 -0400 Subject: [PATCH] Encode: add bound check for uint64 > math.Int64 (#785) 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 --- marshaler.go | 13 ++++++++++++- marshaler_test.go | 13 +++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/marshaler.go b/marshaler.go index c14c1ce..a779be0 100644 --- a/marshaler.go +++ b/marshaler.go @@ -107,6 +107,13 @@ func (enc *Encoder) SetIndentTables(indent bool) *Encoder { // a newline character or a single quote. In that case they are emitted as // quoted strings. // +// Unsigned integers larger than math.MaxInt64 cannot be encoded. Doing so +// results in an error. This rule exists because 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. To encode such numbers, a +// solution is a custom type that implements encoding.TextMarshaler. +// // When encoding structs, fields are encoded in order of definition, with their // exact name. // @@ -303,7 +310,11 @@ func (enc *Encoder) encode(b []byte, ctx encoderCtx, v reflect.Value) ([]byte, e b = append(b, "false"...) } case reflect.Uint64, reflect.Uint32, reflect.Uint16, reflect.Uint8, reflect.Uint: - b = strconv.AppendUint(b, v.Uint(), 10) + x := v.Uint() + if x > uint64(math.MaxInt64) { + return nil, fmt.Errorf("toml: not encoding uint (%d) greater than max int64 (%d)", x, math.MaxInt64) + } + b = strconv.AppendUint(b, x, 10) case reflect.Int64, reflect.Int32, reflect.Int16, reflect.Int8, reflect.Int: b = strconv.AppendInt(b, v.Int(), 10) default: diff --git a/marshaler_test.go b/marshaler_test.go index 468546c..31b7512 100644 --- a/marshaler_test.go +++ b/marshaler_test.go @@ -1102,6 +1102,19 @@ func TestLocalTime(t *testing.T) { require.Equal(t, expected, string(out)) } +func TestMarshalUint64Overflow(t *testing.T) { + // The TOML spec only requires implementation to provide support for the + // int64 range. To avoid generating TOML documents that would not be + // supported by standard-compliant parsers, uint64 > max int64 cannot be + // marshaled. + x := map[string]interface{}{ + "foo": uint64(math.MaxInt64) + 1, + } + + _, err := toml.Marshal(x) + require.Error(t, err) +} + func ExampleMarshal() { type MyConfig struct { Version int