markers: reject a decoded mark payload with no type marks

markers/markers.go

@@ -268,11 +268,13 @@ func encodeMark(_ context.Context, err error) (msg string, _ []string, payload p
 
 func decodeMark(_ context.Context, cause error, _ string, _ []string, payload proto.Message) error {
 	m, ok := payload.(*errorspb.MarkPayload)
-	if !ok {
+	if !ok || len(m.Types) == 0 {
 		// If this ever happens, this means some version of the library
-		// (presumably future) changed the payload type, and we're
-		// receiving this here. In this case, give up and let
-		// DecodeError use the opaque type.
+		// (presumably future) changed the payload type or stopped
+		// filling in the type marks, and we're receiving this here. In
+		// this case, give up and let DecodeError use the opaque type.
+		// A mark without any type would also break SafeFormatError
+		// below, which reports types[0].
 		return nil
 	}
 	return &withMark{cause: cause, mark: errorMark{msg: m.Msg, types: m.Types}}

markers/markers_test.go

@@ -24,8 +24,10 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/errors/errbase"
+	"github.com/cockroachdb/errors/errorspb"
 	"github.com/cockroachdb/errors/markers"
 	"github.com/cockroachdb/errors/testutils"
+	"github.com/gogo/protobuf/types"
 	pkgErr "github.com/pkg/errors"
 )
 
@@ -803,3 +805,31 @@ func (errorUncomparable) Is(target error) bool {
 	_, ok := target.(errorUncomparable)
 	return ok
 }
+
+// TestDecodeMarkWithoutTypes checks that a MarkPayload that arrives
+// without any type marks does not break the formatting of the decoded
+// error. The payload is built by hand because the local encoder always
+// emits at least one type mark; a peer running a different version of
+// the library is not bound by that.
+func TestDecodeMarkWithoutTypes(t *testing.T) {
+	ctx := context.Background()
+
+	enc := errbase.EncodeError(ctx, markers.Mark(errors.New("boom"), errors.New("refmsg")))
+	w := enc.GetWrapper()
+	if w == nil {
+		t.Fatalf("expected a wrapper, got %+v", enc)
+	}
+	payload, err := types.MarshalAny(&errorspb.MarkPayload{Msg: "refmsg"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	w.Details.FullDetails = payload
+
+	decoded := errbase.DecodeError(ctx, enc)
+	if s := decoded.Error(); s != "boom" {
+		t.Errorf("Error(): expected %q, got %q", "boom", s)
+	}
+	if s := fmt.Sprintf("%+v", decoded); strings.Contains(s, "PANIC=") {
+		t.Errorf("formatting the decoded error panicked: %s", s)
+	}
+}