From 279fcc66b8dd3333acedf3776f2150f86b1e3277 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Sat, 20 Jun 2020 23:30:50 -0700 Subject: [PATCH] Swallow panic when calling String or Error If a panic occurs while calling String or Error, the reporter recovers from it and ignores it, proceeding with its usual functionality for formatting a value. --- cmp/compare_test.go | 13 +++++++++++++ cmp/report_reflect.go | 18 ++++++++++++------ cmp/testdata/diffs | 12 ++++++++++++ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/cmp/compare_test.go b/cmp/compare_test.go index c94f6c0..b34530b 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -8,6 +8,7 @@ import ( "bytes" "crypto/md5" "encoding/json" + "errors" "flag" "fmt" "io" @@ -874,6 +875,18 @@ func reporterTests() []test { ) return []test{{ + label: label + "/PanicStringer", + x: struct{ X fmt.Stringer }{struct{ fmt.Stringer }{nil}}, + y: struct{ X fmt.Stringer }{bytes.NewBuffer(nil)}, + wantEqual: false, + reason: "panic from fmt.Stringer should not crash the reporter", + }, { + label: label + "/PanicError", + x: struct{ X error }{struct{ error }{nil}}, + y: struct{ X error }{errors.New("")}, + wantEqual: false, + reason: "panic from error should not crash the reporter", + }, { label: label + "/AmbiguousType", x: foo1.Bar{}, y: foo2.Bar{}, diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index 2d722ea..28e0e92 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -125,12 +125,18 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, // implementations crash when doing so. if (t.Kind() != reflect.Ptr && t.Kind() != reflect.Interface) || !v.IsNil() { var prefix, strVal string - switch v := v.Interface().(type) { - case error: - prefix, strVal = "e", v.Error() - case fmt.Stringer: - prefix, strVal = "s", v.String() - } + func() { + // Swallow and ignore any panics from String or Error. + defer func() { recover() }() + switch v := v.Interface().(type) { + case error: + strVal = v.Error() + prefix = "e" + case fmt.Stringer: + strVal = v.String() + prefix = "s" + } + }() if prefix != "" { maxLen := len(strVal) if opts.LimitVerbosity { diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index 949e8d1..05fa3fd 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -252,6 +252,18 @@ })), } >>> TestDiff/Transformer/AcyclicString +<<< TestDiff/Reporter/PanicStringer + struct{ X fmt.Stringer }{ +- X: struct{ fmt.Stringer }{}, ++ X: s"", + } +>>> TestDiff/Reporter/PanicStringer +<<< TestDiff/Reporter/PanicError + struct{ X error }{ +- X: struct{ error }{}, ++ X: e"", + } +>>> TestDiff/Reporter/PanicError <<< TestDiff/Reporter/AmbiguousType interface{}( - "github.com/google/go-cmp/cmp/internal/teststructs/foo1".Bar{},