msgpack: Never marshal integers as MessagePack float

Previously we were using MessagePack's float family for anything that
could convert losslessly to float64.

Unfortunately, cty's interpretation of numbers uses binary floating point
internally but decimal strings as the presentation, including when numbers
are converted to strings, and so we need to make some assumptions about
precision when we're choosing a string representation of a number, to
avoid confusing situations where slight inaccuracy in the binary
interpretation of a number presented in decimal would appear as unwanted
extra digits in the string representation.

Slight differences caused by precision handling are acceptable for the
fractional parts of numbers, but callers expect integers to be presented
exactly as originally given. Round-tripping a large integer through a
float64, even if the binary representation is exact, causes us to end up
with a rounded string representation after decoding, because the decoded
number has 52-bit precision instead of the 512 we use when dealing with
string representations of large integers.

Therefore we'll now use MessagePack float encodings only for numbers that
have a fractional component. If an integer cannot fit in any of
MessagePack's integer encodings then we'll encode it as a string containing
decimal digits instead, so that it'll decode back to a number with the
same precision and thus the same decimal string representation.

Author: liamcervante

CHANGELOG.md

@@ -1,5 +1,6 @@
 # 1.14.4 (Unreleased)
 
+* `msgpack`: Now uses string encoding instead of float encoding for a whole number that is too large to fit in any of MessagePack's integer types.
 
 # 1.14.3 (February 29, 2024)
 

cty/msgpack/marshal.go

@@ -89,7 +89,7 @@ func marshal(val cty.Value, ty cty.Type, path cty.Path, enc *msgpack.Encoder) er
 				bf := val.AsBigFloat()
 				if iv, acc := bf.Int64(); acc == big.Exact {
 					err = enc.EncodeInt(iv)
-				} else if fv, acc := bf.Float64(); acc == big.Exact {
+				} else if fv, acc := bf.Float64(); acc == big.Exact && !bf.IsInt() {
 					err = enc.EncodeFloat64(fv)
 				} else {
 					err = enc.EncodeString(bf.Text('f', -1))

cty/msgpack/roundtrip_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/convert"
 )
 
 func TestRoundTrip(t *testing.T) {
@@ -96,6 +97,10 @@ func TestRoundTrip(t *testing.T) {
 			cty.MustParseNumberVal("9223372036854775809"),
 			cty.Number,
 		},
+		{
+			cty.MustParseNumberVal("18446744073709551616"),
+			cty.Number,
+		},
 		{
 			awkwardFractionVal,
 			cty.Number,
@@ -363,6 +368,119 @@ func TestRoundTrip(t *testing.T) {
 	}
 }
 
+func TestRoundTrip_fromString(t *testing.T) {
+	tests := []struct {
+		Value string
+		Type  cty.Type
+	}{
+		{
+			"0",
+			cty.Number,
+		},
+		{
+			"1",
+			cty.Number,
+		},
+		{
+			"-1",
+			cty.Number,
+		},
+		{
+			"9223372036854775807",
+			cty.Number,
+		},
+		{
+			"9223372036854775808",
+			cty.Number,
+		},
+		{
+			"9223372036854775809",
+			cty.Number,
+		},
+		{
+			"18446744073709551616",
+			cty.Number,
+		},
+		{
+			"-9223372036854775807",
+			cty.Number,
+		},
+		{
+			"-9223372036854775808",
+			cty.Number,
+		},
+		{
+			"-9223372036854775809",
+			cty.Number,
+		},
+		{
+			"-18446744073709551616",
+			cty.Number,
+		},
+		{
+			"true",
+			cty.Bool,
+		},
+		{
+			"false",
+			cty.Bool,
+		},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%#v as %#v", test.Value, test.Type), func(t *testing.T) {
+			stringVal := cty.StringVal(test.Value)
+
+			original, err := convert.Convert(stringVal, test.Type)
+			if err != nil {
+				t.Fatalf("input type must be convertible from string: %s", err)
+			}
+
+			{
+				// We'll first make sure that the conversion works even without
+				// MessagePack involved, since otherwise we might falsely blame
+				// the MessagePack encoding for bugs in package convert.
+				stringGot, err := convert.Convert(original, cty.String)
+				if err != nil {
+					t.Fatalf("result must be convertible to string: %s", err)
+				}
+
+				if !stringGot.RawEquals(stringVal) {
+					t.Fatalf("value did not round-trip to string even without msgpack\ninput:  %#v\nresult: %#v", test.Value, stringGot)
+				}
+			}
+
+			b, err := Marshal(original, test.Type)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			t.Logf("encoded as %x", b)
+
+			got, err := Unmarshal(b, test.Type)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !got.RawEquals(original) {
+				t.Errorf(
+					"value did not round-trip\ninput:  %#v\nresult: %#v",
+					test.Value, got,
+				)
+			}
+
+			stringGot, err := convert.Convert(got, cty.String)
+			if err != nil {
+				t.Fatalf("result must be convertible to string: %s", err)
+			}
+
+			if !stringGot.RawEquals(stringVal) {
+				t.Errorf("value did not round-trip to string\ninput:  %#v\nresult: %#v", test.Value, stringGot)
+			}
+
+		})
+	}
+}
+
 // Unknown values with very long string prefix refinements do not round-trip
 // losslessly. If the prefix is longer than 256 bytes it will be truncated to
 // a maximum of 256 bytes.