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

protodesc: too strict about proto3 field names when creating descriptors from FileDescriptorProto #1616

Closed
jhump opened this issue May 7, 2024 · 0 comments
Assignees

Comments

@jhump
Copy link
Contributor

jhump commented May 7, 2024

The following simple file works just fine with protoc:

syntax = "proto3";
message Foo {
  string fooBar = 1;
  string FOO_BAR = 2;
}

The resulting descriptor has fooBar for the JSON name of the first and FOOBAR for the second.

But protodesc.NewFile with this descriptor fails:

proto: message "Foo" using proto3 semantics has conflict: "FOO_BAR" with "fooBar"

The checks in protoc once upon a time were case-insensitive, and I guess the Go runtime mirrored that validation check. But the implementation in protoc has since changed. (It is now case-sensitive; it also now includes checks to avoid collisions on custom JSON names.) The Go runtime needs to be changed to follow suit so that it can correctly handle any descriptor produced by protoc.

Also, the error message says "proto3 semantics". And, sure enough, if I change this to edition = "2023";, then the protodesc.NewFile operation succeeds! However, if there really were a name conflict (due to conflict in default computed JSON names), it should be an error in edition 2023, too, since that message's JSON format feature is ALLOW (since that's the default for 2023, unless overridden via option features.json_format = LEGACY_BEST_EFFORT).

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

2 participants