Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

attributes: print typed nil values instead of panic #6574

Merged
merged 8 commits into from
Sep 22, 2023
72 changes: 69 additions & 3 deletions attributes/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ package attributes

import (
"fmt"
"reflect"
"strings"
)

Expand Down Expand Up @@ -121,13 +122,78 @@ func (a *Attributes) String() string {
return sb.String()
}

func str(x any) string {
if v, ok := x.(fmt.Stringer); ok {
const nilAngleString = "<nil>"

func str(x any) (s string) {
defer func() {
if r := recover(); r != nil {
// If it panics with a nil pointer, just say "<nil>". The likeliest causes are a
// [fmt.Stringer] that fails to guard against nil or a nil pointer for a
// value receiver, and in either case, "<nil>" is a nice result.
//
// Adapted from the code in fmt/print.go.
if v := reflect.ValueOf(x); v.Kind() == reflect.Pointer && v.IsNil() {
s = nilAngleString
return
}

// The panic was likely not caused by fmt.Stringer.
panic(r)
}
}()
if x == nil { // NOTE: typed nils will not be caught by this check
return nilAngleString
searKing marked this conversation as resolved.
Show resolved Hide resolved
} else if v, ok := x.(fmt.Stringer); ok {
return v.String()
searKing marked this conversation as resolved.
Show resolved Hide resolved
} else if v, ok := x.(string); ok {
return v
}
return fmt.Sprintf("<%p>", x)
value := reflect.ValueOf(x)
switch value.Kind() {
case reflect.Chan, reflect.Func, reflect.Map, reflect.Pointer, reflect.Slice, reflect.UnsafePointer:
return fmt.Sprintf("<%p>", x)
default:
// This will call badVerb to print as "<%p>", but without leading "%!(" and tailing ")"
return badVerb(x, value)
}
searKing marked this conversation as resolved.
Show resolved Hide resolved
}

// badVerb is like fmt.Sprintf("%p", arg), but with
// leading "%!verb(" replaced by "<" and tailing ")" replaced by ">".
// If an invalid argument is given for a '%p', such as providing
// an int to %p, the generated string will contain a
// description of the problem, as in these examples:
//
// # our style
//
// Wrong type or unknown verb: <type=value>
// Printf("%p", 1): <int=1>
//
// # fmt style as `fmt.Sprintf("%p", arg)`
//
// Wrong type or unknown verb: %!verb(type=value)
// Printf("%p", 1): %!d(int=1)
//
// Adapted from the code in fmt/print.go.
func badVerb(arg any, value reflect.Value) string {
var buf strings.Builder
switch {
case arg != nil:
buf.WriteByte('<')
buf.WriteString(reflect.TypeOf(arg).String())
buf.WriteByte('=')
_, _ = fmt.Fprintf(&buf, "%v", arg)
buf.WriteByte('>')
case value.IsValid():
buf.WriteByte('<')
buf.WriteString(value.Type().String())
buf.WriteByte('=')
_, _ = fmt.Fprintf(&buf, "%v", 0)
buf.WriteByte('>')
default:
buf.WriteString(nilAngleString)
}
return buf.String()
}

// MarshalJSON helps implement the json.Marshaler interface, thereby rendering
Expand Down
38 changes: 38 additions & 0 deletions attributes/attributes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ func (s stringVal) Equal(o any) bool {
return ok && s.s == os.s
}

type stringerVal struct {
s string
}

func (s stringerVal) String() string {
return s.s
}

func ExampleAttributes() {
type keyOne struct{}
type keyTwo struct{}
Expand All @@ -57,6 +65,36 @@ func ExampleAttributes_WithValue() {
// Key two: {two}
}

func ExampleAttributes_String() {
type key struct{}
var typedNil *stringerVal
a1 := attributes.New(key{}, typedNil) // typed nil implements [fmt.Stringer]
a2 := attributes.New(key{}, (*stringerVal)(nil)) // typed nil implements [fmt.Stringer]
a3 := attributes.New(key{}, (*stringVal)(nil)) // typed nil not implements [fmt.Stringer]
a4 := attributes.New(key{}, nil) // untyped nil
a5 := attributes.New(key{}, 1)
a6 := attributes.New(key{}, stringerVal{s: "two"})
a7 := attributes.New(key{}, stringVal{s: "two"})
a8 := attributes.New(1, true)
fmt.Println("a1:", a1.String())
fmt.Println("a2:", a2.String())
fmt.Println("a3:", a3.String())
fmt.Println("a4:", a4.String())
fmt.Println("a5:", a5.String())
fmt.Println("a6:", a6.String())
fmt.Println("a7:", a7.String())
fmt.Println("a8:", a8.String())
// Output:
// a1: {"<attributes_test.key={}>": "<nil>" }
// a2: {"<attributes_test.key={}>": "<nil>" }
// a3: {"<attributes_test.key={}>": "<0x0>" }
// a4: {"<attributes_test.key={}>": "<nil>" }
// a5: {"<attributes_test.key={}>": "<int=1>" }
// a6: {"<attributes_test.key={}>": "two" }
// a7: {"<attributes_test.key={}>": "<attributes_test.stringVal={two}>" }
// a8: {"<int=1>": "<bool=true>" }
}

// Test that two attributes with the same content are Equal.
func TestEqual(t *testing.T) {
type keyOne struct{}
Expand Down
22 changes: 22 additions & 0 deletions resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,25 @@ func (s) TestResolverAddressesToEndpoints(t *testing.T) {
t.Fatalf("timed out waiting for endpoints")
}
}

// TestResolverAddressesWithTypedNilAttribute ensures no panic if typed-nil attributes within resolver.State.Addresses
searKing marked this conversation as resolved.
Show resolved Hide resolved
func (s) TestResolverAddressesWithTypedNilAttribute(t *testing.T) {
const scheme = "testresolveraddresseswithtypednilattribute"
r := manual.NewBuilderWithScheme(scheme)
searKing marked this conversation as resolved.
Show resolved Hide resolved
resolver.Register(r)

addrAttr := attributes.New("typed_nil", (*stringerVal)(nil))
r.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: "addr1", Attributes: addrAttr}}})

cc, err := Dial(r.Scheme()+":///",
WithTransportCredentials(insecure.NewCredentials()),
WithResolvers(r))
searKing marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
t.Fatalf("Unexpected error dialing: %v", err)
}
defer cc.Close()
}

type stringerVal struct{ s string }

func (s stringerVal) String() string { return s.s }