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

protoreflect x dynamicpb interaction: should descriptors match names or be pointer-identical? #1633

Open
stapelberg opened this issue Jul 26, 2024 · 0 comments

Comments

@stapelberg
Copy link

Currently, when using the protoreflect package, the descriptors have to be pointer-identical: https://github.com/protocolbuffers/protobuf-go/blob/219bda23ffda544ed4cc5d5a75d34ce3b100ce51/internal/impl/message_reflect.go#L438-L443

However, the dynamicpb package is less strict and allows setting a message with a non-identical descriptor, as long as the descriptor name matches: https://github.com/protocolbuffers/protobuf-go/blob/219bda23ffda544ed4cc5d5a75d34ce3b100ce51/types/dynamicpb/dynamic.go#L589-L591

This means users can get into a situation where they mix descriptors of different sources (e.g. global proto registry vs. a custom registry) in the same message hierarchy, which ultimately means the protoreflect package will panic when calling e.g. Has.

Here’s a demo CL which contains a failing test for this issue, to illustrate: https://go.dev/cl/601255

Here’s a CL which adds validation to prevent this issue: https://go.dev/cl/500315

It’s not entirely clear to us whether we can make the validation stricter without risking breakage. If anyone has any information one way or the other, please let us know.

If we can’t make a decision on this, we could change the behavior, but also introduce an escape hatch like we have in the protoregistry package: https://github.com/protocolbuffers/protobuf-go/blob/219bda23ffda544ed4cc5d5a75d34ce3b100ce51/reflect/protoregistry/registry.go#L30-L65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant