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

Bug: Leading slash for type urls #617

Closed
grod220 opened this issue Nov 16, 2023 · 2 comments · Fixed by #618
Closed

Bug: Leading slash for type urls #617

grod220 opened this issue Nov 16, 2023 · 2 comments · Fixed by #618

Comments

@grod220
Copy link
Contributor

grod220 commented Nov 16, 2023

I have a grpc query that returns a field with a google.protobuf.Any. At runtime it has the type url of: /ibc.lightclients.tendermint.v1.ClientState

The corresponding message it represents has the type url of: type.googleapis.com/ibc.lightclients.tendermint.v1.ClientState.

The discrepancy of type urls makes our registry not able to match these during the Any unpacking. Here is the logic that converts type urls to type names:

const name = slash > 0 ? url.substring(slash + 1) : url;

This logic gives us:

Url: /ibc.lightclients.tendermint.v1.ClientState
Name: /ibc.lightclients.tendermint.v1.ClientState

Url: type.googleapis.com/ibc.lightclients.tendermint.v1.ClientState
Name: ibc.lightclients.tendermint.v1.ClientState

No match ❌

Can Any.prototype.typeUrlToName be expanded to handle leading slashes?

@timostamm
Copy link
Member

The implementation assumes that type_url is a valid URL (optionally omitting the scheme) based on the documentation.

But it doesn't hurt to relax this a bit and accept just a leading slash. I just checked the Go implementation, and it also accepts this form.

@grod220
Copy link
Contributor Author

grod220 commented Nov 16, 2023

Opened a PR, thanks for the review 🙏

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

Successfully merging a pull request may close this issue.

2 participants