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>

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)) {

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{}