Ensure protodesc.FileDescriptor references are sourced from the globa…

…l proto registry when possible (#411)

common/types/pb/BUILD.bazel

@@ -19,6 +19,7 @@ go_library(
         "@org_golang_google_genproto//googleapis/api/expr/v1alpha1:go_default_library",
         "@org_golang_google_protobuf//proto:go_default_library",
         "@org_golang_google_protobuf//reflect/protoreflect:go_default_library",
+        "@org_golang_google_protobuf//reflect/protoregistry:go_default_library",
         "@org_golang_google_protobuf//types/dynamicpb:go_default_library",
         "@org_golang_google_protobuf//types/known/anypb:go_default_library",
         "@org_golang_google_protobuf//types/known/durationpb:go_default_library",
@@ -43,6 +44,7 @@ go_test(
         "//test/proto2pb:test_all_types_go_proto",
         "//test/proto3pb:test_all_types_go_proto",
         "@org_golang_google_protobuf//reflect/protodesc:go_default_library",
+        "@org_golang_google_protobuf//reflect/protoreflect:go_default_library",
         "@org_golang_google_protobuf//types/descriptorpb:go_default_library",
     ],
 )

common/types/pb/pb.go

@@ -19,6 +19,7 @@ package pb
 import (
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
+	"google.golang.org/protobuf/reflect/protoregistry"
 
 	anypb "google.golang.org/protobuf/types/known/anypb"
 	durpb "google.golang.org/protobuf/types/known/durationpb"
@@ -29,6 +30,10 @@ import (
 )
 
 // Db maps from file / message / enum name to file description.
+//
+// Each Db is isolated from each other, and while information about protobuf descriptors may be
+// fetched from the global protobuf registry, no descriptors are added to this registry, else
+// the isolation guarantees of the Db object would be violated.
 type Db struct {
 	revFileDescriptorMap map[string]*FileDescription
 	// files contains the deduped set of FileDescriptions whose types are contained in the pb.Db.
@@ -93,6 +98,18 @@ func (pbdb *Db) RegisterDescriptor(fileDesc protoreflect.FileDescriptor) (*FileD
 	if found {
 		return fd, nil
 	}
+	// Make sure to search the global registry to see if a protoreflect.FileDescriptor for
+	// the file specified has been linked into the binary. If so, use the copy of the descriptor
+	// from the global cache.
+	//
+	// Note: Proto reflection relies on descriptor values being object equal rather than object
+	// equivalence. This choice means that a FieldDescriptor generated from a FileDescriptorProto
+	// will be incompatible with the FieldDescriptor in the global registry and any message created
+	// from that global registry.
+	globalFD, err := protoregistry.GlobalFiles.FindFileByPath(fileDesc.Path())
+	if err == nil {
+		fileDesc = globalFD
+	}
 	fd = NewFileDescription(fileDesc, pbdb)
 	for _, enumValName := range fd.GetEnumNames() {
 		pbdb.revFileDescriptorMap[enumValName] = fd

common/types/pb/pb_test.go

@@ -20,7 +20,11 @@ import (
 	"reflect"
 	"testing"
 
+	"google.golang.org/protobuf/reflect/protodesc"
+	"google.golang.org/protobuf/reflect/protoreflect"
+
 	proto3pb "github.com/google/cel-go/test/proto3pb"
+	descpb "google.golang.org/protobuf/types/descriptorpb"
 )
 
 func TestDbCopy(t *testing.T) {
@@ -40,3 +44,46 @@ func TestDbCopy(t *testing.T) {
 		t.Error("db.Copy() did not result in eqivalent objects.")
 	}
 }
+
+func TestProtoReflectRoundTrip(t *testing.T) {
+	msg := &proto3pb.TestAllTypes{SingleBool: true}
+	fdMap := CollectFileDescriptorSet(msg)
+	files := []*descpb.FileDescriptorProto{}
+	for _, fd := range fdMap {
+		files = append(files, protodesc.ToFileDescriptorProto(fd))
+	}
+	// Round-tripping from a protoreflect.FileDescriptor to a FileDescriptorSet and back
+	// will result in completely independent copies of the protoreflect.FileDescriptor
+	// whose values are incompatible with each other.
+	//
+	// This test showcases what happens when the protoregistry.GlobalFiles values are
+	// used when a given protoreflect.FileDescriptor is linked into the binary.
+	fds := &descpb.FileDescriptorSet{File: files}
+	pbReg, err := protodesc.NewFiles(fds)
+	if err != nil {
+		t.Fatalf("protodesc.NewFiles() failed: %v", err)
+	}
+	pbdb := NewDb()
+	pbReg.RangeFiles(func(fd protoreflect.FileDescriptor) bool {
+		_, err := pbdb.RegisterDescriptor(fd)
+		if err != nil {
+			t.Fatalf("pbdb.RegisterDecriptor(%v) failed: %v", fd, err)
+		}
+		return true
+	})
+	msgType, found := pbdb.DescribeType("google.expr.proto3.test.TestAllTypes")
+	if !found {
+		t.Fatal("pbdb.DescribeType(google.expr.proto3.test.TestAllTypes) failed")
+	}
+	boolField, found := msgType.FieldByName("single_bool")
+	if !found {
+		t.Fatal("msgType.FieldByName(single_bool) failed")
+	}
+	val, err := boolField.GetFrom(msg)
+	if err != nil {
+		t.Errorf("boolField.GetFrom(msg) failed: %v", err)
+	}
+	if val != true {
+		t.Errorf("got TestAllTypes.single_bool %v, wanted true", val)
+	}
+}