search

golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.79k stars 1.58k forks source link

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

Closed jhump closed 6 months ago

jhump commented 6 months ago

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).