encoding/gob: fix infinite recursion caused by ignoring recursive type

This was a simple oversight: the algorithm to handle recursive types
needed to be applied to the ignore-item case as well.

Fixes #10415.

Change-Id: I39ef31cad680ab8334e141f60d2f8707896785d1
Reviewed-on: https://go-review.googlesource.com/8942
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

diff --git a/src/encoding/gob/decode.go b/src/encoding/gob/decode.go
index b34110f6f9017f7844299ff1c3ee3180c5ece8fc..e913f15c545a240cc0760cd681fcc10af9664653 100644 (file)
--- a/src/encoding/gob/decode.go
+++ b/src/encoding/gob/decode.go
@@ -863,16 +863,22 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg
 }
 
 // decIgnoreOpFor returns the decoding op for a field that has no destination.
-func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp {
+func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp) *decOp {
+       // If this type is already in progress, it's a recursive type (e.g. map[string]*T).
+       // Return the pointer to the op we're already building.
+       if opPtr := inProgress[wireId]; opPtr != nil {
+               return opPtr
+       }
        op, ok := decIgnoreOpMap[wireId]
        if !ok {
+               inProgress[wireId] = &op
                if wireId == tInterface {
                        // Special case because it's a method: the ignored item might
                        // define types and we need to record their state in the decoder.
                        op = func(i *decInstr, state *decoderState, value reflect.Value) {
                                state.dec.ignoreInterface(state)
                        }
-                       return op
+                       return &op
                }
                // Special cases
                wire := dec.wireType[wireId]
@@ -881,25 +887,25 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp {
                        errorf("bad data: undefined type %s", wireId.string())
                case wire.ArrayT != nil:
                        elemId := wire.ArrayT.Elem
-                       elemOp := dec.decIgnoreOpFor(elemId)
+                       elemOp := dec.decIgnoreOpFor(elemId, inProgress)
                        op = func(i *decInstr, state *decoderState, value reflect.Value) {
-                               state.dec.ignoreArray(state, elemOp, wire.ArrayT.Len)
+                               state.dec.ignoreArray(state, *elemOp, wire.ArrayT.Len)
                        }
 
                case wire.MapT != nil:
                        keyId := dec.wireType[wireId].MapT.Key
                        elemId := dec.wireType[wireId].MapT.Elem
-                       keyOp := dec.decIgnoreOpFor(keyId)
-                       elemOp := dec.decIgnoreOpFor(elemId)
+                       keyOp := dec.decIgnoreOpFor(keyId, inProgress)
+                       elemOp := dec.decIgnoreOpFor(elemId, inProgress)
                        op = func(i *decInstr, state *decoderState, value reflect.Value) {
-                               state.dec.ignoreMap(state, keyOp, elemOp)
+                               state.dec.ignoreMap(state, *keyOp, *elemOp)
                        }
 
                case wire.SliceT != nil:
                        elemId := wire.SliceT.Elem
-                       elemOp := dec.decIgnoreOpFor(elemId)
+                       elemOp := dec.decIgnoreOpFor(elemId, inProgress)
                        op = func(i *decInstr, state *decoderState, value reflect.Value) {
-                               state.dec.ignoreSlice(state, elemOp)
+                               state.dec.ignoreSlice(state, *elemOp)
                        }
 
                case wire.StructT != nil:
@@ -922,7 +928,7 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp {
        if op == nil {
                errorf("bad data: ignore can't handle type %s", wireId.string())
        }
-       return op
+       return &op
 }
 
 // gobDecodeOpFor returns the op for a type that is known to implement
@@ -1056,9 +1062,9 @@ func (dec *Decoder) compileSingle(remoteId typeId, ut *userTypeInfo) (engine *de
 func (dec *Decoder) compileIgnoreSingle(remoteId typeId) (engine *decEngine, err error) {
        engine = new(decEngine)
        engine.instr = make([]decInstr, 1) // one item
-       op := dec.decIgnoreOpFor(remoteId)
+       op := dec.decIgnoreOpFor(remoteId, make(map[typeId]*decOp))
        ovfl := overflow(dec.typeString(remoteId))
-       engine.instr[0] = decInstr{op, 0, nil, ovfl}
+       engine.instr[0] = decInstr{*op, 0, nil, ovfl}
        engine.numInstr = 1
        return
 }
@@ -1101,8 +1107,8 @@ func (dec *Decoder) compileDec(remoteId typeId, ut *userTypeInfo) (engine *decEn
                localField, present := srt.FieldByName(wireField.Name)
                // TODO(r): anonymous names
                if !present || !isExported(wireField.Name) {
-                       op := dec.decIgnoreOpFor(wireField.Id)
-                       engine.instr[fieldnum] = decInstr{op, fieldnum, nil, ovfl}
+                       op := dec.decIgnoreOpFor(wireField.Id, make(map[typeId]*decOp))
+                       engine.instr[fieldnum] = decInstr{*op, fieldnum, nil, ovfl}
                        continue
                }
                if !dec.compatibleType(localField.Type, wireField.Id, make(map[reflect.Type]typeId)) {

diff --git a/src/encoding/gob/encoder_test.go b/src/encoding/gob/encoder_test.go
index 8a72a3118cf84a0850973b50cf74596755c92c46..dc657348223ce005dc3055ea9d68da918a3697fc 100644 (file)
--- a/src/encoding/gob/encoder_test.go
+++ b/src/encoding/gob/encoder_test.go
@@ -527,6 +527,30 @@ func TestDecodeIntoNothing(t *testing.T) {
        }
 }
 
+func TestIgnoreRecursiveType(t *testing.T) {
+       // It's hard to build a self-contained test for this because
+       // we can't build compatible types in one package with
+       // different items so something is ignored. Here is
+       // some data that represents, according to debug.go:
+       // type definition {
+       //      slice "recursiveSlice" id=106
+       //              elem id=106
+       // }
+       data := []byte{
+               0x1d, 0xff, 0xd3, 0x02, 0x01, 0x01, 0x0e, 0x72,
+               0x65, 0x63, 0x75, 0x72, 0x73, 0x69, 0x76, 0x65,
+               0x53, 0x6c, 0x69, 0x63, 0x65, 0x01, 0xff, 0xd4,
+               0x00, 0x01, 0xff, 0xd4, 0x00, 0x00, 0x07, 0xff,
+               0xd4, 0x00, 0x02, 0x01, 0x00, 0x00,
+       }
+       dec := NewDecoder(bytes.NewReader(data))
+       // Issue 10415: This caused infinite recursion.
+       err := dec.Decode(nil)
+       if err != nil {
+               t.Fatal(err)
+       }
+}
+
 // Another bug from golang-nuts, involving nested interfaces.
 type Bug0Outer struct {
        Bug0Field interface{}