Print count of unrecognized bytes in "buf curl" (#2586)

jhump · web-flow · commit 9aaf28d23972 · 2023-11-14T09:22:33.000-05:00
This provides a cue to user's that data is in the response but
getting dropped (which suggests an incomplete or out-of-date
schema) by logging the presence of unrecognized bytes when
the `-v` flag is used.
diff --git a/private/buf/bufcurl/invoker.go b/private/buf/bufcurl/invoker.go
@@ -290,6 +290,11 @@ func (inv *invoker) handleResponse(data []byte, msg *dynamicpb.Message) error {
 			protoencoding.JSONMarshalerWithEmitUnpopulated(),
 		)
 	}
+	unrecognized := countUnrecognized(msg.ProtoReflect())
+	if unrecognized > 0 {
+		inv.printer.Printf("Response message (%s) contained %d bytes of unrecognized fields.",
+			msg.ProtoReflect().Descriptor().FullName(), unrecognized)
+	}
 	outputBytes, err := protoencoding.NewJSONMarshaler(inv.res, jsonMarshalerOptions...).Marshal(msg)
 	if err != nil {
 		return err
@@ -437,6 +442,37 @@ func (s *streamMessageProvider) next(msg proto.Message) error {
 		}
 		return fmt.Errorf("%s at offset %d: %w", s.name, s.dec.InputOffset(), err)
 	}
-	proto.Reset(msg)
-	return protoencoding.NewJSONUnmarshaler(s.res).Unmarshal(jsonData, msg)
+	return protoencoding.NewJSONUnmarshaler(
+		s.res, protoencoding.JSONUnmarshalerWithDisallowUnknown(),
+	).Unmarshal(jsonData, msg)
+}
+
+func countUnrecognized(msg protoreflect.Message) int {
+	var count int
+	msg.Range(func(field protoreflect.FieldDescriptor, val protoreflect.Value) bool {
+		switch {
+		case field.IsMap() && isMessageKind(field.MapValue().Kind()):
+			// Note: Technically, each message entry could have had unrecognized field
+			// bytes, but they are discarded by the runtime. So we can only look at
+			// unrecognized fields in message values inside the map.
+			mapVal := val.Map()
+			mapVal.Range(func(_ protoreflect.MapKey, v protoreflect.Value) bool {
+				count += countUnrecognized(v.Message())
+				return true
+			})
+		case field.IsList() && isMessageKind(field.Kind()):
+			listVal := val.List()
+			for i, length := 0, listVal.Len(); i < length; i++ {
+				count += countUnrecognized(listVal.Get(i).Message())
+			}
+		case isMessageKind(field.Kind()):
+			count += countUnrecognized(val.Message())
+		}
+		return true
+	})
+	return count + len(msg.GetUnknown())
+}
+
+func isMessageKind(k protoreflect.Kind) bool {
+	return k == protoreflect.MessageKind || k == protoreflect.GroupKind
 }
diff --git a/private/pkg/protoencoding/json_unmarshaler.go b/private/pkg/protoencoding/json_unmarshaler.go
@@ -20,20 +20,24 @@ import (
 )
 
 type jsonUnmarshaler struct {
-	resolver Resolver
+	resolver        Resolver
+	disallowUnknown bool
 }
 
-func newJSONUnmarshaler(resolver Resolver) Unmarshaler {
-	return &jsonUnmarshaler{
+func newJSONUnmarshaler(resolver Resolver, options ...JSONUnmarshalerOption) Unmarshaler {
+	jsonUnmarshaler := &jsonUnmarshaler{
 		resolver: resolver,
 	}
+	for _, option := range options {
+		option(jsonUnmarshaler)
+	}
+	return jsonUnmarshaler
 }
 
 func (m *jsonUnmarshaler) Unmarshal(data []byte, message proto.Message) error {
 	options := protojson.UnmarshalOptions{
-		Resolver: m.resolver,
-		// TODO: make this an option
-		DiscardUnknown: true,
+		Resolver:       m.resolver,
+		DiscardUnknown: !m.disallowUnknown,
 	}
 	return options.Unmarshal(data, message)
 }
diff --git a/private/pkg/protoencoding/protoencoding.go b/private/pkg/protoencoding/protoencoding.go
@@ -117,8 +117,18 @@ func NewWireUnmarshaler(resolver Resolver) Unmarshaler {
 // NewJSONUnmarshaler returns a new Unmarshaler for json.
 //
 // resolver can be nil if unknown and are only needed for extensions.
-func NewJSONUnmarshaler(resolver Resolver) Unmarshaler {
-	return newJSONUnmarshaler(resolver)
+func NewJSONUnmarshaler(resolver Resolver, options ...JSONUnmarshalerOption) Unmarshaler {
+	return newJSONUnmarshaler(resolver, options...)
+}
+
+// JSONUnmarshalerOption is an option for a new JSONUnmarshaler.
+type JSONUnmarshalerOption func(*jsonUnmarshaler)
+
+// JSONUnmarshalerWithDisallowUnknown says to disallow unrecognized fields.
+func JSONUnmarshalerWithDisallowUnknown() JSONUnmarshalerOption {
+	return func(jsonUnmarshaler *jsonUnmarshaler) {
+		jsonUnmarshaler.disallowUnknown = true
+	}
 }
 
 // NewTxtpbUnmarshaler returns a new Unmarshaler for txtpb.

Original file line number	Diff line number	Diff line change
`@@ -20,20 +20,24 @@ import (`
`20`	`20`	`)`
`21`	`21`
`22`	`22`	`type jsonUnmarshaler struct {`
`23`		`- resolver Resolver`
	`23`	`+ resolver Resolver`
	`24`	`+ disallowUnknown bool`
`24`	`25`	`}`
`25`	`26`
`26`		`-func newJSONUnmarshaler(resolver Resolver) Unmarshaler {`
`27`		`- return &jsonUnmarshaler{`
	`27`	`+func newJSONUnmarshaler(resolver Resolver, options ...JSONUnmarshalerOption) Unmarshaler {`
	`28`	`+ jsonUnmarshaler := &jsonUnmarshaler{`
`28`	`29`	`resolver: resolver,`
`29`	`30`	`}`
	`31`	`+ for _, option := range options {`
	`32`	`+ option(jsonUnmarshaler)`
	`33`	`+ }`
	`34`	`+ return jsonUnmarshaler`
`30`	`35`	`}`
`31`	`36`
`32`	`37`	`func (m *jsonUnmarshaler) Unmarshal(data []byte, message proto.Message) error {`
`33`	`38`	`options := protojson.UnmarshalOptions{`
`34`		`- Resolver: m.resolver,`
`35`		`- // TODO: make this an option`
`36`		`- DiscardUnknown: true,`
	`39`	`+ Resolver: m.resolver,`
	`40`	`+ DiscardUnknown: !m.disallowUnknown,`
`37`	`41`	`}`
`38`	`42`	`return options.Unmarshal(data, message)`
`39`	`43`	`}`