From 97bafac0dea33a3f74db88ab54dea2937b9e8aef Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 2 Jun 2022 12:14:04 -0700 Subject: [PATCH] Remove all of the encoding/gob support code I originally introduced all of this here as a concession to trying to HashiCorp trying to get cty values to round-trip through the various very magical gob-based wire interfaces in Terraform v0.11 and earlier, back when they thought cty and what became "HCL 2" would just be a drop-in replacement for HCL 1 and the various different competing representations of dynamic values. However, in practice none of that ever actually worked out and instead Terraform now natively uses cty JSON/msgpack and other such mechanisms for its wire protocols, making this gob handling code redundant. This stuff caused various maintenance headaches and panic bugs which make them a burden to retain. Although this is technically a breaking change to the public cty API, in practice this stuff was only here to allow Terraform to use it _indirectly_ as a side-effect of passing values into the encoding/gob package, and so I'm treating it as if it had been an internal implementation detail even though in principle it could've been depended on externally. If anyone was doing that, I'm sorry; I suggest copying the relevant support code into your own package instead if you wish to continue using cty with gob. --- CHANGELOG.md | 9 +- cty/gob.go | 204 -------------------------------------- cty/gob_test.go | 69 ------------- cty/set/gob.go | 76 -------------- cty/set_internals_test.go | 17 +--- cty/types_to_register.go | 57 ----------- 6 files changed, 9 insertions(+), 423 deletions(-) delete mode 100644 cty/gob.go delete mode 100644 cty/gob_test.go delete mode 100644 cty/set/gob.go delete mode 100644 cty/types_to_register.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a0e480e4..23a91d1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ -# 1.10.1 (Unreleased) +# 1.11.0 (Unreleased) +## Upgrade Notes + +This release contains some changes to some aspects of the API that are either legacy or de-facto internal (from before the Go toolchain had an explicit idea of that). Any external module using these will experience these as breaking changes, but we know of no such caller and so are admitting these without a major release in the interests of not creating churn for users of the main API. + +* **`encoding/gob` support utilities removed**: we added these as a concession to HashiCorp who wanted to try to send `cty` values over some legacy protocols/formats used by legacy versions of HashiCorp Terraform. In the end those efforts were not successful for other reasons and so no Terraform release ever relied on this functionality. + + `encoding/gob` support has been burdensome due to how its unmarshaler interface is defined and so `cty` values and types are no longer automatically compatible with `encoding/gob`. Callers should instead use explicitly-implemented encodings, such as the built-in JSON and msgpack encodings or external libraries which use the public `cty` API to encode and decode. # 1.10.0 (November 2, 2021) diff --git a/cty/gob.go b/cty/gob.go deleted file mode 100644 index a0961b8a..00000000 --- a/cty/gob.go +++ /dev/null @@ -1,204 +0,0 @@ -package cty - -import ( - "bytes" - "encoding/gob" - "errors" - "fmt" - "math/big" - - "github.com/zclconf/go-cty/cty/set" -) - -// GobEncode is an implementation of the gob.GobEncoder interface, which -// allows Values to be included in structures encoded with encoding/gob. -// -// Currently it is not possible to represent values of capsule types in gob, -// because the types themselves cannot be represented. -func (val Value) GobEncode() ([]byte, error) { - if val.IsMarked() { - return nil, errors.New("value is marked") - } - - buf := &bytes.Buffer{} - enc := gob.NewEncoder(buf) - - gv := gobValue{ - Version: 0, - Ty: val.ty, - V: val.v, - } - - err := enc.Encode(gv) - if err != nil { - return nil, fmt.Errorf("error encoding cty.Value: %s", err) - } - - return buf.Bytes(), nil -} - -// GobDecode is an implementation of the gob.GobDecoder interface, which -// inverts the operation performed by GobEncode. See the documentation of -// GobEncode for considerations when using cty.Value instances with gob. -func (val *Value) GobDecode(buf []byte) error { - r := bytes.NewReader(buf) - dec := gob.NewDecoder(r) - - var gv gobValue - err := dec.Decode(&gv) - if err != nil { - return fmt.Errorf("error decoding cty.Value: %s", err) - } - if gv.Version != 0 { - return fmt.Errorf("unsupported cty.Value encoding version %d; only 0 is supported", gv.Version) - } - - // Because big.Float.GobEncode is implemented with a pointer reciever, - // gob encoding of an interface{} containing a *big.Float value does not - // round-trip correctly, emerging instead as a non-pointer big.Float. - // The rest of cty expects all number values to be represented by - // *big.Float, so we'll fix that up here. - gv.V = gobDecodeFixNumberPtr(gv.V, gv.Ty) - - val.ty = gv.Ty - val.v = gv.V - - return nil -} - -// GobEncode is an implementation of the gob.GobEncoder interface, which -// allows Types to be included in structures encoded with encoding/gob. -// -// Currently it is not possible to represent capsule types in gob. -func (t Type) GobEncode() ([]byte, error) { - buf := &bytes.Buffer{} - enc := gob.NewEncoder(buf) - - gt := gobType{ - Version: 0, - Impl: t.typeImpl, - } - - err := enc.Encode(gt) - if err != nil { - return nil, fmt.Errorf("error encoding cty.Type: %s", err) - } - - return buf.Bytes(), nil -} - -// GobDecode is an implementatino of the gob.GobDecoder interface, which -// reverses the encoding performed by GobEncode to allow types to be recovered -// from gob buffers. -func (t *Type) GobDecode(buf []byte) error { - r := bytes.NewReader(buf) - dec := gob.NewDecoder(r) - - var gt gobType - err := dec.Decode(>) - if err != nil { - return fmt.Errorf("error decoding cty.Type: %s", err) - } - if gt.Version != 0 { - return fmt.Errorf("unsupported cty.Type encoding version %d; only 0 is supported", gt.Version) - } - - t.typeImpl = gt.Impl - - return nil -} - -// Capsule types cannot currently be gob-encoded, because they rely on pointer -// equality and we have no way to recover the original pointer on decode. -func (t *capsuleType) GobEncode() ([]byte, error) { - return nil, fmt.Errorf("cannot gob-encode capsule type %q", t.FriendlyName(friendlyTypeName)) -} - -func (t *capsuleType) GobDecode() ([]byte, error) { - return nil, fmt.Errorf("cannot gob-decode capsule type %q", t.FriendlyName(friendlyTypeName)) -} - -type gobValue struct { - Version int - Ty Type - V interface{} -} - -type gobType struct { - Version int - Impl typeImpl -} - -type gobCapsuleTypeImpl struct { -} - -// goDecodeFixNumberPtr fixes an unfortunate quirk of round-tripping cty.Number -// values through gob: the big.Float.GobEncode method is implemented on a -// pointer receiver, and so it loses the "pointer-ness" of the value on -// encode, causing the values to emerge the other end as big.Float rather than -// *big.Float as we expect elsewhere in cty. -// -// The implementation of gobDecodeFixNumberPtr mutates the given raw value -// during its work, and may either return the same value mutated or a new -// value. Callers must no longer use whatever value they pass as "raw" after -// this function is called. -func gobDecodeFixNumberPtr(raw interface{}, ty Type) interface{} { - // Unfortunately we need to work recursively here because number values - // might be embedded in structural or collection type values. - - switch { - case ty.Equals(Number): - if bf, ok := raw.(big.Float); ok { - return &bf // wrap in pointer - } - case ty.IsMapType() && ty.ElementType().Equals(Number): - if m, ok := raw.(map[string]interface{}); ok { - for k, v := range m { - m[k] = gobDecodeFixNumberPtr(v, ty.ElementType()) - } - } - case ty.IsListType() && ty.ElementType().Equals(Number): - if s, ok := raw.([]interface{}); ok { - for i, v := range s { - s[i] = gobDecodeFixNumberPtr(v, ty.ElementType()) - } - } - case ty.IsSetType() && ty.ElementType().Equals(Number): - if s, ok := raw.(set.Set); ok { - newS := set.NewSet(s.Rules()) - for it := s.Iterator(); it.Next(); { - newV := gobDecodeFixNumberPtr(it.Value(), ty.ElementType()) - newS.Add(newV) - } - return newS - } - case ty.IsObjectType(): - if m, ok := raw.(map[string]interface{}); ok { - for k, v := range m { - aty := ty.AttributeType(k) - m[k] = gobDecodeFixNumberPtr(v, aty) - } - } - case ty.IsTupleType(): - if s, ok := raw.([]interface{}); ok { - for i, v := range s { - ety := ty.TupleElementType(i) - s[i] = gobDecodeFixNumberPtr(v, ety) - } - } - } - - return raw -} - -// gobDecodeFixNumberPtrVal is a helper wrapper around gobDecodeFixNumberPtr -// that works with already-constructed values. This is primarily for testing, -// to fix up intentionally-invalid number values for the parts of the test -// code that need them to be valid, such as calling GoString on them. -func gobDecodeFixNumberPtrVal(v Value) Value { - raw := gobDecodeFixNumberPtr(v.v, v.ty) - return Value{ - v: raw, - ty: v.ty, - } -} diff --git a/cty/gob_test.go b/cty/gob_test.go deleted file mode 100644 index 3e783067..00000000 --- a/cty/gob_test.go +++ /dev/null @@ -1,69 +0,0 @@ -package cty - -import ( - "bytes" - "testing" - - "encoding/gob" -) - -func TestGobabilty(t *testing.T) { - tests := []Value{ - StringVal("hi"), - True, - NumberIntVal(1), - NumberFloatVal(96.5), - ListVal([]Value{True}), - MapVal(map[string]Value{"true": True}), - SetVal([]Value{True}), - TupleVal([]Value{True}), - ObjectVal(map[string]Value{"true": True}), - - // Numbers are particularly tricky because big.Float.GobEncode is - // implemented as a pointer method and thus big floats lose their - // "pointerness" during gob round-trip. For that reason, we're testing - // all of the containers with nested numbers inside to make sure that - // our fixup step is working correctly for all of them. - ListVal([]Value{NumberIntVal(1)}), - MapVal(map[string]Value{ - "num": NumberIntVal(1), - }), - SetVal([]Value{NumberIntVal(1)}), - TupleVal([]Value{NumberIntVal(1)}), - ObjectVal(map[string]Value{ - "num": NumberIntVal(1), - }), - } - - for _, testValue := range tests { - t.Run(testValue.GoString(), func(t *testing.T) { - tv := testGob{ - testValue, - } - - buf := &bytes.Buffer{} - enc := gob.NewEncoder(buf) - - err := enc.Encode(tv) - if err != nil { - t.Fatalf("gob encode error: %s", err) - } - - var ov testGob - - dec := gob.NewDecoder(buf) - err = dec.Decode(&ov) - if err != nil { - t.Fatalf("gob decode error: %s", err) - } - - if !ov.Value.RawEquals(tv.Value) { - t.Errorf("value did not survive gobbing\ninput: %#v\noutput: %#v", tv, ov) - } - }) - } -} - -type testGob struct { - Value Value -} diff --git a/cty/set/gob.go b/cty/set/gob.go deleted file mode 100644 index da2978f6..00000000 --- a/cty/set/gob.go +++ /dev/null @@ -1,76 +0,0 @@ -package set - -import ( - "bytes" - "encoding/gob" - "fmt" -) - -// GobEncode is an implementation of the interface gob.GobEncoder, allowing -// sets to be included in structures encoded via gob. -// -// The set rules are included in the serialized value, so the caller must -// register its concrete rules type with gob.Register before using a -// set in a gob, and possibly also implement GobEncode/GobDecode to customize -// how any parameters are persisted. -// -// The set elements are also included, so if they are of non-primitive types -// they too must be registered with gob. -// -// If the produced gob values will persist for a long time, the caller must -// ensure compatibility of the rules implementation. In particular, if the -// definition of element equivalence changes between encoding and decoding -// then two distinct stored elements may be considered equivalent on decoding, -// causing the recovered set to have fewer elements than when it was stored. -func (s Set) GobEncode() ([]byte, error) { - gs := gobSet{ - Version: 0, - Rules: s.rules, - Values: s.Values(), - } - - buf := &bytes.Buffer{} - enc := gob.NewEncoder(buf) - err := enc.Encode(gs) - if err != nil { - return nil, fmt.Errorf("error encoding set.Set: %s", err) - } - - return buf.Bytes(), nil -} - -// GobDecode is the opposite of GobEncode. See GobEncode for information -// on the requirements for and caveats of including set values in gobs. -func (s *Set) GobDecode(buf []byte) error { - r := bytes.NewReader(buf) - dec := gob.NewDecoder(r) - - var gs gobSet - err := dec.Decode(&gs) - if err != nil { - return fmt.Errorf("error decoding set.Set: %s", err) - } - if gs.Version != 0 { - return fmt.Errorf("unsupported set.Set encoding version %d; need 0", gs.Version) - } - - victim := NewSetFromSlice(gs.Rules, gs.Values) - s.vals = victim.vals - s.rules = victim.rules - return nil -} - -type gobSet struct { - Version int - Rules Rules - - // The bucket-based representation is for efficient in-memory access, but - // for serialization it's enough to just retain the values themselves, - // which we can re-bucket using the rules (which may have changed!) when - // we re-inflate. - Values []interface{} -} - -func init() { - gob.Register([]interface{}(nil)) -} diff --git a/cty/set_internals_test.go b/cty/set_internals_test.go index 269e69c6..2f65f26c 100644 --- a/cty/set_internals_test.go +++ b/cty/set_internals_test.go @@ -44,21 +44,6 @@ func TestSetHashBytes(t *testing.T) { "12", nil, }, - { - // This weird case is an intentionally-invalid number value that - // mimics the incorrect result of a gob round-trip of a cty.Number - // value. For more information, see the function - // gobDecodeFixNumberPtr. Unfortunately the set internals need to - // be tolerant of this situation because gob-decoding a set - // causes this situation to arise before we have had an opportunity - // to run gobDecodeFixNumberPtr yet. - Value{ - ty: Number, - v: *big.NewFloat(13), - }, - "13", - nil, - }, { StringVal(""), `""`, @@ -178,7 +163,7 @@ func TestSetHashBytes(t *testing.T) { } for _, test := range tests { - t.Run(gobDecodeFixNumberPtrVal(test.value).GoString(), func(t *testing.T) { + t.Run(test.value.GoString(), func(t *testing.T) { gotRaw, gotMarks := makeSetHashBytes(test.value) got := string(gotRaw) if got != test.want { diff --git a/cty/types_to_register.go b/cty/types_to_register.go deleted file mode 100644 index e1e220aa..00000000 --- a/cty/types_to_register.go +++ /dev/null @@ -1,57 +0,0 @@ -package cty - -import ( - "encoding/gob" - "fmt" - "math/big" - "strings" - - "github.com/zclconf/go-cty/cty/set" -) - -// InternalTypesToRegister is a slice of values that covers all of the -// internal types used in the representation of cty.Type and cty.Value -// across all cty Types. -// -// This is intended to be used to register these types with encoding -// packages that require registration of types used in interfaces, such as -// encoding/gob, thus allowing cty types and values to be included in streams -// created from those packages. However, registering with gob is not necessary -// since that is done automatically as a side-effect of importing this package. -// -// Callers should not do anything with the values here except pass them on -// verbatim to a registration function. -// -// If the calling application uses Capsule types that wrap local structs either -// directly or indirectly, these structs may also need to be registered in -// order to support encoding and decoding of values of these types. That is the -// responsibility of the calling application. -var InternalTypesToRegister []interface{} - -func init() { - InternalTypesToRegister = []interface{}{ - primitiveType{}, - typeList{}, - typeMap{}, - typeObject{}, - typeSet{}, - setRules{}, - set.Set{}, - typeTuple{}, - big.Float{}, - capsuleType{}, - []interface{}(nil), - map[string]interface{}(nil), - } - - // Register these with gob here, rather than in gob.go, to ensure - // that this will always happen after we build the above. - for _, tv := range InternalTypesToRegister { - typeName := fmt.Sprintf("%T", tv) - if strings.HasPrefix(typeName, "cty.") { - gob.RegisterName(fmt.Sprintf("github.com/zclconf/go-cty/%s", typeName), tv) - } else { - gob.Register(tv) - } - } -}