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

Registering protobuf service or interface type twice should cause an error #7679

Closed
aaronc opened this issue Oct 26, 2020 · 0 comments · Fixed by #7729
Closed

Registering protobuf service or interface type twice should cause an error #7679

aaronc opened this issue Oct 26, 2020 · 0 comments · Fixed by #7729
Assignees

Comments

@aaronc
Copy link
Member

aaronc commented Oct 26, 2020

Summary

Disallow duplicate registration of protobuf services and interface implementations.

Problem Definition

GRPCQueryRouter, MsgServiceRouter and InterfaceRegistry currently do not error or panic on duplicate registration.

Especially for service registration (GRPCQueryRouter or MsgServiceRouter) we should only allow a single registration because a duplicate registration means there are two modules trying to register the same service and that is either a configuration error or something malicious.

Registering an interface implementation twice is maybe not so bad but could also be problematic. At least we should verify that the exact same concrete type is registered for the same type URL.

Proposal

Either panic or return an error (requires a method signature change) on duplicate service registration.

For duplicate interface implementation registration (InterfaceRegistry.RegisterImplementations), either:

  • panic or error always on duplicate registrations,
  • or only panic or error when the same type URL is registered with a different concrete type (this is probably preferable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants